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.
