How to solve "invalid write size of n bytes" and "Address ___ is n bytes after a block of size n alloc'd"?

Question

valgrind --tool=memcheck --leak-check=yes ./9algorithm (here is the error message I get from valgrind)

==3110==  Invalid write of size 1
==3110==    at 0x400FD8: main (9algorithm.c:223)
==3110==  Address 0x51fc372 is 0 bytes after a block of size 2 alloc'd
==3110==    at 0x4C2C6AE: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x400FF5: main (9algorithm.c:226)
==3110== 
==3110== Invalid write of size 1
==3110==    at 0x40102F: main (9algorithm.c:233)
==3110==  Address 0x52ff657 is 1 bytes after a block of size 1,990 alloc'd
==3110==    at 0x4C2C6AE: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x400FF5: main (9algorithm.c:226)
==3110== Conditional jump or move depends on uninitialised value(s)
==3110==    at 0x4C3052E: strstr (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x400B20: mag_parse (9algorithm.c:63)
==3110==    by 0x40105B: main (9algorithm.c:235)
==3110== 
==3110== Conditional jump or move depends on uninitialised value(s)
==3110==    at 0x4C3052E: strstr (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x400C56: place_parse (9algorithm.c:103)
==3110==    by 0x401088: main (9algorithm.c:236)
==3110== 
==3110== Conditional jump or move depends on uninitialised value(s)
==3110==    at 0x4C3052E: strstr (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x400DB2: time_parse (9algorithm.c:145)
==3110==    by 0x4010B6: main (9algorithm.c:237)
==3110== 
==3110== Conditional jump or move depends on uninitialised value(s)
==3110==    at 0x4E80D0F: vfprintf (vfprintf.c:1655)
==3110==    by 0x4E87FF8: printf (printf.c:34)
==3110==    by 0x401131: main (9algorithm.c:239)
==3110== 
(null), Magnitude : (null), (null)
==3110== Conditional jump or move depends on uninitialised value(s)
==3110==    at 0x4C2B5C2: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x4011DF: main (9algorithm.c:326)
==3110== 
==3110== Conditional jump or move depends on uninitialised value(s)
==3110==    at 0x4C2B5C2: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x4011EF: main (9algorithm.c:327)
==3110== 
==3110== Conditional jump or move depends on uninitialised value(s)
==3110==    at 0x4C2B5C2: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x4011FF: main (9algorithm.c:328)
==3110== 
==3110== 
==3110== HEAP SUMMARY:
==3110==     in use at exit: 3,579 bytes in 2 blocks
==3110==   total heap usage: 1,794 allocs, 1,792 frees, 1,623,686 bytes allocated
==3110== 
==3110== 3,579 bytes in 2 blocks are definitely lost in loss record 1 of 1
==3110==    at 0x4C2C6AE: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==3110==    by 0x400FF5: main (9algorithm.c:226)
==3110== 
==3110== LEAK SUMMARY:
==3110==    definitely lost: 3,579 bytes in 2 blocks
==3110==    indirectly lost: 0 bytes in 0 blocks
==3110==      possibly lost: 0 bytes in 0 blocks
==3110==    still reachable: 0 bytes in 0 blocks
==3110==         suppressed: 0 bytes in 0 blocks
==3110== 
==3110== For counts of detected and suppressed errors, rerun with: -v
==3110== Use --track-origins=yes to see where uninitialised values come from
==3110== ERROR SUMMARY: 1799 errors from 10 contexts (suppressed: 2 from 2)

when I run ./9algorithm , it gives me this error:

, Magnitude : , (null)
*** Error in `./9algorithm': free(): invalid pointer: 0x00007fff8a4d7a10 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x80996)[0x7f48a7eb3996]
./9algorithm[0x4011e0]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f48a7e54de5]
./9algorithm[0x400839]
======= Memory map: ========
00400000-00402000 r-xp 00000000 08:07 1965405                            /home/Desktop/9algorithm
00601000-00602000 r--p 00001000 08:07 1965405                            /home/Desktop/9algorithm
00602000-00603000 rw-p 00002000 08:07 1965405                            /home/Desktop/9algorithm
01377000-01398000 rw-p 00000000 00:00 0                                  [heap]
7f48a7c1d000-7f48a7c32000 r-xp 00000000 08:07 654085                     /lib/x86_64-linux-gnu/libgcc_s.so.1
7f48a7c32000-7f48a7e31000 ---p 00015000 08:07 654085                     /lib/x86_64-linux-gnu/libgcc_s.so.1
7f48a7e31000-7f48a7e32000 r--p 00014000 08:07 654085                     /lib/x86_64-linux-gnu/libgcc_s.so.1
7f48a7e32000-7f48a7e33000 rw-p 00015000 08:07 654085                     /lib/x86_64-linux-gnu/libgcc_s.so.1
7f48a7e33000-7f48a7ff0000 r-xp 00000000 08:07 658204                     /lib/x86_64-linux-gnu/libc-2.17.so
7f48a7ff0000-7f48a81f0000 ---p 001bd000 08:07 658204                     /lib/x86_64-linux-gnu/libc-2.17.so
7f48a81f0000-7f48a81f4000 r--p 001bd000 08:07 658204                     /lib/x86_64-linux-gnu/libc-2.17.so
7f48a81f4000-7f48a81f6000 rw-p 001c1000 08:07 658204                     /lib/x86_64-linux-gnu/libc-2.17.so
7f48a81f6000-7f48a81fb000 rw-p 00000000 00:00 0 
7f48a81fb000-7f48a821e000 r-xp 00000000 08:07 658180                     /lib/x86_64-linux-gnu/ld-2.17.so
7f48a8403000-7f48a8406000 rw-p 00000000 00:00 0 
7f48a8418000-7f48a841d000 rw-p 00000000 00:00 0 
7f48a841d000-7f48a841e000 r--p 00022000 08:07 658180                     /lib/x86_64-linux-gnu/ld-2.17.so
7f48a841e000-7f48a8420000 rw-p 00023000 08:07 658180                     /lib/x86_64-linux-gnu/ld-2.17.so
7fff8a4b9000-7fff8a4da000 rw-p 00000000 00:00 0                          [stack]
7fff8a5ca000-7fff8a5cc000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
Aborted (core dumped)

main.c (line 223 - invalid write size) (line 233 - invalid write size) (line 226 - address ... after a block ....)

int main()
{
   FILE *fp = 0;
   char *filename = "e_quake.txt";
   struct information *e_quakePtr = 0;
   char *line = 0;
   char c = 0;
   int k = 0;
   int h = 0;

   fp = fopen(filename,"r");

   if(!fp){
       printf("Unable to open file: %s\n",filename);
       return -1;
   }

   e_quakePtr = (struct information *)malloc(1 * sizeof(struct information));

   if(!e_quakePtr){
       printf("Error on malloc on e_quakePtr");
       return 1;
   }

   line = (char *)malloc(1 * sizeof(char));

   if(!line){
       printf("Error on malloc on line");
       return 1;
   }

   while( (c = fgetc(fp)) != EOF ){
    if(c != '\n'){
        line[h] = c;
        h++;

        line = (char *)realloc(line,(h+1) * sizeof(char));
        if(!line){
            printf("Error on realloc on line");
            return 1;
        }
    }else{
        h++;
        line[h] = 0;

        e_quakePtr[k].mag = mag_parse(line);
        e_quakePtr[k].place = place_parse(line);
        e_quakePtr[k].time = time_parse(line);

        printf("%s, Magnitude : %s, %s\n",e_quakePtr[k].place, e_quakePtr[k].mag, e_quakePtr[k].time);

        k++;
        e_quakePtr = (struct information *)realloc(e_quakePtr,(k+1) * sizeof(struct information));

        if(!e_quakePtr){
            printf("Error on realloc on e_quakePtr");
            return 1;
        }

        h = 0;
        free(line);

        line = (char *)malloc(1 * sizeof(char));

        if(!line){
            printf("Error on malloc on line");
            return 1;
        }

    }


   free(e_quakePtr->mag);
   free(e_quakePtr->place);
   free(e_quakePtr->time);
   free(e_quakePtr);

   fclose(fp);
   return 0;
}

i can't find the root of the problem. the external functions i've used seem to work when i try it with another algorithm. but for this one, it had problems with my main.c

Not related to your problem: You should use NULL instead of 0 for pointers initialization. — HAL9000, Mar 22 '14 at 15:03
In C you [should not cast the return of `malloc` (and family)](http://stackoverflow.com/a/605858/440558). — Some programmer dude, Mar 22 '14 at 15:05
what is the difference if I cast it or not? @Joachim Pileborg — Mahrrkiee, Mar 22 '14 at 15:07
Also, be careful with reassigning the pointer you `realloc` to itself. If `realloc` fails it will return `NULL` and you will loose your original pointer, possibly causing a memory leak. — Some programmer dude, Mar 22 '14 at 15:09
@HAL9000, why do you say that? `0` is a perfectly valid null-pointer constant, and much less ambiguous than `NULL` for which nobody knows which type it has. http://gustedt.wordpress.com/2010/11/07/dont-use-null/ — Jens Gustedt, Mar 22 '14 at 15:46

score 2 · Answer 1 · answered Mar 22 '14 at 15:05

2

you have an error in your logic at

 h++;
 line[h] = 0;

this character is not allocated, I think.

Also:

casting the return of malloc and friends easily hides subtle bugs, don't do that.
sizeof(char) is 1 by definition

answered Mar 22 '14 at 15:05

Jens Gustedt

76,821
6
102
177

where do i put realloc? before the "h++;" expression? – Mahrrkiee Mar 22 '14 at 15:16

score 0 · Answer 2 · answered Mar 22 '14 at 15:09

0

You shouldnt realloc like that. Do instead

char *temp;

temp = realloc(line, (h+1) * sizeof(char));
if(!temp){
    printf("Error on realloc on line");
    free(line);
    return 1;
}
else{line = temp;}

valter

answered Mar 22 '14 at 15:09

γηράσκω δ' αεί πολλά διδασκόμε

7,172
2
21
35

so i need to initialize another variable temp? – Mahrrkiee Mar 22 '14 at 15:12
@Mahrrkiee Yes, because if realloc fails then line becomes NULL and you cannot free it anymore – γηράσκω δ' αεί πολλά διδασκόμε Mar 22 '14 at 15:14

Gábor Buella · Answer 3 · 2014-03-22T15:24:53.167

0

Each time your write

 line = (char *)malloc(1 * sizeof(char));

it just allocates one char for line. Probably you meant more than one char, like when you use realloc. You also malloc it with size 1 each time in loop, as long as you don't meet a newline, that just overwrites the previous pointer, leaking 1 char of memory. <- EDIT ok, I noticed you free it before that, but still it doesn't make much sense allocate 1 char. You can just get rid of these two lines, the next realloc in the loop will solve it for you:

    free(line);

    line = (char *)malloc(1 * sizeof(char));

BTW, sizeof(char) is 1 by definition in the C standard, so

 1 * sizeof(char) == 1 * 1 == 1

sizeof just returns "how many chars wide" a type is. It is like asking "the number of chars in char"

If you would like to make sure it is type-aware allocation, the common best practice is:

  line = malloc(n * sizeof(*line));

This way, even the type of *line changes, the malloc will be still ok.

edited Mar 22 '14 at 15:24

answered Mar 22 '14 at 15:18

Gábor Buella

1,840
14
22

will realloc(line,0) be the same sa free(line)? @buellagabor – Mahrrkiee Mar 22 '14 at 15:24
As long as you use these thing in `main` , there is no real need to `free` them, they will be taken back by the OS once `main` returns. If this would a general function, that can called anywhere, you would need to just free it on the end of the function, like you do with e_quakePtr. – Gábor Buella Mar 22 '14 at 15:28
realloc(p, 0) -> My man page says "if size is zero and ptr is not NULL, a new, minimum sized object is allocated and the original object is freed." The Standard doesn't say anything about this case, just that "Unspeciﬁed behavior" is "Whether the calloc, malloc, and realloc functions return a null pointer or a pointer to an allocated object when the size requested is zero" So your realloc might return NULL, or just allocate 1 byte, I don't know – Gábor Buella Mar 22 '14 at 15:32

score 0 · Answer 4 · answered Mar 22 '14 at 15:28

0

Here are some useful Valgrind tips for you -

Excerpt from Valgrind documentation

answered Mar 22 '14 at 15:28

StackSmasher

359
1
3
6

How to solve "invalid write size of n bytes" and "Address ___ is n bytes after a block of size n alloc'd"?

4 Answers4