On Tue, Dec 21, 2010 at 10:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> If it's done properly, I don't see how this would be a risk.
>
> I'm fairly uncomfortable about the broad swath and low return of this
> patch. Noah is assuming that none of these places are relying on
> strncmp to stop short upon finding a null, and I don't believe that
> that's a safe assumption in every single place. Nor do I believe that
> it's worth the effort of trying to prove it safe in most of those
> places.
>
> I think this might be a good idea in the varchar.c and varlena.c calls,
> but I'd be inclined to leave the rest of the calls alone.
Eh, I already committed somewhat more than that. I did think about
the concern which you raise. It seems pretty clear that's not a
danger in readfuncs.c. In the hstore and ltree cases, at least at
first blush, it appears to me that it would be downright broken for
someone to be counting on a null to terminate the comparison. The
intent of these bits of code appears to be to do equality comparison a
string stored as a byte count + a byte string, rather than a
null-terminated cstring, so unless I'm misunderstanding something it's
more likely that the use of strncmp() would lead to a bug; the prior
coding doesn't look like it would be correct if NUL bytes were
possible. The tsearch cases also appear to be safe in this regard,
but since I decided against committing those on other grounds I
haven't looked at them as carefully.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company