Hi Jurgen, > On 7 Oct 2020, at 09:39, Jürgen Groß <[email protected]> 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 <[email protected]> >> --- >> 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 This would cover the case where NAME_MAX is shorter then resultLen. Cheers Bertrand > > > Juergen
