Valgrind - Invalid read of size 1 in a "readline" function implemented in C

Question

I've the following code where basically I implemented my own read-line function for exercising me in memory allocation, etc, in C. Before I asked a question, but no one actually helped in trying to correct my code eventually except for suggesting to use valgrind. Since I'd never used it before, it's quite hard for me to understand everything.

My code is the following:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>

/**
Gets and a variable-size line from the standard input.
*/
char* readline(){

    size_t n = 10;
    char* final = calloc(n, sizeof(char));
    final[0] = '\0';
    char* tmp; // used for allocating memory temporarily

    // constant buffer size used to store the read characters
    // before storing them in the final buffer
    char buf[10]; 

    while(fgets(buf, 10, stdin) != NULL) {

        if(buf[strlen(buf) - 1] == '\n') {

            if(strlen(buf) > 1) {

                if((n - strlen(final)) < (strlen(buf) + 1)) {
                    // -1 because buf contains also \n at the end
                    n = strlen(final) + strlen(buf);
                    tmp = calloc(n, sizeof(char));

                    for(int i=0; i <= strlen(final); ++i)
                        tmp[i] = final[i];

                    free(final);
                } else {
                    tmp = final;
                }

                int i, j;
                for(i = strlen(final), j = 0; j <= (strlen(buf) - 2); ++i, ++j)
                    tmp[i] = buf[j];

                tmp[i] = '\0';

                final = tmp;
                tmp = NULL;
            }

            break;

        } else { // no newline inserted at the end

            if((n - strlen(final)) < (strlen(buf) + 1)) {
                n *= 2;
                tmp = calloc(n, sizeof(char));

                for(int i = 0; i <= strlen(final); ++i)
                    tmp[i] = final[i];

                free(final);

            } else {
                tmp = final;
            }       

            // Starts inserting from the '\0' char
            // Insert also the '\0' at the end
            for(int i = strlen(tmp), j = 0; j <= 9; ++i, ++j)
                tmp[i] = buf[j];

            final = tmp;
            tmp = NULL;
        }
    }

    return final;
}



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

    if(argc < 2){
        fprintf(stderr, "usage: at least one string as command-line argument.\n");
        exit(1);
    } else {
        char* line = readline();
        printf("line = %s\n", line);
        printf("size = %lu\n", strlen(line));
        free(line);
    }

    return 0;
}

When I run valgrind with the command:

valgrind ./findword hello

I get the following ouput

==14084== Memcheck, a memory error detector
==14084== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==14084== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==14084== Command: ./findword hello
==14084== 
hello world, how are you?
==14084== Invalid read of size 1
==14084==    at 0x10000A669: strlen (in /usr/local/Cellar/valgrind/HEAD/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==14084==    by 0x100000C19: readline (findword.c:46)
==14084==    by 0x100000E6C: main (findword.c:93)
==14084==  Address 0x100a78740 is 0 bytes inside a block of size 20 free'd
==14084==    at 0x10000927F: free (in /usr/local/Cellar/valgrind/HEAD/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==14084==    by 0x100000C03: readline (findword.c:40)
==14084==    by 0x100000E6C: main (findword.c:93)
==14084==  Block was alloc'd at
==14084==    at 0x100009541: calloc (in /usr/local/Cellar/valgrind/HEAD/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==14084==    by 0x100000D0F: readline (findword.c:61)
==14084==    by 0x100000E6C: main (findword.c:93)
==14084== 
==14084== Invalid read of size 1
==14084==    at 0x10000A672: strlen (in /usr/local/Cellar/valgrind/HEAD/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==14084==    by 0x100000C19: readline (findword.c:46)
==14084==    by 0x100000E6C: main (findword.c:93)
==14084==  Address 0x100a78742 is 2 bytes inside a block of size 20 free'd
==14084==    at 0x10000927F: free (in /usr/local/Cellar/valgrind/HEAD/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==14084==    by 0x100000C03: readline (findword.c:40)
==14084==    by 0x100000E6C: main (findword.c:93)
==14084==  Block was alloc'd at
==14084==    at 0x100009541: calloc (in /usr/local/Cellar/valgrind/HEAD/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==14084==    by 0x100000D0F: readline (findword.c:61)
==14084==    by 0x100000E6C: main (findword.c:93)
==14084== 
line = hello world, how are you?
size = 25
==14084== 
==14084== HEAP SUMMARY:
==14084==     in use at exit: 30,666 bytes in 189 blocks
==14084==   total heap usage: 276 allocs, 87 frees, 36,962 bytes allocated
==14084== 
==14084== LEAK SUMMARY:
==14084==    definitely lost: 0 bytes in 0 blocks
==14084==    indirectly lost: 0 bytes in 0 blocks
==14084==      possibly lost: 2,064 bytes in 1 blocks
==14084==    still reachable: 4,096 bytes in 1 blocks
==14084==         suppressed: 24,506 bytes in 187 blocks
==14084== Rerun with --leak-check=full to see details of leaked memory
==14084== 
==14084== For counts of detected and suppressed errors, rerun with: -v
==14084== ERROR SUMMARY: 19 errors from 2 contexts (suppressed: 0 from 0)

So, apparently, I've a lot of errors, but I'm not managing to find them. For example, valgrind is claiming Invalid read of size 1, but I see nowhere where I'm reading a wrong location in memory which produces undefined behaviour.

Edit

I've recompiled my code with

 gcc -g -o findword findword.c

And I've replaced the new valgrind output above.

Answer 1

Well, for one: you calloc a new buffer:

tmp = calloc(n, sizeof(char));

and copy the contents:

for(int i=0; i <= strlen(final); ++i)
   tmp[i] = final[i];

and free the final:

free(final);

But you do not assign a new pointer final, thus now final points to freed memory, yet later you do strlen() on that.

Also do not call strlen() all the time, it is very slow. Especially not in the loop conditions. Use strcpy or strncpy to copy strings to new arrays instead of looping. Use realloc for resizing the memory area instead of callocing. See my example that you didn't want to see.