On Sun, Dec 2, 2018 at 3:37 PM Thomas Schmitt <scdbac...@gmx.net> wrote:
> Hi, > > i think that this is now wrong: > > > if (track_num == CDIO_CDROM_LEADOUT_TRACK) > > > track_num = TOTAL_TRACKS + 1; > Ok. You have commit access to the repository: fix it the way you think it should be. > Edd Barrett wrote: > > I think that's ok after all. If we look at the definition of > > TOTAL_TRACKS, we see that it takes the starting track into account: > > #define TOTAL_TRACKS (_obj->tochdr.ending_track \ > > - _obj->tochdr.starting_track + 1) > > But it subtracts the nominal start track number from the nominal end > number to get the count of tracks. > (Assuming that _obj->tochdr.starting_track == _obj->gen.i_first_track.) > > gnu_linux.c adds the count of tracks to the nominal start number. > p_env->gen.i_tracks + p_env->gen.i_first_track > This is equal to (TOTAL_TRACKS + 1) only if .i_first_track == 1. > (Assuming that _env->gen.i_tracks == TOTAL_TRACKS.) > > My main argument is: > > Before your pending change, (TOTAL_TRACKS + 1) was the highest permissible > nominal track number in netbsd.c. One above the highest track number > of a payload track, under the assumption that first_track is 1. > > By your pending change, the highest permissible nominal track number > becomes > (first_track + _obj->gen.i_tracks) > with > first_track = _obj->gen.i_first_track; > > This matches the gnu_linux.c replacement value for > CDIO_CDROM_LEADOUT_TRACK: > p_env->gen.i_tracks + p_env->gen.i_first_track > which is the highest permissible number there too and one above the > highest payload track number. > > ------------------------------------------------------------------------ > Example: > > Let's assume > first_track= 7 , ending_track= 11 > => TOTAL_TRACKS= 5 > => if (track_num == CDIO_CDROM_LEADOUT_TRACK) track_num= 6; > But this means > track_num < first_track > and thus > return CDIO_INVALID_TRACK; > > ------------------------------------------------------------------------ > > fgrep finds usage of cdio_get_track_*(CDIO_CDROM_LEADOUT_TRACK) in > > libcdio/example/C++/OO/tracks.cpp > libcdio/example/audio.c > libcdio/example/discid.c > libcdio/example/tracks.c > libcdio/src/cddb.c > > ... plus "Uh oh, this looks wrong too" in these drivers: > > libcdio/lib/driver/FreeBSD/freebsd.c > libcdio/lib/driver/MSWindows/win32.c > libcdio/lib/driver/osx.c > libcdio/lib/driver/image/bincue.c (are start tracks > 1 possible ?) > libcdio/lib/driver/image/cdrdao.c (are start tracks > 1 possible ?) > > ... plus a wrong assumption in demo code: > > src/cd-info.c loops from first_track to i_tracks and then hops to > CDIO_CDROM_LEADOUT_TRACK: > > if (i == i_tracks) i = CDIO_CDROM_LEADOUT_TRACK-1; > > Edd found the same spot in his tests: > > cd-info suffers from the same > > issue that the NetBSD driver had: assumes the start track is 1. > > Here's the fix for that: > > > https://github.com/vext01/libcdio/commit/49b1910b19d9a4b41eb2200e61d77e20f1802f58 > > if (i == i_first_track + i_tracks - 1) { > > i = CDIO_CDROM_LEADOUT_TRACK; > > Rocky Bernstein wrote: > > the value in the assignment to i should have been > > CDIO_CDROM_LEADOUT_TRACK-1, not CDIO_CDROM_LEADOUT_TRACK. > > So we seem to agree at this point. > > But does this work as expected if there still is in netbsd.c : > > if (track_num == CDIO_CDROM_LEADOUT_TRACK) > track_num = TOTAL_TRACKS + 1; > > Even if track_num does not become lower than first_track, it will still > be the number of a payload track if first_track is larger than 1. > But the caller wants info from the lead-out pseudo-track. > > Error symptom would be a LBA of lead-out that is lower than the end > address of the last payload track. > > > > cd-paranoia looks more complicated. Can someone try a disk with an odd > > starting track number on Linux? Does it work? > > Dunno if i ever ran cdparanoia. > What command exactly , and what shall be the outcome ? > > Do you see different behavior with odd versus even start number ? > > > Have a nice day :) > > Thomas > > >
