Experimental Physics and
Industrial Control System

I have decided to following Torsten's suggestion of using memcpy() rather than strncpy().  This avoid the compiler warning, though next year the compiler may get smarter and see through this as well.

-        if (outStrings) strncpy(&outStrings[i*MAX_ENUM_STRING_SIZE], inStrings[i], MAX_ENUM_STRING_SIZE);
+        if (outStrings) {
+            size_t len = strlen(inStrings[i]);
+            if (len > MAX_ENUM_STRING_SIZE-1) len = MAX_ENUM_STRING_SIZE-1;
+            memcpy(&outStrings[i*MAX_ENUM_STRING_SIZE], inStrings[i], len);
+            outStrings[i*MAX_ENUM_STRING_SIZE + len] = '\0';
+        }

My fix is a little different from Torsten's, because mine should be safe against inStrings[i] being smaller than MAX_NUM_STRING_SIZE.

Mark


-----Original Message-----
From: Torsten Bögershausen <torsten.bogershausen at ess.eu> 
Sent: Wednesday, September 16, 2020 1:41 AM
To: Mark Rivers <rivers at cars.uchicago.edu>; EPICS core-talk <core-talk at aps.anl.gov>
Subject: Re: Compiler warning question

Hej Mark,
it seams as if new compilers don't like strncpy() any more ?
The problem that strncpy() solves against strcpy() is that we don't everwrite memory.
The problem that strncpy() does not solve is to make sure that there is always a terminating NUL byte.
The BSD world has invented strlcpy() for this.

One alternative would be to simply use memcpy(), but one count less, leaving place for the '\0' byte, and then adding a '\0' at the reserved space.
The following works for me (gcc 8.3 Debian)


diff --git a/asyn/devEpics/devAsynInt32.c b/asyn/devEpics/devAsynInt32.c index 52a1dacd..8669bf63 100644
--- a/asyn/devEpics/devAsynInt32.c
+++ b/asyn/devEpics/devAsynInt32.c
@@ -427,7 +427,14 @@ static void setEnums(char *outStrings, int *outVals, epicsEnum16 *outSeverities,
          if (outSeverities) outSeverities[i] = 0;
      }
      for (i=0; (i<numIn && i<numOut); i++) {
-        if (outStrings) strncpy(&outStrings[i*MAX_ENUM_STRING_SIZE], 
inStrings[i], MAX_ENUM_STRING_SIZE);
+        if (outStrings) {
+            /* memcpy nearly all, leave one byte for '\0' */
+            memcpy(&outStrings[i*MAX_ENUM_STRING_SIZE],
+                   inStrings[i],
+                   MAX_ENUM_STRING_SIZE-1);
+            /* make sure that we have a terminating '\0' */
+            outStrings[i*MAX_ENUM_STRING_SIZE + MAX_ENUM_STRING_SIZE-1]
= '\0';
+        }
          if (outVals) outVals[i] = inVals[i];
          if (outSeverities) outSeverities[i] = inSeverities[i];
      }



On 9/16/20 12:30 AM, Mark Rivers via Core-talk wrote:
> With gcc 8.3.1 I am getting warnings in asyn that I don't get with 
> older versions of gcc (e.g. 4.8.5).
> 
> [epics@viper asyn]$ make -s
> 
> In function 'setEnums.constprop',
> 
>      inlined from 'interruptCallbackEnumBi' at
> ../../asyn/devEpics/devAsynInt32.c:759:5:
> 
> ../../asyn/devEpics/devAsynInt32.c:444:25: warning: 'strncpy' forming 
> offset [27, 51] is out of the bounds [0, 26] [-Warray-bounds]
> 
>           if (outStrings) strncpy(&outStrings[i*MAX_ENUM_STRING_SIZE],
> inStrings[i], MAX_ENUM_STRING_SIZE-1);
> 
>                           
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> 
> In function 'setEnums.constprop',
> 
>      inlined from 'interruptCallbackEnumBo' at
> ../../asyn/devEpics/devAsynInt32.c:773:5:
> 
> ../../asyn/devEpics/devAsynInt32.c:444:25: warning: 'strncpy' forming 
> offset [27, 51] is out of the bounds [0, 26] [-Warray-bounds]
> 
>           if (outStrings) strncpy(&outStrings[i*MAX_ENUM_STRING_SIZE],
> inStrings[i], MAX_ENUM_STRING_SIZE-1);
> 
>                           
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~~~~~~~~~~~~
> 
> These warnings are when writing enum strings to the ONST, TWST, . 
> fields of bo, bi, mbbo, and mbbi records.  The function is passed the 
> address of the ZRST field, and computes the addresses of the other 
> fields based on the knowledge of how large each field is (26 bytes) 
> and that the fields are consecutive.  The compiler now is smart enough 
> to say that the array I passed is actually only 26 bytes and so 
> complains about writing to the other fields.
> 
> Is there a way to fix this via a cast to avoid that warning?
> 
> Mark
> 

References:

Compiler warning question Mark Rivers via Core-talk

Re: Compiler warning question Torsten Bögershausen via Core-talk

Navigate by Date:

Prev: [Bug 1896295] Re: WIN32: IOC scan loops hang mdavidsaver via Core-talk

Next: Re: [Merge] ~freddie-akeroyd/epics-base:fix_win32_monotonic_time into epics-base:7.0 Freddie Akeroyd via Core-talk

Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  <2020>  2021  2022  2023  2024 

Navigate by Thread:

Prev: Re: Compiler warning question Benjamin Franksen via Core-talk

Next: Build failed: EPICS Base 7 base-7.0-87 AppVeyor via Core-talk

Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  2019  <2020>  2021  2022  2023  2024