Address out of bounds problem

[raczzoli]

I have a function which reads headers from mp3 files, and extracts the title, artist, album etc. The code looks like this:

typedef struct
{
	char *title;
	char *artist;
	char *album;
} media_t;


static int extract_mp3(FILE *fp, media_t *media)
{
	char tag[4];
	tag[0]	= '\0';

	fseek(fp, -128, SEEK_END);
	fread (tag, 1, 3, fp);

	tag[3] = '\0';

	if(strcmp((const char *)tag, "TAG") == 0)
	{
		media->title	= NULL;
		media->artist	= NULL;
		media->album	= NULL;

		media->title 	= malloc(31);
		media->artist 	= malloc(31);
		media->album 	= malloc(31);

		fread(media->title, 1, 30, fp);
		fread(media->artist, 1, 30, fp);
		fread(media->album, 1, 30, fp);

		media->title 	= trim(media->title,' ');
		media->artist 	= trim(media->artist,' ');
		media->album 	= trim(media->album,' ');
	}

	return 0;
}

I didn't include the part with the fopen. So here is my problem: If I put a breakpoint after the mallocs, in the debugger everything looks fine, all three variables (title,artist,album) look initialized, containing \0. But if I put a breakpoint after the first fread (fread(media->title, 1, 30, fp) the last variable (album) changes it's value, and the debugger tells me that it's address is out of bounds, and before the last fread the execution breaks with segfault.

Can somebody tell me, what am I doing wrong, and how can the last member change it's value when the execution didn't even get to it.


Thank you.

[Salem]

fread() won't append a \0 to the ends of your strings.
Nor will malloc fill the space with zeros.

If trim() is looking for a \0, it's going to be screwed.

Likewise, if trim returns a pointer which is NOT the one passed in, then you're screwed when you call free().

[raczzoli]

Hi, and thanks for the reply.

I changed the code, so now it looks like this;

media->title 	= malloc(31); memset(media->title, 0, 31);
media->artist 	= malloc(31); memset(media->artist, 0, 31);
media->album 	= malloc(31); memset(media->album, 0, 31);

I also commented out the lines where the trim function is called, but the problem persists. After the first fread the media->album gets a new address which apparently out of bounds. The interesting thing is, that if I comment out the first fread, so only the artist and album variables remain, everything is ok, the album changes it's value only when the execution passes the fread for it. Is it something wrong with the fread function?

[oogabooga]

You can replace this:

media->title = malloc(31); memset(media->title, 0, 31);

with this

media->title = calloc(31, 1);

But the code you've posted doesn't seem to have a problem so you'll have to post a whole program, preferably the smallest program that recreates the problem.

[Salem]

What does this do, if your main is only this.

int main ( ) {
    FILE *fp = fopen("file.mp3", "rb");
    media_t m;
    extract_mp3(fp, &m);
    return 0;
}

[raczzoli]

This way it works, so the problem is somewhere else in my application. Thanks for your help, and I will let you know if I solved it, and how.

[raczzoli]

I finally solved it. I changed the media_t structure so all the members are statically allocated. Now it looks like this:

typedef struct
{
	char title[128];
	char artist[128];
	char album[128];
} media_t;

I've also removed the mallocs from the extract_mp3 function, only left the memset. Don't know what was the problem with the first solution, but this way it works just fine, and because there is no more dynamic allocation it is faster too.

Thank you for your help.