valgrind error: invalid read

Question

I am writing following program:

int main() {
    long sLen = 0;
    char ch, *str = (char *)malloc(sizeof(char));
    while((ch = getchar()) != '\n')
    {
        str[sLen] = ch;
        str = (char *)realloc(str, (1 + (sLen++)) * sizeof(char));
    }

    int length = strlen(str);

    printf("Length of string: %d\n", length);
    printf("Input string: \"%s\"\n", str);
    sort(str);
    printf("Sorted: \"%s\"\n", str);
    printf("Number of permutation: %i\n", factorial(length));
    permute(str, 0, length);
    free(str);
    return 0;
}

void sort(char sorted_str[]) {
    for (int i = 0; sorted_str[i+1]; ++i)
        for (int j = i + 1; sorted_str[j]; ++j)
            if (sorted_str[i] > sorted_str[j]) {
                char tmp = sorted_str[i];
                sorted_str[i] = sorted_str[j];
                sorted_str[j] = tmp;
            }

}

void permute(char *a, int i, int n) {
    if (i == (n-1)) printf("%s\n", a);
    else {
        for (int j = i; j < n; j++) {
            swap((a+i), (a+j));
            permute(a, i+1, n);
            swap((a+i), (a+j));
        }
    }
}

And valgrind give me this:

==20113== Invalid read of size 1
==20113==    at 0x4C294F4: strlen (mc_replace_strmem.c:390)
==20113==    by 0x400749: main (main.c:23)
==20113==  Address 0x51ba091 is 0 bytes after a block of size 1 alloc'd
==20113==    at 0x4C28CCE: realloc (vg_replace_malloc.c:632)
==20113==    by 0x40072B: main (main.c:19)
==20113== Invalid read of size 1
==20113==    at 0x4E749EA: vfprintf (vfprintf.c:1623)
==20113==    by 0x4E7D1A9: printf (printf.c:35)
==20113==    by 0x400776: main (main.c:26)
==20113==  Address 0x51ba091 is 0 bytes after a block of size 1 alloc'd
==20113==    at 0x4C28CCE: realloc (vg_replace_malloc.c:632)
==20113==    by 0x40072B: main (main.c:19)
==20113== Invalid read of size 1
==20113==    at 0x400893: sort (main.c:36)
==20113==    by 0x400782: main (main.c:27)
==20113==  Address 0x51ba091 is 0 bytes after a block of size 1 alloc'd
==20113==    at 0x4C28CCE: realloc (vg_replace_malloc.c:632)
==20113==    by 0x40072B: main (main.c:19)
==20113== Invalid read of size 1
==20113==    at 0x4E749EA: vfprintf (vfprintf.c:1623)
==20113==    by 0x4E7D1A9: printf (printf.c:35)
==20113==    by 0x400798: main (main.c:28)
==20113==  Address 0x51ba091 is 0 bytes after a block of size 1 alloc'd
==20113==    at 0x4C28CCE: realloc (vg_replace_malloc.c:632)
==20113==    by 0x40072B: main (main.c:19)
==20113== Invalid read of size 1
==20113==    at 0x4C29514: __GI_strlen (mc_replace_strmem.c:391)
==20113==    by 0x4E98DAA: puts (ioputs.c:37)
==20113==    by 0x4008C8: permute (main.c:47)
==20113==    by 0x4007C7: main (main.c:30)
==20113==  Address 0x51ba091 is 0 bytes after a block of size 1 alloc'd
==20113==    at 0x4C28CCE: realloc (vg_replace_malloc.c:632)
==20113==    by 0x40072B: main (main.c:19)
==20113== HEAP SUMMARY:
==20113==     in use at exit: 0 bytes in 0 blocks
==20113==   total heap usage: 2 allocs, 2 frees, 2 bytes allocated
==20113== All heap blocks were freed -- no leaks are possible
==20113== For counts of detected and suppressed errors, rerun with: -v
==20113== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 4 from 4)

Can somebody help with this? It's working, but I want to have a correct code. My input is a. Program is working with this, but valgrind give me this 5 errors. I know that my mistake is reading from memory, that valgrind don't like, but I cannot find this place in code.

When you call strlen in main, str isn't null-terminated, so it proceeds into illegal memory regions. (The call is also redundant, because you already keep track of the length so far in sLen.) The other string functions, including your own sort, require str to be null-terminated, too. — M Oehm

David Ranieri David Ranieri · Accepted Answer · 2015-04-28T17:54:37

long sLen = 0;
int ch; /* getchar() wants an int (not a a char) */
char *str = malloc(1); /* sizeof(char) is always 1, and don't cast malloc */
/* check the result of malloc */
if (str == NULL) {
    perror("malloc");
    exit(EXIT_FAILURE);
}
while ((ch = getchar()) != '\n')
{
    str[sLen] = ch;
    str = realloc(str, 1 + ++sLen); // use ++sLen in order to get enough space
    /* check the result of realloc */
    if (str == NULL) {
        perror("realloc");
        exit(EXIT_FAILURE);
    }
}
/* this is redundant, use sLen */
/* int length = strlen(str); */
int length = sLen;
/* NUL-terminate the string */
str[sLen] = '\0';

On the other hand:

for (int i = 0; sorted_str[i+1]; ++i)

This is dangerous (you read outside of the bounds of the array if sorted_str is empty), change to:

for (int i = 0; sorted_str[i] && sorted_str[i+1]; ++i)

valgrind error: invalid read

1 Answers