I have below code which gives me 2 errors - Invalid write of size 1
and Address 0x41f52a8 is 0 bytes after a block of size 128 alloc'd
. Below is the complete valgrind stack.
I could do a guess work and identify that memcpy(content2 + totalLength + 1, fileContentTemp, readBytes);
line of code has problem. So, I corrected it to memcpy(content2 + totalLength, fileContentTemp, readBytes);
and then my valgrind became really happy and all pass.
But I am not able to understand the reason. To me it looks that from 2nd time onwards I need to do + 1
because I would not like memcpy
to start writing from content2 + totalLength
address since it is the address where last byte was written so I thought to increment and starting from next address.
Also, another intriguing part is that if I do not correct it then I get following runtime error *** Error in ./server_issue': double free or corruption (!prev): 0x08c24170 ***
, I guess coming because of this line of code free(content2);
. free
basically free the pointer, now if pointer is having correct memory address (I verified that content2
has right memory address) then why exception occurs.
Code:
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <string.h>
typedef char BYTE;
bool load(FILE*, BYTE**, size_t*);
int main(void) {
FILE *file = fopen("/home/jharvard/psets/pset6/pset6_working/public/hello.html", "r");
BYTE *content;
size_t length;
load(file, &content, &length);
}
bool load(FILE *file, BYTE **content, size_t *length) {
//printf("file %p\n", file);
int totalLength = 0;
int readBytes = 0;
BYTE *content2 = NULL;
BYTE *fileContentTemp[64]; // working with 222222
while ((readBytes = fread(fileContentTemp, 1, 64, file)) > 0) {
printf("Reallocating %d bytes, ", readBytes);
content2 = realloc(content2, sizeof(BYTE) * (totalLength + readBytes));
printf("%p\n", content2);
if (totalLength != 0) {
memcpy(content2 + totalLength + 1, fileContentTemp, readBytes);
} else {
memcpy(content2 + totalLength, fileContentTemp, readBytes);
}
totalLength = totalLength + readBytes;
}
*length = totalLength;
*content = content2;
free(content2);
//printf("CC image: %s\n", *content);
//printf("length is %d\n", *length);
//printf("fileContent %p\n", *content);
//printf("file %p\n", file);
fclose(file);
//printf("length is %d\n", *length);
return true;
}
Valgrind stack:
appliance (~/psets/pset6/pset6_working): valgrind ./server_issue
==3206== Memcheck, a memory error detector
==3206== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==3206== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==3206== Command: ./server_issue
==3206==
Reallocating 64 bytes, 0x41f51b8
Reallocating 64 bytes, 0x41f5228
==3206== Invalid write of size 1
==3206== at 0x402F04B: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3206== by 0x80486AC: load (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206== by 0x8048599: main (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206== Address 0x41f52a8 is 0 bytes after a block of size 128 alloc'd
==3206== at 0x402C324: realloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3206== by 0x804865C: load (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206== by 0x8048599: main (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206==
Reallocating 64 bytes, 0x41f52d8
Reallocating 64 bytes, 0x41f53c8
Reallocating 60 bytes, 0x41f54f8
==3206==
==3206== HEAP SUMMARY:
==3206== in use at exit: 0 bytes in 0 blocks
==3206== total heap usage: 6 allocs, 6 frees, 1,308 bytes allocated
==3206==
==3206== All heap blocks were freed -- no leaks are possible
==3206==
==3206== For counts of detected and suppressed errors, rerun with: -v
==3206== ERROR SUMMARY: 4 errors from 1 contexts (suppressed: 0 from 0)
This comes down to the fact that a list of the first N integers, starting from 0, ends at N-1.
Let's try a concrete example. Suppose you write 4 bytes from index 0. You write indices 0, 1, 2 and 3. That's 4 bytes in total. So the next one to right is index 4, and not 5.
Suppose you write N bytes from index M. You write M, M+1, ..., M+N-1. The next one you write is M+N.
So replace the entire if
statement with
memcpy(content2 + totalLength, fileContentTemp, readBytes);
The if
should have felt wrong. An operation like that should not need special casing, so the fact you had an if
should have raised alarm bells.