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

[Reimar Döffinger]

Hello,
On Sat, Jan 19, 2008 at 10:59:24AM -0500, Joshua Roys wrote:
> I was experiencing crashes using latest SVN (25766) mplayer when trying
> to play a CD using CDDB.  This patch fixes them, and no memory
> violations are shown using valgrind anymore.

Please provide a valgrind log of the problem.

> 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

> -			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.

Greetings,
Reimar Döffinger