Thanks for traking this down. When I do valgrind testing, I just configure with:
configure --disabled-shared It is not elegant but it works. On Tue, Jun 26, 2018 at 9:13 AM, Thomas Schmitt <[email protected]> wrote: > Hi, > > i applied an equivalent change as in cdtext-raw.c, after Leon confirmed > that the gesture in driver/image/bincue.c > > /* Truncate header when it is too large. */ > if (cdt_data[0] > 0x80) { > size -= 4; > } > > was indeed intended to handle an MMC header which may be prepended to > the text pack array. > > See at end of mail or in > http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=ts-cdtext-fix&id= > 388e859770724155b13da7fbdce7bfbb65adb104 > > For testing, i faked a .CUE file which is intended to match > test/data/cdtext.cdt. > test/data/cdtextfile.cue : > > CDTEXTFILE "cdtext.cdt" > FILE "cdtextfile.bin" BINARY > TRACK 01 AUDIO > INDEX 00 00:00:00 > TRACK 02 AUDIO > INDEX 00 00:01:00 > INDEX 01 00:01:05 > TRACK 03 AUDIO > INDEX 00 00:03:00 > INDEX 01 00:03:05 > > A program which refers to .cue files is test/testpregap.c. It uses > cdio_open(,DRIVER_UNKNOWN). On my clone it is not compiled by default: > $ cd test > $ make testpregap > CC testpregap.o > CCLD testpregap > $ > > I added cdtextfile.cue to its list of test images. > It demands "cdtextfile.bin" when i run it. So i copied > $ cp data/cdda.bin data/cdtextfile.bin > Interesting error messages appear if track 2 gets less than 2 seconds of > length. > > But i want my own errors. So i spoiled the file name in cdtextfile.cue: > CDTEXTFILE "Xcdtext.cdt" > which yields > cdio 3 message: could not retrieve file info for > `.../libcdio/test/data/Xcdtext.cdt': > No such file or directory > > ------------------------------------------------------------------------- > Now for testing my code changes in bincue.c: > ------------------------------------------------------------------------- > The corrected destructor call if file size < 5: > http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=ts-cdtext-fix&id= > 53d38c7063c84c09c157441aad97934f603ef99f > > I changed in cdtextfile.cue > CDTEXTFILE "bad_cdtext.cdt" > with > $ dd if=/dev/zero bs=4 count=1 of=data/bad_cdtext.cdt > yields: > cdio 3 message: .../libcdio/test/data/cdtextfile.cue line 1: file > `.../libcdio/test/data/bad_cdtext.cdt' is too small to contain CD-TEXT > which is then supposed to be followed by the destructor call. > > No SIGSEGV. > > If somebody knows how to use valgrind in the pseudo-binaries like > ./testpregap, then please teach me. > > ------------------------------------------------------------------------- > The freshly changed recognition and skipping of MMC header: > > $ echo -n $'\x06\xC2' >test/data/cdtext_w_head.cdt > $ dd if=/dev/zero bs=1 count=2 >>data/cdtext_w_head.cdt > ... > $ cat data/cdtext.cdt >>data/cdtext_w_head.cdt > and in cdtextfile.cue > CDTEXTFILE "cdtext_w_head.cdt" > yields: > cdio 3 message: .../libcdio/test/data/cdtextfile.cue line 1: skipped 4 > bytes of apparent MMC header in CD-TEXT file `.../libcdio/test/data/cdtext_ > w_head.cdt' > > No SIGSEGV or protests from CD-TEXT parsing. > > (I had to raise the log_level of the message to CDIO_LOG_WARN because > else testpregap does not report it. But i think that CDIO_LOG_INFO is the > appropriate level for production.) > > ------------------------------------------------------------------------- > > I will think about whether and how above two experiments should become > part of libcdio. > Corrently it is a problem for me that testpregap wants a dedicated .bin > file for each .cue file. This would mean two or three *.bin of 700 KB each. > > > ========================================================================== > > diff --git a/lib/driver/image/bincue.c b/lib/driver/image/bincue.c > index f51beb4..68ed73c 100644 > --- a/lib/driver/image/bincue.c > +++ b/lib/driver/image/bincue.c > @@ -357,8 +357,8 @@ parse_cuefile (_img_private_t *cd, const char > *psz_cue_name) > } else if (0 == strcmp ("CDTEXTFILE", psz_keyword)) { > if(NULL != (psz_field = strtok (NULL, "\"\t\n\r"))) { > if (cd) { > - uint8_t *cdt_data = NULL; > - int size; > + uint8_t *cdt_data = NULL, *cdt_packs; > + int size, mmc_len; > CdioDataSource_t *source; > char *dirname = cdio_dirname(psz_cue_name); > char *psz_filename = cdio_abspath(dirname, psz_field); > @@ -394,9 +394,22 @@ parse_cuefile (_img_private_t *cd, const char > *psz_cue_name > ) > goto err_exit; > } > > - /* Truncate header when it is too large. */ > - if (cdt_data[0] > 0x80) { > - size -= 4; > + /* Check whether obviously a MMC header is prepended and if > so,skip it. > + cdtext_data_init() wants to see only the text pack bytes. > + */ > + cdt_packs = cdt_data; > + if (cdt_data[0] < 0x80) { > + /* This cannot be a text pack start */ > + mmc_len = CDIO_MMC_GET_LEN16(cdt_data) + 2; > + if ((size == mmc_len || size == mmc_len + 1) && mmc_len % 18 == > 4 && > + cdt_data[4] >= 0x80 && cdt_data[4] <= 0x8f) { > + /* It looks much like a MMC header followed by a text pack > start */ > + size -= 4; > + cdt_packs = cdt_data + 4; > + cdio_log (CDIO_LOG_INFO, > + "%s line %d: skipped 4 bytes of apparent MMC header > in CD-TEXT file `%s'\n", > + psz_cue_name, i_line, psz_filename); > + } > } > > /* ignore trailing 0 */ > @@ -408,7 +421,7 @@ parse_cuefile (_img_private_t *cd, const char > *psz_cue_name) > cd->gen.cdtext = cdtext_init (); > } > > - if(0 != cdtext_data_init(cd->gen.cdtext, cdt_data, size)) > + if(0 != cdtext_data_init(cd->gen.cdtext, cdt_packs, size)) > cdio_log (log_level, "%s line %d: failed to parse CD-TEXT file > `%s'", psz_cue_name, i_line, psz_filename); > > cdio_stdio_destroy (source); > > ========================================================================== > > Have a nice day :) > > Thomas > > >
