On 13/03/2015 16:47, Roman Khimov wrote:
> Both scan-build and cppcheck complain here. Sure, it's not an error, just a
> harmless dead code, but well, tools don't like dead code and I personally
> don't like it either, so IMO it's better to drop it if there are no valid
> reasons for it to stay.
Fine, I removed it. *shrug*
> Speaking of dead code, cppcheck also sees some in src/sysdeps/trycmsgcloexec.c
> and src/sysdeps/trygetpeerucred.c, but from what I see those are currently
> stubs, so I didn't touch them.
Yes, some of the code in src/sysdeps/ is not supposed to be run, but
only compiled and/or linked. It's there to test for feature of the host
system.
> It's purely stylistic thing, so if you as a git master owner think it doesn't
> make sense, I'm fine with it.
Oh, it makes sense, but I don't like this approach. It smells too much of
defensive programming, in which you do things "just to be sure". Well,
"when in doubt, add parentheses" is the wrong approach to me, the right
approach being "when in doubt, RTFM and remove the doubt".
> Well, this one is from me personally, fixing 5/7 and 7/7 I wasn't sure that
> nothing changes 'n' because child_spawn() is not a 10-line function and 'n' is
> not fun to search for. Making it const easily ensures that 'n' is the same 'n'
> in error handler as it was in the beginning of the function.
Oh, OK. I understand now. And you're right, n isn't modified in child_spawn().
Fixes committed, new release ready. Thanks again!
--
Laurent
Received on Fri Mar 13 2015 - 16:02:58 UTC