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.

Reply via email to