[MPlayer-dev-eng] [PATCH] stream_cdda.c bugs and such

[Reimar Döffinger]

On 3 Jan 2012, at 17:22, Ingo Brückl <ib at wupperonline.de> wrote:
> I wrote on Mon, 26 Dec 2011 13:50:34 +0100:
> 
>> Reimar Döffinger wrote on Fri, 23 Dec 2011 17:48:53 +0100:
> 
>>> On Tue, Dec 20, 2011 at 06:54:07PM +0100, Ingo Brückl wrote:
>>>> #3: -1 is never a valid track
> 
>>> No, but -1 means "error". Your patch breaks the error case (e.g. broken
>>> TOC) by always returning 0 then.
> 
>> Oh, I see, but no use is made of the error. The return value is instantly
>> used for calculation. Shouldn't there be a check and a STREAM_ERROR then?
> 
>>  Index: stream/stream_cdda.c
>>  ===================================================================
>>  --- stream/stream_cdda.c      (revision 34466)
>>  +++ stream/stream_cdda.c      (working copy)
>>  @@ -247,6 +247,7 @@
>>       {
>>         int start_track = get_track_by_sector(p, p->start_sector);
>>         int end_track = get_track_by_sector(p, p->end_sector);
>>  +      if (start_track == -1 || end_track == -1) return STREAM_ERROR;
>>         *(unsigned int *)arg = end_track + 1 - start_track;
>>         return STREAM_OK;
>>       }
>>  @@ -256,6 +257,7 @@
>>         unsigned int track = *(unsigned int *)arg;
>>         int start_track = get_track_by_sector(p, p->start_sector);
>>         int end_track = get_track_by_sector(p, p->end_sector);
>>  +      if (start_track == -1 || end_track == -1) return STREAM_ERROR;
>>         int seek_sector;
>>         track += start_track;
>>         if (track > end_track) {
>>  @@ -273,6 +275,7 @@
>>       {
>>         int start_track = get_track_by_sector(p, p->start_sector);
>>         int cur_track = get_track_by_sector(p, p->sector);
>>  +      if (start_track == -1 || cur_track == -1) return STREAM_ERROR;
>>         *(unsigned int *)arg = cur_track - start_track;
>>         return STREAM_OK;
>>       }
> 
> Ping?

Except for the second patch mixing code and declarations it should be ok.
I don't know if anything will handle the error correctly though.