gcc-8 Wstringop-truncation

Question

I'm trying to fix some C code where gcc-8 complains about Wstringop-truncation (code is here)

When compiling that code on a server which I can not control neither can add pragma statements nor can disable Wstringop-truncation diagnostics, the warning which I receive is:

gcc-8  -I"/home/hornik/tmp/R/include" -DNDEBUG -I./cqdb/include -I./crf/src -I./liblbfgs/include -I./include -I"/home/hornik/lib/R/Library/3.6/x86_64-linux-gnu/Rcpp/include" -I/usr/local/include   -fpic  -g -O2 -Wall -pedantic -mtune=native -c cqdb/src/cqdb.c -o cqdb/src/cqdb.o
cqdb/src/cqdb.c: In function ‘cqdb_writer_close’:
cqdb/src/cqdb.c:270:5: warning: ‘strncpy’ output truncated before terminating nul copying 4 bytes from a string of the same length [-Wstringop-truncation]
     strncpy((char*)header.chunkid, CHUNKID, 4);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cqdb/src/cqdb.c: In function ‘cqdb_reader’:
cqdb/src/cqdb.c:469:9: warning: ‘strncpy’ specified bound 4 equals destination size [-Wstringop-truncation]
         strncpy((char*)db->header.chunkid, (const char*)p, 4);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I would like to rewrite the strncpy statements to remove these warnings. Am I right that I need to replace in the following lines

strncpy((char*)header.chunkid, CHUNKID, 4);
with strncpy((char*)header.chunkid, CHUNKID, 5);

and strncpy((char*)db->header.chunkid, (const char*)p, 4);
with strncpy((char*)db->header.chunkid, (const char*)p, 5);

The relevant code in cqdb.c is put below. It basically checks if the file is of type 'CQDB'. Mark that although I would really love to have access, I do not have access to this machine so I can not test out if the fixes to the C code will work.

#define CHUNKID             "CQDB"

typedef struct {
    int8_t      chunkid[4]; /**< Chunk identifier, "CQDB". */
    uint32_t    size;       /**< Chunk size including this header. */
    uint32_t    flag;       /**< Global flags. */
    uint32_t    byteorder;  /**< Byte-order indicator. */
    uint32_t    bwd_size;   /**< Number of elements in the backward array. */
    uint32_t    bwd_offset; /**< Offset to the backward array. */
} header_t;

int cqdb_writer_close(cqdb_writer_t* dbw)
{
header_t header;
strncpy((char*)header.chunkid, CHUNKID, 4);
...
}

cqdb_t* cqdb_reader(const void *buffer, size_t size)
{
    cqdb_t* db = NULL;
    /* Check the file chunkid. */
    if (memcmp(buffer, CHUNKID, 4) != 0) {
        return NULL;
    }
    db = (cqdb_t*)calloc(1, sizeof(cqdb_t));
    const uint8_t* p = NULL;
    db->buffer = buffer;
    p = db->buffer;
    strncpy((char*)db->header.chunkid, (const char*)p, 4);
...
}

I'm not allowed to add pragma's, it's an R package wrapping C++ code, the CRAN policy distributing R packages disallows setting specific pragma's. Thank you for the memcpy suggestion, I'll look into the docs of that. — , Sep 10 '18 at 20:46
You definitely do not want to try copying 5 bytes into a 4-byte buffer as you suggest — M.M, Sep 10 '18 at 21:12
Are you writing C or C++? You've tagged the question C++, but the code you posted is all C code. — Miles Budnek, Sep 10 '18 at 21:21
"I would like to rewrite the strncpy statements to remove these warnings" (followed by subtly but tragically breaking changes) - [surely gives me some deja vu...](https://research.swtch.com/openssl) Please, do not _ever_ try to "fix" warnings without fully understanding what you are doing; you risk introducing way worse problems than some garbage in console when compiling. — Matteo Italia, Sep 10 '18 at 21:44
Use `memmove()` or `memcpy()` instead of `strncpy()` when you definitely won't copy the null byte from the source string. It might even be faster, but it fixes the warnings. It's basically what M.M suggests in their answer. — Jonathan Leffler, Sep 10 '18 at 22:51
@JonathanLeffler if the code needs to cope with strings that might be shorter than 4 then you'd need to take care — M.M, Sep 10 '18 at 23:28
@M.M — you always need to take care. :D . You're right, but the compile won't gripe if the literal )including the null terminator) is not longer than the length specified. That is, the compiler won't generate a warning for the case where extra care must be taken. — Jonathan Leffler, Sep 10 '18 at 23:38

M.M · Accepted Answer · 2018-09-10T22:13:23.367

The usage of strncpy in the question is actually correct (left-justifying some characters in a buffer, right-padding with null bytes), but the warning is because this function is often misused by people trying to copy a null-terminated string.

For the code shown in the question I would replace the strncpy calls with:

set_chunkid(&header);

where you add a new function:

void set_chunkid(header_t *hdr)
{
    _Static_assert(sizeof CHUNKID == sizeof hdr->chunkid + 1, "chunk ID not 4 chars");

    memcpy(&hdr->chunkid, CHUNKID, sizeof hdr->chunkid);
}

If there are other use cases that this function doesn't cover then update the question.

Thank you very much for the suggestion, will try it out. – Sep 10 '18 at 21:52 — , Sep 10 '18 at 21:52

gcc-8 Wstringop-truncation

1 Answers1