Re: Is this code safe?

Someone reported an issue on XL mailing list about the following code in
fe-connect.c failing on Power PC platform:

1844 if (getsockopt(conn->sock, SOL_SOCKET, SO_ERROR,
1845 (char *) &optval, &optlen) == -1)
1846 {
1847 appendPQExpBuffer(&conn->errorMessage,
1848 libpq_gettext("could not get socket error status:
%s\n"),
1849 SOCK_STRERROR(SOCK_ERRNO, sebuf,
sizeof(sebuf)));
1850 goto error_return;
1851 }
1852 else if (optval != 0)
1853 {
1854 /*
1855 * When using a nonblocking connect, we will
typically see
1856 * connect failures at this point, so provide a
friendly
1857 * error message.
1858 */
1859 connectFailureMessage(conn, optval);
1860 pqDropConnection(conn);
1861
1862 /*
1863 * If more addresses remain, keep trying, just as
in the
1864 * case where connect() returned failure
immediately.
1865 */
1866 if (conn->addr_cur->ai_next != NULL)
1867 {
1868 conn->addr_cur = conn->addr_cur->ai_next;
1869 conn->status = CONNECTION_NEEDED;
1870 goto keep_going;
1871 }
1872 goto error_return;
1873 }

TBH the reported failure is in another component of XC code which is
borrowed/copied from the above portion. So it could very well be XC
specific issue. But the reporter claims that initialising optval to 0 where
its declared or commenting out "else if" part fixes his problem. I found
that strange because the preceding getsockopt() should have initialised
optval anyways.

Can some kind of compiler optimisation reorder things such that the "else
if" expression is evaluated using the old, uninitialised value of optval?
Or these branches are evaluated sequentially so there is no chance that the
"else if" expression would not see the new value of optval set by
getsockopt()?

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee

Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> writes:
> Can some kind of compiler optimisation reorder things such that the "else
> if" expression is evaluated using the old, uninitialised value of optval?

Any such behavior is patently illegal per the C standard. Not that that's
always stopped compiler writers; but I think you might be looking at a
compiler bug. We'd need more context about the exact platform, compiler,
etc.

regards, tom lane

On Thu, Aug 28, 2014 at 11:20 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> writes:
> > Can some kind of compiler optimisation reorder things such that the "else
> > if" expression is evaluated using the old, uninitialised value of optval?
>
> Any such behavior is patently illegal per the C standard.

Thanks a lot for explaining that.

> Not that that's
> always stopped compiler writers; but I think you might be looking at a
> compiler bug.

I'd requested the reporter to try moving the getsockopt() call outside "if"
expression. That hasn't helped. So I think its a case of getsockopt()
neither returning an error nor setting optval to any sane value. Will try
to get more details about the platform etc.

Thanks,
Pavan

--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee