Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy()

[Juan Quintela <quintela@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 21/08/2018 08:03, Thomas Huth wrote:
>>>> gcc is not necessarily wrong, as it CAN catch real erroneous uses of 
>>>> strncpy().  It's just that 99% of the time, strncpy() is the WRONG 
>>>> function to use, and so the remaining few cases where it actually does 
>>>> what you want are so rare that you have to consult the manual anyways. 
>>>> If nothing else, the gcc warning is making people avoid strncpy() even 
>>>> where it is safe (which is not a bad thing, in my opinion, because the 
>>>> contract of strncpy() is so counter-intuitive).
>>>>
>>> I am wondering if we should simply add a helper for these special cases
>>> that zeroes the buffer and uses g_strlcpy(), instead of
>>> ignoring/disabling the warning.
>> Yes, a helper function with a proper comment about its purpose is likely
>> the best way to go.
>
> But why use g_strlcpy in the function (which has an off-by-one effect)?
>
> Maybe it could be a qemu_strncpy that is the same as strncpy but returns
> -ERANGE if the source is longer than the buffer that is passed in (but
> not if it fits perfectly without a terminating NUL):
>
>     int qemu_strncpy(const char *d, const char *s, int dsize)
>     {
>         while (*s && dsize) {
>             *d++ = *s++;
>             dsize--;
>         }
>         /* It's okay if D is just past the end of the buffer,
>          * and S is sitting on the terminating NUL.
>          */
>         if (*s) {
>             return -ERANGE;
>         }
>         while (dsize) {
>             *d++ = 0;
              dsize--;  ?????
>         }
>         return 0;
>     }
>
> Then we have the problem of yet another incomplete transition, but at
> least we could convert those that GCC complains about.

I think this is the best approach.  At least we have a semantic that is
"clear", as trivial as:
- we copy a maximum of dsize chars
- source size has to be at maximum dsize
- we pad destination if size of source is smaller than dsize
- destination could be not zero terminated if source is exactly dsize
  length.

And people wonders why I consider C unsuited to handle strings.

Later, Juan.

PD.  I think that adding a strncpy that don't behave like strncpy is a
     bad idea.  What about strlncpy()?  I.e. does the work of both?  As
     I have no clue what the "l" on strlcpy stands for, we can choose
     any other letter.