[LEDE-DEV] [PATCH 5/7] firewall3: add UBUS support for ipset sections

[Philip Prindeville]

> On May 2, 2017, at 6:15 AM, Pierre Lebleu <pme.lebleu at gmail.com> wrote:
> 
> Hi Philip,
> 
> 2017-04-29 3:11 GMT+02:00 Philip Prindeville <philipp_subx at 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