I have an old C library that I have to build on a modern/current system. This library has not been developed for at least 5 or 6 years (perhaps even more). So naturally, building it with a modern C compiler throws a lot of warnings. So I started going through the warnings and fixing the code.
But there is one warning where I really don't know what it means. I'm compiling
with GCC 9.3.0 and -O2 -m32 -Wall in the CFLAGS:
[ 17%] Building C object cu_datastructs/CMakeFiles/cu_datastructs.dir/cu_nlist.c.o
In file included from /usr/include/string.h:519,
from /home/shaoran/projects/cu_utils/remus_installer/cu_utils/cu_datastructs/cu_nlist.c:3:
In function ‘strcpy’,
inlined from ‘cu_nlist_swap’ at /home/shaoran/projects/cu_utils/remus_installer/cu_utils/cu_datastructs/cu_nlist.c:389:2:
/usr/include/bits/string_fortified.h:90:10: warning: ‘__builtin_strcpy’ accessing 1 byte at offsets [-1073741824, 1073741823] and [-1073741824, 1073741823] may overlap 1 byte at offset -1073741824 [-Wrestrict]
90 | return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The code in question is:
int cu_nlist_swap(struct cu_nlist *nl, int id1, int id2)
{
unsigned char *c_tmp;
cu_nlist_names n_tmp;
if(nl == NULL || id1 < 0 || id2 < 0)
return 0;
if(id1 >= nl->size || id2 >= nl->size) /* out of bounds */
return 0;
if(id1 == id2)
return 1;
c_tmp = nl->items[id1];
nl->items[id1] = nl->items[id2];
nl->items[id2] = c_tmp;
strcpy(n_tmp, nl->names[id1]);
strcpy(nl->names[id1], nl->names[id2]); // <--- line with the warning
strcpy(nl->names[id2], n_tmp);
return 1;
}
where cu_nlist is defined as:
// on a different .h file
#define CU_NAMELEN 1024
// ...
#define CU_NLIST_NAME_LEN CU_NAMELEN
#define CU_NLIST_STARTCAP 2
typedef unsigned char* (*create_f)();
typedef void (*free_f)(unsigned char*);
typedef int (*copy_f)(unsigned char*,unsigned char*);
typedef char cu_nlist_names[CU_NLIST_NAME_LEN];
typedef struct cu_nlist {
create_f ocreate;
free_f ofree;
copy_f ocopy;
int cap;
int size;
unsigned char** items; //< holds the items in the named list, initialized with NULL
cu_nlist_names* names; //< holds the names for the items, initialized with NULL
} cu_nlist;
Am I blind? Why is the cu_nlist_swap generating this warning, and what does
that mean? If I remove -O2 then the warning is not issued, I cannot understand
it. What exactly is GCC telling me?
So, the idea of this structure is to have a named list of objects, when you
append a new value, the append function checks the size. If the number of
elements (size) reached the maximum (cap), then we resize the buffers to hold
double the amount of the previous memory. This is done via this function:
static int nlist_resize(struct cu_nlist* nl)
{
void *tmp;
int sz;
if (nl->cap == 0)
sz = CU_NLIST_STARTCAP;
else
sz = 2*nl->cap;
tmp = realloc(nl->items, sz * sizeof(unsigned char*));
if(tmp == NULL)
return 0;
nl->items = tmp;
tmp = realloc(nl->names, sz*sizeof(cu_nlist_names));
if(tmp == NULL)
return 0;
nl->names = tmp;
nl->cap = sz;
return 1;
}
So nl->names points to a memory block of 1024*sz bytes, nl->names[0]
points to the first 1024 bytes, nl->names[1] points to the next 1024 bytes,
etc. So I don't see why there may be memory that overlaps. What am I
missing here?
I know that declaring name as char** name and allocating CU_NLIST_NAME_LEN
bytes for every new name when cu_nlist_append is called would be better, but I
would have to rewrite a lot more of this code and this is something that at the moment I
cannot do. I know this is not pretty code and could be more efficient with the
memory usage, but I didn't write this code, I'm just maintaining it (it's legacy
code from old projects of my company that are not developed anymore). I want to
understand what GCC is warning me about.