CODEWITHSUNDEEP
cvalgrindstrstr
Paul
Paul

Reputation: 21985

Invalid read of size 1 in strstr() using valgrind

I'm inspecting some piece of code with valgrind and I get this error:

==7001== Invalid read of size 1
==7001==    at 0x402E21B: strstr (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==7001==    by 0x8049742: replace_in_file (functions.c:191)
==7001==    by 0x8049C55: parse_dir (functions.c:295)
==7001==    by 0x8049059: main (main.c:214)
==7001==  Address 0x42018c3 is 0 bytes after a block of size 3 alloc'd
==7001==    at 0x402B018: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==7001==    by 0x80496EC: replace_in_file (functions.c:183)
==7001==    by 0x8049C55: parse_dir (functions.c:295)
==7001==    by 0x8049059: main (main.c:214)

The code snippet looks as follows:

long int replace_in_file(char **sets, FILE *file, char *path, const char *mode){
    long int n = 0, size = 0, len, remaining_len;

    char *buffer,
         *found,
         *before,
         *after;

    size = file_size(file);
    if(-1 != size){

        buffer = malloc(size * sizeof(char));

        rewind(file);
        fread(buffer, 1, size, file);

        int i=0;
        do{
            do{
                found = strstr(buffer, sets[i]); // LINE 191
                if(NULL != found && '\0' != sets[i][0]){

//rest of code...

I cannot realise why I get that error, because everything works as expected and in the debugger every variable seems fine.

What is wrong, how can I fix it?

Upvotes: 3

Views: 4239

Answers (3)

Yefim Dinitz
Yefim Dinitz

Reputation: 424

from what I read in my manpage, fread() does not NULL-terminate its result according to C89 - and it does not on my system, either - so strstr() will simply read over the end of buffer unless you terminate it e.g. like so

buffer = malloc((size + 1) * sizeof(char));
memset(buffer, '\0', size + 1);

(which has the advantage of also terminating it in case fread fails to read the whole file)

a sidenote: since you already use sizeof(char) in your malloc() call, you might also consider replacing the 1 in the fread() for sake of consistency.

Upvotes: 2

chrisaycock
chrisaycock

Reputation: 37928

Your buffer is not nul-terminated. Usually, you can just bzero() it before the fread() to ensure that it will work with strstr() and similar functions.

Upvotes: 4

FatalError
FatalError

Reputation: 54591

fread() does not nul-terminate the data it reads, but strstr() expects a nul-terminated string. You should allocate size+1 bytes and explicitly add the nul-terminator to be safe.

Upvotes: 6

Related Questions