On Fri, Nov 15, 2013 at 12:33 PM, Tomas Vondra <t...@fuzzy.cz> wrote:
> > It is likely far better explained here --> > > http://www.courtesan.com/todd/papers/strlcpy.html > > > > For example , the following 2 lines in jsonfuncs.c > > > > memset(name, 0, NAMEDATALEN); > > strncpy(name, fname, NAMEDATALEN); > > 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. Either people do that because of habit / > copy'n'paste, or maybe there are supported platforms when strncpy does not > behave like this for some reason. > > I had not thought of the fact the some platforms don't properly implement strncpy(). On quick check http://man.he.net/man3/strncpy seems to indicate that this behaviour is part of the C89 standard. So does this mean we can always assume that all supported platforms always 0 out the remaining buffer? > I seriously doubt this inefficiency is going to be measurable in real > world. If the result was a buffer-overflow bug, that'd be a different > story, but maybe we could check the ~120 calls to strncpy in the whole > code base and replace it with strlcpy where appropriate. > > The example was more of a demonstration of wrong assumption rather than wasted cycles. Though the wasted cycles was on my mind a bit too. I was more focused on trying to draw a bit of attention to commit 061b88c732952c59741374806e1e41c1ec845d50 which uses strncpy and does not properly set the last byte to 0 afterwards. I think this case could just be replaced with strlcpy which does all this hard work for us. Regards David Rowley > That being said, thanks for looking into things like this. > > Tomas > >