On 08/22/2013 11:25 AM, Kinkie wrote:
> - clang is very strict on the number of parentheses in conditionals:
> as a way to double-check that the programmer knows what she's doing it
> will accept as valid
> if (foo==bar) {} and if ((foo=bar)) {} , and emit a warning (which
> then -Wall turns into an error) in case of if ((foo==bar)) {} and if
> (foo=bar) {}.
Makes sense.
> This leads to bad results when the condition is a cpp macro, even
> worse when the macro is defined in an external library.
Ouch. That is a clang bug IMHO.
How about making COMPILE_STACK_EMPTY and friends into functions instead
of deleting them?
> - if (!COMPILE_STACK_EMPTY)
> + if (!compile_stack.avail == 0)
That would automatically avoid weird code like the one quoted above and
make code more self-documenting.
> - if (BN_is_zero(serial))
> + if BN_is_zero(serial) // BN_is_zero has its own set of surrounding
> parentheses
Please no. Let's not violate the visual syntax of an if statement. If we
have to accommodate clang bugs, let's write
if (BN_is_zero(serial) == true)
or something like that.
> - clang's static code checker complains with the idiom
> if (strcmp(foo,"") != 0) {}
> I've turned them into if (strlen(foo) != 0) {}
Sounds like an improvement to me.
> - if (strcmp(Called_Address, "") != 0) { /* If the Called Address =
> "" */
> + if (strlen(Called_Address) != 0) { /* If the Called Address = "" */
BTW, that comment seems to say the opposite of what the code does.
HTH,
Alex.