[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy()
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy() |
Date: |
Tue, 21 Aug 2018 13:52:33 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Paolo Bonzini <address@hidden> 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.
- [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Philippe Mathieu-Daudé, 2018/08/17
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Juan Quintela, 2018/08/20
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), David Hildenbrand, 2018/08/20
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Paolo Bonzini, 2018/08/20
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), David Hildenbrand, 2018/08/20
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Thomas Huth, 2018/08/20
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Eric Blake, 2018/08/20
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), David Hildenbrand, 2018/08/20
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Thomas Huth, 2018/08/21
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Paolo Bonzini, 2018/08/21
- Re: [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(),
Juan Quintela <=