quote arguments in macros?

[Michael Wojcik]

> From: openssl-users [mailto:openssl-users-bounces at openssl.org] On Behalf Of
> Claus Assmann
> Sent: Tuesday, April 07, 2020 05:13
>
> I just got a compiler warning while modifying some code:
>
> SSL_set_tlsext_host_name(ssl, sni)
> ->
>
> #define IS_EMPTY(s)  (NULL == (s) || '\0' == *(s))
> SSL_set_tlsext_host_name(ssl, !IS_EMPTY(sni) ? sni : other)
>
> warning: cast to 'char *' from smaller integer type 'int'
>
>       'SSL_set_tlsext_host_name'
> SSL_ctrl(s,SSL_CTRL_SET_TLSEXT_HOSTNAME,TLSEXT_NAMETYPE_host_name,(char
> *)name)
>
> ^^^^
>
> Shouldn't the arguments in those macros be quoted, e.g.,
> SSL_ctrl((s),SSL_CTRL_SET_TLSEXT_HOSTNAME,TLSEXT_NAMETYPE_host_name,(char
> *)(name))
> ?

That's parenthesized, not quoted.

Arguably they should be; that's common C practice, for precisely this reason: macro expansion of arguments that are more-complex expressions than simple identifiers may cause issues due to precedence and association.

In this particular case, I don't think that cast should be there at all in the macro definition. What if the actual argument for name is not compatible with char*? In that case the macro hides an error. And since the final formal parameter to SSL_ctrl is actually void*, that cast isn't doing anything useful. Also, ideally, the final formal parameter would be const char*, but that would require a new API because SSL_ctrl overloads the semantics of its final parameter for both set and get operations. (This is an unfortunate old UNIX convention - see e.g. fcntl and ioctl - which predates the introduction of the const type-qualifier to the C language and const-correctness.)

But in either case, the fact is that some of the macro definitions that use SSL_ctrl parenthesize their formal parameters, and some don't. (Typically the ones in ssl.h do, for example, and those in tls1.h don't.) And some have infelicitous casts applied to those parameters, while others don't. (And some are violations of the C standard; for example, it's not guaranteed that function pointers have a representation that's compatible with void*, as e.g. SSL_set_tlsext_debug_callback assumes.)

If someone wants that changed, they'd probably have to open an issue, create a patch, submit a PR, and then be prepared to defend the change. My guess is that this would not be high on the OpenSSL team's priority list.

In the meantime, an easy workaround is to parenthesize it yourself in the macro invoocation. Or avoid using complex expressions as actual parameters to the function calls.

--
Michael Wojcik
Distinguished Engineer, Micro Focus