> On 2 Oct 2020, at 11:44, Jürgen Groß <jgr...@suse.com> wrote:
>
> On 02.10.20 12:12, Bertrand Marquis wrote:
>> Hi,
>>> On 2 Oct 2020, at 08:25, Jürgen Groß <jgr...@suse.com> wrote:
>>>
>>> On 02.10.20 08:59, Jan Beulich wrote:
>>>> On 02.10.2020 08:51, Jürgen Groß wrote:
>>>>> On 02.10.20 08:20, Jan Beulich wrote:
>>>>>> On 02.10.2020 06:50, Jürgen Groß wrote:
>>>>>>> On 01.10.20 18:38, Bertrand Marquis wrote:
>>>>>>>> Hi Juergen,
>>>>>>>>
>>>>>>>>> On 14 Sep 2020, at 11:58, Bertrand Marquis <bertrand.marq...@arm.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On 12 Sep 2020, at 14:08, Juergen Gross <jgr...@suse.com> wrote:
>>>>>>>>>>
>>>>>>>>>> Making getBridge() static triggered a build error with some gcc
>>>>>>>>>> versions:
>>>>>>>>>>
>>>>>>>>>> error: 'strncpy' output may be truncated copying 15 bytes from a
>>>>>>>>>> string of
>>>>>>>>>> length 255 [-Werror=stringop-truncation]
>>>>>>>>>>
>>>>>>>>>> Fix that by using a buffer with 256 bytes instead.
>>>>>>>>>>
>>>>>>>>>> Fixes: 6d0ec053907794 ("tools: split libxenstat into new
>>>>>>>>>> tools/libs/stat directory")
>>>>>>>>>> Signed-off-by: Juergen Gross <jgr...@suse.com>
>>>>>>>>> Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com>
>>>>>>>>
>>>>>>>> Sorry i have to come back on this one.
>>>>>>>>
>>>>>>>> I still see an error compiling with Yocto on this one:
>>>>>>>> | inlined from 'xenstat_collect_networks' at xenstat_linux.c:306:2:
>>>>>>>> | xenstat_linux.c:81:6: error: 'strncpy' output may be truncated
>>>>>>>> copying 255 bytes from a string of length 255
>>>>>>>> [-Werror=stringop-truncation]
>>>>>>>> | 81 | strncpy(result, de->d_name, resultLen);
>>>>>>>> | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>>>>
>>>>>>>> To solve it, I need to define devBridge[257] as devNoBrideg.
>>>>>>>
>>>>>>> IMHO this is a real compiler error.
>>>>>>>
>>>>>>> de->d_name is an array of 256 bytes, so doing strncpy() from that to
>>>>>>> another array of 256 bytes with a length of 256 won't truncate anything.
>>>>>>
>>>>>> That's a matter of how you look at it, I think: If the original array
>>>>>> doesn't hold a nul-terminated string, the destination array won't
>>>>>> either, yet the common goal of strncpy() is to yield a properly nul-
>>>>>> terminated string. IOW the warning may be since the standard even has
>>>>>> a specific foot note to point out this possible pitfall.
>>>>>
>>>>> If the source doesn't hold a nul-terminated string there will still be
>>>>> 256 bytes copied, so there is no truncation done during strncpy().
>>>>>
>>>>> In fact there is no way to use strncpy() in a safe way on a fixed sized
>>>>> source array with the above semantics: either the target is larger than
>>>>> the source and length is at least sizeof(source) + 1, resulting in a
>>>>> possible read beyond the end of source, or the target is the same length
>>>>> leading to the error.
>>>> I agree with all of what you say, but I can also see why said foot note
>>>> alone may have motivated the emission of the warning.
>>>
>>> The motivation can be explained, yes, but it is wrong. strncpy() is not
>>> limited to source arrays of unknown length. So this warning is making
>>> strncpy() unusable for fixed sized source strings and -Werror. And that
>>> is nothing a compiler should be allowed to do, hence a compiler bug.
>> I do agree that in this case the compiler is doing to much.
>
> It is plain wrong here. Rendering a Posix defined function unusable for
> a completely legal use case is in no way a matter of taste or of "doing
> to much". It is a bug.
Agree.
>
>> We could also choose to turn off the warning either using pragma (which
>> i really do not like) or by adding a cflag for this specific file (but this
>> might
>> hit us later in other places).
>> All in all this currently makes Xen master and staging not possible to
>> compile with Yocto so we need to find a solution as this will also
>> come in any distribution using a new compiler,
>
> A variant you didn't mention would be open coding of strncpy() (or
> having a related inline function in a common header). This route would
> be the one I'd prefer in case the compiler guys insist on the behavior
> being fine.
True this possible, even though I do not like to modify the code that deeply
for one specific compiler.
>
> You didn't tell us which compiler is being used and whether it really is
> up to date. A workaround might be to set EXTRA_CFLAGS_XEN_TOOLS to
> "-Wno-stringop-truncation" for the build.
That’s what i meant by “adding a cflag”, as you suggest we could also
make it global (and not limit it to this file).
The compiler I am using is the one from Yocto Gatesgarth (release coming in
october): gcc version 10.2.0 (released in july 2020).
Cheers
Bertrand
