Re: pgsql: Fix misplaced right paren bugs in pgstatfuncs.c.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Kevin Grittner <kgrittn(at)postgresql(dot)org>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Fix misplaced right paren bugs in pgstatfuncs.c.
Date: 2013-12-29 01:14:57
Message-ID: 29462.1388279697@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Kevin Grittner <kgrittn(at)postgresql(dot)org> writes:
> Fix misplaced right paren bugs in pgstatfuncs.c.
>
> The bug would only show up if the C sockaddr structure contained
> zero in the first byte for a valid address; otherwise it would
> fail to fail, which is probably why it went unnoticed for so long.

Just for the sake of the archives: I think this analysis is wrong.
What we had was equivalent to

if (memcmp(x, y, 0))

The POSIX spec doesn't actually say in so many words what memcmp
should do for zero length, but a reasonable expectation is that
it should return zero ("equal"). So I think really we're getting
constant false here, independently of what's in the sockaddr.

So for example in pg_stat_get_activity(), the test to see if the address
was all-zero would always fail to fire, and it would then try to see what
the address family was --- and unless the platform had used zero for
AF_INET, AF_INET6, or AF_UNIX, it would then fall through to the "Unknown
address type" case, which would set the output columns to null anyway!
Similarly, in pg_stat_get_backend_client_addr and
pg_stat_get_backend_client_port, a zero ss_family field would lead to
returning NULL which was the intended behavior anyhow.

So the reason that it'd gone unnoticed is that there is in fact no
observable bug. It's a good catch anyway.

regards, tom lane

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Eisentraut 2013-12-29 01:55:13 pgsql: Update grammar
Previous Message Peter Eisentraut 2013-12-28 00:52:29 pgsql: Fix whitespace