On Sat, Nov 16, 2013 at 4:09 AM, Alvaro Herrera <alvhe...@2ndquadrant.com>wrote:
> David Rowley escribió: > > On Fri, Nov 15, 2013 at 12:33 PM, Tomas Vondra <t...@fuzzy.cz> wrote: > > > > Be careful with 'Name' data type - it's not just a simple string > buffer. > > > AFAIK it needs to work with hashing etc. so the zeroing is actually > needed > > > here to make sure two values produce the same result. At least that's > how > > > I understand the code after a quick check - for example this is from > the > > > same jsonfuncs.c you mentioned: > > > > > > memset(fname, 0, NAMEDATALEN); > > > strncpy(fname, NameStr(tupdesc->attrs[i]->attname), NAMEDATALEN); > > > hashentry = hash_search(json_hash, fname, HASH_FIND, NULL); > > > > > > So the zeroing is on purpose, although if strncpy does that then the > > > memset is probably superflous. > > This code should probably be using namecpy(). Note namecpy() doesn't > memset() after strncpy() and has survived the test of time, which > strongly suggests that the memset is indeed superfluous. > > I went on a bit of a strncpy cleanup rampage this morning and ended up finding quite a few places where strncpy is used wrongly. I'm not quite sure if I have got them all in this patch, but I' think I've got the obvious ones at least. For the hash_search in jsconfuncs.c after thinking about it a bit more... Can we not just pass the attname without making a copy of it? I see keyPtr in hash_search is const void * so it shouldn't get modified in there. I can't quite see the reason for making the copy. Attached is a patch with various cleanups where I didn't like the look of the strncpy. I didn't go overboard with this as I know making this sort of small changes all over can be a bit scary and I thought maybe it would get rejected on that basis. I also cleaned up things like strncpy(dest, src, strlen(src)); which just seems a bit weird and I'm failing to get my head around why it was done. I replaced these with memcpy instead, but they could perhaps be a plain old strcpy. Regards David Rowley > -- > Álvaro Herrera http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training & Services >
strncpy_cleanup_v0.1.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
