What is this C idiom (if (1))?
Asked Answered
P

3

9

I noticed a strange idiom in openssl source code, here and repeated below:

if ((in == NULL) && (passwds == NULL)) {
        if (1) {                                    (* <---- HERE *)
#ifndef OPENSSL_NO_UI
            /* build a null-terminated list */
            static char *passwds_static[2] = { NULL, NULL };

            passwds = passwds_static;
            if (in == NULL)
                if (EVP_read_pw_string
                    (passwd_malloc, passwd_malloc_size, "Password: ",
                     !(passed_salt || in_noverify)) != 0)
                    goto end;
            passwds[0] = passwd_malloc;
        } else {
#endif
            BIO_printf(bio_err, "password required\n");
            goto end;
        }
}

It seems that this piece of code is equivalent to:

if ((in == NULL) && (passwds == NULL)) {
#ifndef OPENSSL_NO_UI
        /* build a null-terminated list */
        static char *passwds_static[2] = { NULL, NULL };

        passwds = passwds_static;
        if (in == NULL)
            if (EVP_read_pw_string
                (passwd_malloc, passwd_malloc_size, "Password: ",
                 !(passed_salt || in_noverify)) != 0)
                goto end;
        passwds[0] = passwd_malloc;
#else
        BIO_printf(bio_err, "password required\n");
        goto end;
#endif
}

I ruled out some explanations:

  • it could be to introduce block scope for passwds_static, but the enclosing if would serve a similar purpose
  • it could be a construct that through several meaningful transformations becomes meaningless, but that construct is there since the introduction of OPENSSL_NO_UI.

Am I missing something here? What are the advantages of this if (1)? Is this used in other code bases?

Thanks!

Pruter answered 28/3, 2017 at 8:27 Comment(11)
It seems to me that the sole purpose of it is to avoid and #else clause for whatever reasonZabrze
Dup of #2266560?She
@She - No. This isn't about #if 1, since that construct doesn't appear at all in the question. It's about if(1).Zabrze
Indeed, it's not about either #if 0, #if 1, or if (0).Tarrant
The only discernible difference is that using the prerprocessor alone won't produce a warning about unreachable code. But I highly doubt that was the intent.Zabrze
On the warning side, this codes compiles cleanly, except that when OPENSSL_NO_UI is not defined, in_noverify is reported as not used. That happens both in the if (1) and #else cases so it does not seem to be the reason.Tarrant
Perhaps the intention is relevant to unit test purposes. First solution (with if(1)) gives a way to test the entire function which several branches. However, the second solution might require configuration changes to the test engine which in many cases is complicated. Just to summarize my point of view, the approach is good to focus unit testing on code level, but not on configuration level.Impostor
@StoryTeller Ah, yes, I missed the # (and the indent). But I imagine it might serve the same purpose, or considering that in older OpenSSL versions (including 1.1.0-pre*) only what's inside if (1) {..} is present, maybe they will want to change the default behavior in the future (wo needing to recompile w/wo OPENSSL_NO_UI).She
@JeremyP, ahahaha, looking to the coding style I assume you are right ))Impostor
@Impostor The comment is intended to be light hearted but it does have an element of truth. OpenSSL is partly responsible for some of the worst bugs to affect Linux security and a lot of its problems can be blamed on the appalling unmaintainable code base. See this article to get a flavour of the problems.Brinkman
Judging from numerous SO questions about all kinds of oddities in the OpenSSL source, it does appear to be a big pile of crap indeed.Ogburn
P
10

After looking at other similar places, I found an explanation:

if (1) { /* This is a trick we use to avoid bit rot.
          * at least the "else" part will always be
          * compiled.
          */
#ifdef AF_INET6
    family = AF_INET6;
} else {
#endif
    BIOerr(BIO_F_ACPT_STATE, BIO_R_UNAVAILABLE_IP_FAMILY);
    goto exit_loop;
}

In most cases (including their CI I guess), OPENSSL_NO_UI is not defined, so both branches are compiled. If the API one of the branches uses changes, it will be spotted by the compiler, and it can be fixed, without having to test all the compile-time switches.

Pruter answered 28/3, 2017 at 9:1 Comment(0)
O
2

It is apparently used to remove code that shouldn't be executed, similar to using #ifdef compiler switches. Most often strange things like this is an indication of poor version control more than anything else.

Please note that this is not recommended practice - there should be no source code present that never gets executed in no circumstances. It is better practice to use version control, or if that's not possible use compiler switches, or if that's not possible then "comment out" the code.

Ogburn answered 28/3, 2017 at 9:42 Comment(0)
T
0

the statement:

if(1) {

is to always have a matching opening brace for the closing brace.

Thirtytwomo answered 28/3, 2017 at 15:47 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.