Hacker News Reader

Comment by Someone | Hacker News Reader

The assignment in the while boolean condition sets per to NULL at the end of the loop.

To be fair, I think that the given block could be written better both in terms of clarity and in terms of knocking out unneeded functions.

    s = fd == -1 ? socket(ss->ss_family, SOCK_STREAM, IPPROTO_TCP) : fd;
    if (s == -1)
        goto bad;

Is the equivalent of

    if (fd == -1)
    {
        s = -1;
        goto bad;
    }
    else
    {
        s = socket(ss->ss_family, SOCK_STREAM, IPPROTO_TCP);
    }

but with one less comparison, assignment of a constant to s in the error condition instead of a variable, which is usually faster, and a more clear, explicit, layout. Now, the compiler may make these optimizations for you, but it still leaves an uglier bit of code. Tristate operators can be useful, but in some cases, like this, they can make the code less efficient.

Your refactor is wrong. You've reversed the ternary and removed the socket() return check. The consolation prize is that you've reinforced the issue of lack of clarity in the code.

You're right. I was thrown off by them putting the exceptional condition as the first part of the ternary operation. Chalk this to overly clever code.

    if (fd != -1)
    {
        s = fd;
    }
    else
    {
        s = socket(ss->ss_family, SOCK_STREAM, IPPROTO_TCP);
        if (s == -1)
        {
            goto bad;
        }
    }

or in rewriting the ternary function for clarity: // If we have a socket, use it, otherwise, try to get one s = (fd != -1) ? fd : socket(ss->ss_family, SOCK_STREAM, IPPROTO_TCP); if ( s == -1 ) // we still don't have a socket, error out goto bad;

Ternary operators should, in my opinion, be put to the 3 am test. If you think you'd get confused at looking at code that uses one at 3 am, then you've not written it properly and should clarify.

Yes, you are right. It should not take such close reading of the code though to make sure that this condition can not occur.

Since you have such valuable insights, perhaps you should discuss them on openbsd-tech.

Reply on news.ycombinator.com