> On 7 Oct 2020, at 10:14, Jürgen Groß <jgr...@suse.com> wrote:
>
> On 07.10.20 10:56, Bertrand Marquis wrote:
>> Hi Jurgen,
>>> On 7 Oct 2020, at 09:39, Jürgen Groß <jgr...@suse.com> wrote:
>>>
>>> On 07.10.20 10:28, Bertrand Marquis wrote:
>>>> Use memcpy in getBridge to prevent gcc warnings about truncated
>>>> strings. We know that we might truncate it, so the gcc warning
>>>> here is wrong.
>>>> Revert previous change changing buffer sizes as bigger buffers
>>>> are not needed.
>>>> Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com>
>>>> ---
>>>> Changes in v2:
>>>> Use MIN between string length of de->d_name and resultLen to copy only
>>>> the minimum size required and prevent crossing to from an unallocated
>>>> space.
>>>> ---
>>>> tools/libs/stat/xenstat_linux.c | 11 +++++++++--
>>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>> diff --git a/tools/libs/stat/xenstat_linux.c
>>>> b/tools/libs/stat/xenstat_linux.c
>>>> index d2ee6fda64..0ace03af1b 100644
>>>> --- a/tools/libs/stat/xenstat_linux.c
>>>> +++ b/tools/libs/stat/xenstat_linux.c
>>>> @@ -29,6 +29,7 @@
>>>> #include <string.h>
>>>> #include <unistd.h>
>>>> #include <regex.h>
>>>> +#include <xen-tools/libs.h>
>>>> #include "xenstat_priv.h"
>>>> @@ -78,7 +79,13 @@ static void getBridge(char *excludeName, char *result,
>>>> size_t resultLen)
>>>> sprintf(tmp, "/sys/class/net/%s/bridge",
>>>> de->d_name);
>>>> if (access(tmp, F_OK) == 0) {
>>>> - strncpy(result, de->d_name, resultLen);
>>>> + /*
>>>> + * Do not use strncpy to prevent
>>>> compiler warning with
>>>> + * gcc >= 10.0
>>>> + * If de->d_name is longer then
>>>> resultLen we truncate it
>>>
>>> s/then/than/
>> Will fix
>>>
>>>> + */
>>>> + memcpy(result, de->d_name,
>>>> MIN(strnlen(de->d_name,
>>>> +
>>>> sizeof(de->d_name)),resultLen - 1));
>>>
>>> You can't use sizeof(de->d_name) here, as AFAIK there is no guarantee
>>> that de->d_name isn't e.g. defined like "char d_name[]".
>>>
>>> My suggestion to use NAME_MAX as upper boundary for the length was
>>> really meant to be used that way.
>>>
>>> And additionally you might want to add 1 to the strnlen() result in
>>> order to copy the trailing 0-byte, too (or you should zero out the
>>> result buffer before and omit writing the final zero byte).
>>>
>>> Thinking more about it zeroing the result buffer is better as it even
>>> covers the theoretical case of NAME_MAX being shorter than resultLen.
>> Setting the result buffer completely to 0 and doing after a copy sounds like
>> a big complexity.
>> How about:
>> copysize = MIN(strnlen(de->d_name,NAME_MAX), resultLen - 1);
>> memcpy(result, de->d_name, copysize);
>> result[copysize + 1] = 0
>
> result[copysize] = 0;
>
>> This would cover the case where NAME_MAX is shorter then resultLen.
>
> Why is:
>
> memset(result, 0, resultLen);
> memcpy(result, de->d_name, MIN(strnlen(de->d_name,NAME_MAX), resultLen - 1));
>
> A big complexity?
it is potentially more computation (complexity was not the right word maybe).
>
> In the end both variants are fine.
in the end I am fully ok with any, at then end there is no performance issue
here.
I will use use the memset solution and push a v3.
Cheers
Bertrand
>
>
> Juergen
