[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by
From: |
Thomas Huth |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy() |
Date: |
Tue, 21 Aug 2018 08:03:30 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 2018-08-20 21:59, David Hildenbrand wrote:
> On 20.08.2018 21:48, Eric Blake wrote:
>> On 08/20/2018 12:16 PM, Thomas Huth wrote:
>>
>>>>
>>>> Maybe really set it to zero (memset) before using the g_strlcpy? I am
>>>> not a fan of disabling warnings, but if you think this is
>>>> easier/cleaner, let's go for that.
>>
>> I'm not a fan of strlcpy in general (by the time you've properly set it
>> up to detect/report/avoid truncation errors, you've added more
>> boilerplate code than you would have by just doing memcpy() yourself).
>>
>>>
>>> FWIW, that new warning from GCC is IMHO just annoying. I had the same
>>> problem in the SLOF sources, too:
>>>
>>> https://github.com/aik/SLOF/commit/d8a9354c2a35136
>>>
>>> The code with strncpy was perfectly valid before, but to get rid of the
>>> warning, I replaced it with a more cumbersome memcpy instead (and mad
>>> sure that the memory is already cleared earlier in that function). Now
>>> seeing that the problem with strncpy pops up here, too, I think it would
>>> maybe be better to shut up the warning of GCC, since it's clearly GCC
>>> who's wrong here.
>>
>> 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.
Thomas
- [Qemu-trivial] [PATCH] migration: Replace strncpy() by g_strlcpy(), Philippe Mathieu-Daudé, 2018/08/17
- Re: [Qemu-trivial] [PATCH] migration: Replace strncpy() by g_strlcpy(), Juan Quintela, 2018/08/20
- Re: [Qemu-trivial] [PATCH] migration: Replace strncpy() by g_strlcpy(), David Hildenbrand, 2018/08/20
- Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Paolo Bonzini, 2018/08/20
- Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), David Hildenbrand, 2018/08/20
- Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Thomas Huth, 2018/08/20
- Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Eric Blake, 2018/08/20
- Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), David Hildenbrand, 2018/08/20
- Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(),
Thomas Huth <=
- Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: Replace strncpy() by g_strlcpy(), Paolo Bonzini, 2018/08/21
- Re: [Qemu-trivial] [PATCH] migration: Replace strncpy() by g_strlcpy(), Juan Quintela, 2018/08/21