lorantalas
lorantalas

Reputation: 284

valgrind error: invalid read

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.

Upvotes: 1

Views: 1320

Answers (1)

David Ranieri
David Ranieri

Reputation: 41046

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)

Upvotes: 1

Related Questions