[MPlayer-dev-eng] [PATCH] cddb crash

[Joshua Roys]

Reimar Döffinger wrote:
> Hello,
> 
> Please provide a valgrind log of the problem.
> 
Attached.

>> Index: stream/stream_cddb.c
>> ===================================================================
>> --- stream/stream_cddb.c	(revision 25766)
>> +++ stream/stream_cddb.c	(working copy)
>> @@ -447,15 +447,28 @@
>>  					mp_msg(MSGT_DEMUX, MSGL_FIXME, "Unable to find '.'\n");
>>  					ptr2=ptr+strlen(ptr); //return -1;
>>  				}
>> +				else
>> +				{
>> +					ptr2 += 3;
>> +				}
>>  			}
>> +			else
>> +			{
>> +				ptr2 += 5;
>> +			}
> 
> Seems like nonsense to me, we shouldn't need the terminating . line
> 
The patch's main purpose is to correct a crash.  In addition I tried to
keep the '.' around, since that's what other media players seem to do.

>> -			cddb_data->xmcd_file = ptr;
>> -			cddb_data->xmcd_file_size = ptr2-ptr+2;
>> +			if ( (cddb_data->xmcd_file = malloc(ptr2 - ptr + 1)) == NULL)
>> +			{
>> +				mp_msg(MSGT_DEMUX,MSGL_FATAL,"Memory allocation failed\n");
>> +				return -1;
>> +			}
>> +			memcpy(cddb_data->xmcd_file, ptr, ptr2 - ptr);
>> +			cddb_data->xmcd_file_size = ptr2 - ptr;
> 
> Possibly makes sense in order to avoid a hack, but
> 
>>  			cddb_data->xmcd_file[cddb_data->xmcd_file_size] = '\0';
>>  			// Avoid the http_free function to free the xmcd file...save a mempcy...
>>  			http_hdr->body = NULL;
> 
> With this it will leak memory.
Here's the difference between the two runs.

Before patch:
==32566== LEAK SUMMARY:
==32566==    definitely lost: 2,544 bytes in 6 blocks.
==32566==    indirectly lost: 56,592 bytes in 3 blocks.
==32566==      possibly lost: 0 bytes in 0 blocks.
==32566==    still reachable: 36,835 bytes in 31 blocks.
==32566==         suppressed: 0 bytes in 0 blocks.

After patch:
==1201== LEAK SUMMARY:
==1201==    definitely lost: 2,544 bytes in 6 blocks.
==1201==    indirectly lost: 56,592 bytes in 3 blocks.
==1201==      possibly lost: 0 bytes in 0 blocks.
==1201==    still reachable: 36,863 bytes in 32 blocks.
==1201==         suppressed: 0 bytes in 0 blocks.

> 
> Greetings,
> Reimar Döffinger

Thanks,

Joshua
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0-crash.txt.gz
Type: application/x-gzip
Size: 2336 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20080119/0cb7cd46/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1-valgrind.txt.gz
Type: application/x-gzip
Size: 7466 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20080119/0cb7cd46/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2-success.txt.gz
Type: application/x-gzip
Size: 2972 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20080119/0cb7cd46/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3-valgrind.txt.gz
Type: application/x-gzip
Size: 3270 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20080119/0cb7cd46/attachment-0003.bin>