On 05/02/2017 05:36 PM, Paul Oranje wrote: > Assignment within a condition is easily read by (dyslectic) humans as a test > for equality (==) and is for that reason als better avoided. > Paul > >> Op 2 mei 2017, om 18:43 heeft Philip Prindeville >> <philipp_s...@redfish-solutions.com> het volgende geschreven: >> >> >>> On May 2, 2017, at 6:15 AM, Pierre Lebleu <pme.leb...@gmail.com> wrote: >>> >>> Hi Philip, >>> >>> 2017-04-29 3:11 GMT+02:00 Philip Prindeville >>> <philipp_s...@redfish-solutions.com>: >>> Inline… >>> >>> >>> [snip] >>>> + if (!(ipset = fw3_alloc_ipset(state))) >>> >>> >>> Minor nit… Assignments inside of conditionals are a bear to step through >>> in a debugger like GDB. >>> >>> -Philip >>> >>> It is a trivial assignment and it is already done in this style along the >>> file. >>> >>> -- >>> Pierre >>> >> >> >> It’s not about trivial vs. nontrivial. It’s about whether you could step >> through the assignment with (say) gdb, execute just the assignment, examine >> the value, and then step through the “if”. And the answer is, “you can’t”. >> Because gdb is a source level debugger where the unit of source is the >> “line”. >> >> (Actually, it’s also the unit of source for gcc when it generates debugging >> symbols.) >> >> The way to separate to 2 individual statements in C (for the purposes of gdb >> debugging) is to put them on separate lines. Yes, that’s a glaring >> limitation of gcc and gdb, but that’s our reality. >> >> As for what’s already done in this style in the file… Having separate >> assignments and tests is *also* done, and indeed it’s done more often. >> >> -Philip
Others may have been clever but it doesn't mean a construct should continue. This if-assignment combo and other clever C constructs are frowned upon in popular software quality standards. Many simply are due to the above mentioned maintainability issues. Some industries may be more sensitive, but the rationale for avoidance is solid. _______________________________________________ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
