Hi,
I need help with this snippet of code which breaks down at the free() line.
I got the above mentioned error. what am I doing wrong?Code:/* called function */ unsigned char* func(){ unsigned long length = 9; unsigned char *p = NULL; p = malloc(size of (unsigned char) * length); /* error checking for p */ return p; } /* caller function */ . . p = func(); free(p); . . .
Thanks.
Could you include a bit more of your code around what your show here?
PS: This looks like a memory corruption, you're probably overwriting malloc internal data while playing with the pointers.
Last edited by root4; 07-30-2012 at 02:15 PM.
The p that was malloc'd inside the function, is local. You can't free it outside the function, since it's out of scope.
Show actual, compilable code.
is not valid c code.Code:p = malloc(size of (unsigned char) * length);
Kurt
That message is a run-time message that usually indicates some of malloc's internal pointers/counters are screwed up. If p were out of scope, it wouldn't even compile, so there's probably a local p in the calling function. I would suspect that in the full code, there is something happening between the p = func(); line and the free(p); line, that overwrites the bounds of allocated memory and trashes malloc's internal record keeping.
@livin: What you posted looks fine. Like root4 said, we need to see more code to be sure. Make sure it's the actual code you are compiling and running, don't paraphrase it or recreate it from memory -- copy and paste from your editor.
What you're doing wrong is unfortunately filtering out the bits that you think aren't relevant, without being qualified to do so, since you don't understand the cause of the problem.Originally Posted by livin
If you don't show enough code, then we wont be able to find the problem. Of course without seeing your code we cant know how much you need to show either. One thing's for sure though, it's more than that that we need to see!
My homepage
Advice: Take only as directed - If symptoms persist, please see your debugger
Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"
Hi guys. Sorry for the late response. The code was in my laptop and I had to wait till I go home to fetch it. Its part of a bigger shared project. What I am trying to do here is trying to get a snapshot of what pages are currently in memory using the mincore linux system call. It takes in an char * argument along with size and starting page address. If I haven't already mentioned, the compilation went without any erros. But again I dont have the whole code as this is a shared project. My part is to get a snapshot of the application pages in memory.
so that is the file whose function gets called by my main() when mincore flag is set when invoking the main as a cmd line argCode:#include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <string.h> #include <errno.h> #include <unistd.h> #include <sys/mman.h> unsigned char *mincore_pages(unsigned long *addr, unsigned long bytes){ unsigned long length_of_vector = ((bytes /getpagesize()) + 1); unsigned char *ptr = NULL; /* initialize to NULL */ if((ptr = malloc( (sizeof(unsigned char)) * length_of_vector )) == NULL){ fprintf(stderr, "Unable to allocate requested memory"); fprintf(stdout,"exiting now..."); exit(1); } else{ /* note that mincore function writes the page information in the address pointed to by ptr. 1 byte is used per page. the last bit of the byte is set if the page is in memory else it remains 0 . so I made sure that I allcated enough bytes using malloc to account for each page of my bytes size chunk using malloc() */ if((mincore(addr, bytes, ptr)) == -1){ fprintf(stderr, "mincore failed with error: %s", strerror(errno)); free(ptr); exit(1); } } return ptr; }
Code:/* my part of main() */ if (mincore){ unsigned char *ptr; unsigned long *p = allocate_page_segment(bytes); /* defined in a file written by my fellow project mate. it returns a valid segment. I checked that using ipcs on my host system by stoping the execution at this line using gdb */ if(ptr = mincore_pages(p,bytes) == (void *) -1){ perror("mincore failed\n"); exit(1); } /* if successful, I just want to free the ptr for now. later, I will try to may be print it out or something */ free(ptr); }
Last edited by livin; 07-31-2012 at 10:08 AM.
This is one of the main reasons to use Valgrind. It will show you where memory is being accessed incorrectly/overwritten/left allocated at the end of the program.
Either way, the code you posted won't compile because many of the "mincore" functions are missing.
Also:
is incorrect due to operator precedence, you should specify:Code:if(ptr = mincore_pages(p,bytes) == (void *) -1)
Code:if ((ptr = mincore_pages(p, bytes)) == -1)
k thanks I did that. but removing the (void *) is warning me that I am comparing a pointer to an interger. I think (void *) is necessary too. But the actual problem comes when I am freeing the mallocd memory and not when I am read/writing it. Freeing should be as simple as passing the malloc returned pointer to free. and since I am not rewriting the pointer anywhere, that should work isn't it?
> I think (void *) is necessary too.
Yes, it is. Sorry for forgetting to re-type it.
> and since I am not rewriting the pointer anywhere, that should work isn't it?
It should work. Most likely your program is overwriting memory somewhere it shouldn't be. Malloc saves ancillary data besides the actual block of memory, especially the size of the block (ever noticed that you don't have to specify how much memory to free?) and the said data is corrupted, free() doesn't know what to do. Again, Valgrind catches these types of errors easily.
The size of the allocation looks suspiciously wrong to me:The division sets off alarm bells.Code:((bytes /getpagesize()) + 1);
Either that or the variable name is very wrong.
I would try changing the first three lines of mincore_pages to:The +1 may not be needed either, not sure.Code:unsigned char *ptr = malloc(bytes+1); if (ptr == NULL) {
Note that there's no point in checking the return result of mincore_pages against -1 because as it is written here, it can only ever return a valid pointer to dynamically alocated memory.
My homepage
Advice: Take only as directed - If symptoms persist, please see your debugger
Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"
iMalc, I dont think allocating bytes+1 is the right size. Each page is represented using the last bit of a byte. So I would just be needing as mnay bytes as the pages available. That is why i calculated the number of pages I have, and allocated that +1 just to be sure it is not overrun. i also checked it in GDB and saw that it was returning a valid number of pages. Everyhting looks fine until free() is executed. Can there be a bug in the mincore implementation?
I think that is the most likely cause.
Another thing I find confusing is that there seems to be a variable called mincore and a function with the same name.
Kurt
Okay, well I don't know anything about minicore, but what struck me as odd is that the calculation with the division is exactly the kind of calculation where you take the total size and divide it by the size of one item, to get the count of the number of items. Then using the count of the number of items to calculate how much to allocate in bytes, through the malloc call, may not make sense.
As I said, either your thinking and hence the calculation, is wrong, OR the variable names e.g. "bytes" don't make sense. If it's not the former, then it is probably the later.
Anyway, taking the division away is a really quick thing you can do to check if it helps with your problem. All it does is causes some overallocation, and provided that doesn't boost the memory usage by a ridiculous amount, there's no harm in trying to rule it out.
Have you understood why mincore_pages can never return NULL or -1?
Last edited by iMalc; 08-01-2012 at 01:58 AM.
My homepage
Advice: Take only as directed - If symptoms persist, please see your debugger
Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"
