Re: [PATCH] Improve portability across compilers

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.
Received on Fri Aug 23 2013 - 04:49:49 MDT