Experimental Physics and
Industrial Control System

Michael Davidsaver via Core-talk <core-talk at aps.anl.gov> · Wed, 16 Sep 2020 07:02:56 -0700

On 9/16/20 5:22 AM, Ralph Lange via Core-talk wrote:
> My 2cts...
> 
> I would rather have a safe implementation for copying C strings (taking both the length of source and target as arguments and null-terminating the target) in libCom. Better than having each software module use a different implementation.
> Using strncpy() also produces loads of issues on static code analysis systems, as it is declared "forbidden" on Windows.

Well, yes.  We could have our out bespoke "safe" string copying
function.  This would certainly do away with at least some warnings as
analysis tools would not assign it any special significance.

> Cheers,
> ~Ralph
> 
> 
> On Wed, 16 Sep 2020 at 08:41, Torsten Bögershausen via Core-talk <core-talk at aps.anl.gov <mailto:core-talk at aps.anl.gov>> wrote:
> 
>     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
>     >
> 

Experimental Physics and Industrial Control System

Experimental Physics and
Industrial Control System