Problems freeing memory

[Archaemic]

Most of the time when I free memory, my PSP is fine, but when I try to free two specific pieces of memory, it gives me a bus error ever single time. I've compiled the same code on Windows (with standard C or SDL system calls instead of PSP system calls) and it runs 100% fine (except for one segfault in portions of SDL's code, not mine. It doesn't crash the program, so I'm fine).

The memory allocation looks like this:

Code: Select all

		book.page_order = malloc(sizeof(char*)*book.num_pages);
		int i;
		unzGoToFirstFile(book.zip_file);
		int status = UNZ_OK;
		for(i = 0; status == UNZ_OK; i++) {
			char *block = init_block(1);
			unzGetCurrentFileInfo(book.zip_file,NULL,block,1024,NULL,0,NULL,0);
			book.page_order[i] = block;
			status = unzGoToNextFile(book.zip_file);
		}

Where init_block is such:

Code: Select all

inline char* init_block(Uint16 kb) {
	Uint32 loc, bytes = kb*1024;
	char* block = malloc(sizeof(char)*bytes);
	for&#40;loc = 0; loc < bytes; loc++&#41;
		block&#91;loc&#93; = 0;
	return block;
&#125;

(I should put in a extra check, but...)

The freeing looks like this:

Code: Select all

void close_comic_book(comic_book book) {
	if(book.type == comic_book_zip)
		unzClose(book.zip_file);
#	ifndef PSP
	int i;
	for&#40;i = 0; i < book.num_pages; i++&#41; &#123;
		if&#40;book.page_order&#91;i&#93; == NULL&#41; &#123;
			printf&#40;"Memory leak detected."&#41;;
			next_line&#40;&#41;;
		&#125;
		else free&#40;book.page_order&#91;i&#93;&#41;; //This is the line that crashes.
	&#125;
#	endif
	free&#40;book.page_order&#41;;
&#125;

It crashes on the first iteration of the loop, so it's not a buffer overflow. Trying to free even the first item manually fails.

Is my init_block code buggy? Or could this be an error with PSPSDK?

Edit] Ah, I forgot to mention. The memory appears to get corrupted by the unzClose, but putting the free before the unzClose doesn't fix anything; it still crashes in the same place.

Code: Select all

(gdb) print *book->page_order
$4 = 0x8a21500 "Deadpool 01/Deadpool - 001 00fc.jpg"
(gdb) print book->page_order[1]
$5 = 0x8a21910 "Deadpool 01/Deadpool - 001 01.jpg"
(gdb) until
471                     unzClose(book->zip_file);
(gdb) until
474             for&#40;i = 0; i < book->num_pages; i++&#41; &#123;
&#40;gdb&#41; print book->page_order&#91;1&#93;
$6 = 0x8a21910 "Deadpool"
&#40;gdb&#41; print book->page_order&#91;&#93;
A syntax error in expression, near `&#93;'.
&#40;gdb&#41; print book->page_order&#91;0&#93;
$7 = 0x8a21500 "Deadpool 01/\020\031¢\bpool - 001 00fc.jpg"

[StrmnNrmn]

Or could this be an error with PSPSDK?

Given how heavily used PSPSDK/zlib are used, is it's much more likely the error is somewhere in your code.

A couple of things you could try:

memset() book.page_order to 0 after allocation - from your code it looks like you may end up with uninitialised memory if unzGoToNextFile returns an error before you've filled in book.num_pages.

Are you using MSVC on Windows or GCC? You could try running with the debug CRT and setting _CRTDBG_CHECK_ALWAYS_DF (see MSDN here). This will warn you about heap corruption etc closer to the point of error.

[Archaemic]

I'm pretty sure it's not PSPSDK too.

Anyway, you can still free uninitialized memory so long as you've allocated it, can't you? I can't imagine why could couldn't.

Also, I forgot to mention, I'm using minizip for the unz functions.

I have absolutely no clue why this is crashing, though. I tried compiling it without optimizations and it still crashes. It's really weird.

[StrmnNrmn]

Anyway, you can still free uninitialized memory so long as you've allocated it, can't you? I can't imagine why could couldn't.

Freeing book.page_order when the contents are uninitialised is fine, but freeing book.page_order will crash if it's not been initialised. In your initialisation loop, you won't initialise all the elements of book.page_order if unzGoToNextFile() returns an error.

So as an example, let's say book.num_pages is 5 and that unzGoToNextFile returns an error the second time it's called. Your book.page_order array will end up looking like this:

book.page_order[0] = someptr;
book.page_order[1] = anotherptr;
book.page_order[2] = random_uninitialised_value1;
book.page_order[3] = random_uninitialised_value2;
book.page_order[4] = random_uninitialised_value3;

Code: Select all

book.page_order = malloc(sizeof(char*)*book.num_pages); 
memset(book.page_order, 0, sizeof(char*)*book.num_pages);
int i;
...

[StrmnNrmn]

One more thing - there's no check in your initialisation for-loop that i < book.num_pages. If there are more files in the zip than you've reserved entries for, you'll stomp all over the memory following book.page_order.

I can't see where you get book.num_pages from, but you might want to update the initialisation loop to look something like this:

Code: Select all

      book.page_order = malloc(sizeof(char*)*book.num_pages);
      memset(book.page_order, 0, sizeof(char*)*book.num_pages); 
      int i;
      unzGoToFirstFile(book.zip_file);
      int status = UNZ_OK;
      for&#40;i = 0; i < book.num_pages; i++&#41; &#123;
         char *block = init_block&#40;1&#41;;
         unzGetCurrentFileInfo&#40;book.zip_file,NULL,block,1024,NULL,0,NULL,0&#41;;
         book.page_order&#91;i&#93; = block;
         status = unzGoToNextFile&#40;book.zip_file&#41;;
         if&#40;status != UNZ_OK&#41;
            break;
      &#125;

[Archaemic]

book.num_pages is obtained by running through the zip file once to find out exactly how many pages there are (not all files in the zip are pages!). Not exactly the most efficient way, but it's the only way I can think of.

And as I said, even freeing the first one crashes the program.

Just for reference, the whole block looks like this now (and works under Windows):

Code: Select all

		book.type = comic_book_zip;
		book.zip_file = unzOpen(filename);
		if(book.zip_file == NULL) {
			printf("Error opening file.");
			next_line();
			unzClose(book.zip_file);
			book.zip_file = NULL;
			return book;
		}
		int status = UNZ_OK;
		do {
			char *block = init_block(1);
			unzGetCurrentFileInfo(book.zip_file,NULL,block,1024,NULL,0,NULL,0);
			if(block[1023] != 0) {
				printf("Filename in zip too long.");
				next_line();
				unzClose(book.zip_file);
				book.zip_file = NULL;
				free(block);
				return book;
			}
			if((ends_with(block,".jpg") > 0) || (ends_with(block,".jpeg") > 0) ||
			  (ends_with(block,".png") > 0) || (ends_with(block,".tiff") > 0) ||
			  (ends_with(block,".bmp") > 0) || (ends_with(block,".gif") > 0))
				book.num_pages++;
			free(block);
		} while((status = unzGoToNextFile(book.zip_file)) == UNZ_OK);
		if(status != UNZ_END_OF_LIST_OF_FILE) {
			printf("Error reading zip.");
			next_line();
			unzClose(book.zip_file);
			book.zip_file = NULL;
			return book;
		&#125; else if&#40;book.num_pages < 1&#41; &#123;
			printf&#40;"Book is empty."&#41;;
			next_line&#40;&#41;;
			unzClose&#40;book.zip_file&#41;;
			book.zip_file = NULL;
			return book;
		&#125;
		book.page_order = malloc&#40;sizeof&#40;char*&#41;*book.num_pages&#41;;
		memset&#40;book.page_order,0,book.num_pages*sizeof&#40;char*&#41;&#41;;
		int i;
		unzGoToFirstFile&#40;book.zip_file&#41;;
		status = UNZ_OK;
		for&#40;i = 0; &#40;status == UNZ_OK&#41; && &#40;i < book.num_pages&#41;; i++&#41; &#123;
			char *block = init_block&#40;1&#41;;
			unzGetCurrentFileInfo&#40;book.zip_file,NULL,block,1024,NULL,0,NULL,0&#41;;
			book.page_order&#91;i&#93; = block;
			status = unzGoToNextFile&#40;book.zip_file&#41;;
		&#125;
		unzGoToFirstFile&#40;book.zip_file&#41;;

I'm also having problems with SDL filling the whole screen when running on the PSP. It's weird. It only writes to the top-left pixel. I think I have some serious memory corruption going on here.

Also, it only does this sometimes. Mostly when I program something wrong. But the calls stay the same. Really weird. This doesn't happen when I compile -O0 (although the crashing still does). psp-gdb just isn't helping me.