Re: strncmp->memcmp when we know the shorter length
Robert Haas <robertmhaas@gmail.com>
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Gurjeet Singh <singh.gurjeet@gmail.com>, Noah Misch <noah@leadboat.com>, pgsql-hackers@postgresql.org
Date: 2010-12-22T03:48:07Z
Lists: pgsql-hackers
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