On Sat, Nov 16, 2013 at 12:53:10PM +1300, David Rowley wrote: > 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.
+1 for the goal of this patch. Another commit took care of your jsonfuncs.c concerns, and the patch for CVE-2014-0065 fixed several of the others. Plenty remain, though. > 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. I suggest preparing one or more patches that focus on the cosmetic-only changes, such as strncpy() -> memcpy() when strncpy() is guaranteed not to reach a NUL byte. With that noise out of the way, it will be easier to give the rest the attention it deserves. Thanks, nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
