valgrind - Address ---- is 0 bytes after a block of size 8 alloc'd

Question

First, I know similar questions have been asked. However, I'd like to have a more general simple question with really primitive C data types. So here it is.

In main.c I call a function to populate those string:

int
main (int argc, char *argv[]){

    char *host = NULL ;
    char *database ;
    char *collection_name;
    char *filename = ""; 
    char *fields = NULL;
    char *query = NULL;
    ...

    get_options(argc, argv, &host, &database, &collection_name, &filename, 
                &fields, &query, &aggregation);

Inside get_options:

if (*filename == NULL ) {
   *filename = (char*)realloc(*filename, strlen(*collection_name)*sizeof(char)+4);
    strcpy(*filename, *collection_name);
    strcat(*filename, ".tde");  # line 69 
}

My program works fine, but then Valgrind tells me I'm doing it wrong:

==8608== Memcheck, a memory error detector
==8608== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==8608== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==8608== Command: ./coll2tde -h localhost -d test -c test
==8608== 
==8608== Invalid write of size 1
==8608==    at 0x403BE2: get_options (coll2tde.c:69)
==8608==    by 0x402213: main (coll2tde.c:92)
==8608==  Address 0xa2edd18 is 0 bytes after a block of size 8 alloc'd
==8608==    at 0x4C28BED: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8608==    by 0x4C28D6F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8608==    by 0x403BBC: get_options (coll2tde.c:67)
==8608==    by 0x402213: main (coll2tde.c:92)

Can you explain the error Address 0xa2edd18 is 0 bytes after a block of size 8 alloc'd? How can I solve this issue?

Answer 1

strcpy adds a null terminator character '\0'. You forgot to allocate space for it:

*filename = (char*)realloc(*filename, strlen(*collection_name)*sizeof(char)+5);

You need to add space for 5 characters: 4 for ".tde" suffix, and one more for the '\0' terminator. Your current code allocates only 4, so the last write is done into the space immediately after the block that you have allocated for the new filename (i.e. 0 bytes after it).

Note: Your code has a common problem - it assigns the results of realloc directly to a pointer being reallocated. This is fine when realloc is successful, but creates a memory leak when it fails. Fixing this error requires storing the result of realloc in a separate variable, and checking it for NULL before assigning the value back to *filename:

char *tmp = (char*)realloc(*filename, strlen(*collection_name)*sizeof(char)+5);
if (tmp != NULL) {
    *filename = tmp;
} else {
    // Do something about the failed allocation
}

Assigning directly to *filename creates a memory leak, because the pointer the *filename has been pointing to below would become overwritten on failure, becoming non-recoverable.