Re: strncpy is not a safe version of strcpy

On 15 Listopad 2013, 0:07, David Rowley wrote:
> Hi All,
>
> As a bit of a background task, over the past few days I've been analysing
> the uses of strncpy in the code just to try and validate if it is the
> right
> function to be using. I've already seen quite a few places where their
> usage is wrongly assumed.
>
> As many of you will know and maybe some of you have forgotten that strncpy
> is not a safe version of strcpy. It is also quite an inefficient way to
> copy a string to another buffer as strncpy will 0 out any space that
> happens to remain in the buffer. If there is no space left after the copy
> then the buffer won't end with a 0.
>
> 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 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.

That being said, thanks for looking into things like this.

Tomas