[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libcdio-devel] libcdio freeze for 0.80 next weekend
|
From: |
Eric Shattow |
|
Subject: |
Re: [Libcdio-devel] libcdio freeze for 0.80 next weekend |
|
Date: |
Tue, 11 Mar 2008 21:36:27 -0500 |
I agree that 0.80 being the next release should be constrained to bug fixes
and security patches only. There's nothing to prevent a follow-up release,
license changes can wait.
On Mon, Mar 10, 2008 at 9:08 PM, R. Bernstein <address@hidden> wrote:
> Thanks for the patch. Here are my thoughts. I welcome other thoughts
> and comments.
>
> The main motivation for this release is to clear out bug fixes and
> possible buffer overflows of the kind security people get excited
> about. (In practice though I think the impact pretty small, but
> I'm no security expert.)
>
> (That said, it is however true that there have been some minor
> feature enhancements like outputing cd-paranoia status information to
> file.)
>
> It was also proposed to convert the libcdio license to GPL v3, but
> that was postponed because it was felt that it unfair to bundle bug
> and security fixes with a license change.
>
> Adding get_track_lba is an API change; also possibly an ABI change
> since the cdio_funcs_t structure now grows to add one more function
> (internal) function pointer.
>
> At a minimum, we'll have to change libcdio_la_CURRENT, REVISION and
> AGE. Nicholas Boullis is more of an expert on this here and so I hope
> he will correct me here. The sense I get though is that because
> libcdio is, well, a *library*, these kinds of changes require other
> packages to get redone. So again it might be best to do the securty
> fix and follow with a relase to change the API (and ABI?).
>
> And while on the topic of good things to add, probably a couple of
> regression tests for the bin/cue and nrg image files would be nice.
>
> As for when the release after next is, that's entirely up how folks
> feel about this. The next release after 0.80 could be for example
> coordinated with the next cued release.
>
> Thanks again.
>
> Robert William Fuller writes:
> > R. Bernstein wrote:
> > > In preparation for a release next Saturday March 15, I guess a
> feature
> > > freeze is in effect for libcdio. (Not there was all that much
> activity
> > > anyway.)
> >
> > Hmm. Please consider the following patch for inclusion in spite of the
> > freeze. Unlike the prior patches, it has been carefully considered,
> and
> > the next release of cued depends on it.
> >
> > Sorry about all the spam with the prior patch attempt. I had broken my
> > finger working in the yard, and I was a bit out of sorts. I did not
> > realize my finger was broken until I went to the emergency care and got
> > some X-rays. Anyway, you won't see 5 more patches in quick succession!
> >
> > This patch adds pregap support for Nero images and bincue. It has been
> > carefully tested. I did not add support for CDRDAO because I do not
> > have any CDRDAO images. I plan to add it when I need it.
> >
> > Thank you!
> >
> > Rob
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/include/cdio/track.h libcdio-0.80/include/cdio/track.h
> > --- libcdio-0.80-clean/include/cdio/track.h 2006-01-23 15:30:
> 28.000000000 -0500
> > +++ libcdio-0.80/include/cdio/track.h 2008-03-10 16:15:
> 32.000000000 -0400
> > @@ -196,6 +196,28 @@ extern "C" {
> > @return the starting LSN or CDIO_INVALID_LSN on error.
> > */
> > lsn_t cdio_get_track_lsn(const CdIo_t *p_cdio, track_t i_track);
> > +
> > + /*!
> > + Return the starting LBA for the pregap for track number
> > + i_track in p_cdio. Track numbers usually start at something
> > + greater than 0, usually 1.
> > +
> > + @param p_cdio object to get information from
> > + @param i_track the track number we want the LBA for
> > + @return the starting LBA or CDIO_INVALID_LBA on error.
> > + */
> > + lba_t cdio_get_track_pregap_lba(const CdIo_t *p_cdio, track_t
> i_track);
> > +
> > + /*!
> > + Return the starting LSN for the pregap for track number
> > + i_track in p_cdio. Track numbers usually start at something
> > + greater than 0, usually 1.
> > +
> > + @param p_cdio object to get information from
> > + @param i_track the track number we want the LSN for
> > + @return the starting LSN or CDIO_INVALID_LSN on error.
> > + */
> > + lsn_t cdio_get_track_pregap_lsn(const CdIo_t *p_cdio, track_t
> i_track);
> >
> > /*!
> > Return the starting MSF (minutes/secs/frames) for track number
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/cdio_private.h libcdio-0.80
> /lib/driver/cdio_private.h
> > --- libcdio-0.80-clean/lib/driver/cdio_private.h 2006-01-23 15:48:
> 11.000000000 -0500
> > +++ libcdio-0.80/lib/driver/cdio_private.h 2008-03-09 21:23:
> 04.000000000 -0400
> > @@ -260,6 +260,13 @@ extern "C" {
> > CDIO_INVALID_LBA is returned on error.
> > */
> > lba_t (*get_track_lba) ( void *p_env, track_t i_track );
> > +
> > + /*!
> > + Return the starting LBA for the pregap for track number
> > + i_track in p_env. Tracks numbers start at 1.
> > + CDIO_INVALID_LBA is returned on error.
> > + */
> > + lba_t (*get_track_pregap_lba) ( const void *p_env, track_t i_track
> );
> >
> > /*!
> > Get format of track.
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/bincue.c libcdio-0.80
> /lib/driver/image/bincue.c
> > --- libcdio-0.80-clean/lib/driver/image/bincue.c 2006-02-13 06:00:
> 53.000000000 -0500
> > +++ libcdio-0.80/lib/driver/image/bincue.c 2008-03-10 17:28:
> 43.000000000 -0400
> > @@ -632,7 +632,7 @@ parse_cuefile (_img_private_t *cd, const
> > goto err_exit;
> > }
> > if (cd) {
> > - cd->tocent[i].pregap = lba;
> > + cd->tocent[i].silence = lba;
> > }
> > } else {
> > goto format_error;
> > @@ -672,7 +672,14 @@ parse_cuefile (_img_private_t *cd, const
> > track_info_t *this_track=
> > &(cd->tocent[cd->gen.i_tracks - cd->gen.i_first_track]);
> >
> > - if (start_index != 0) {
> > + switch (start_index) {
> > +
> > + case 0:
> > + lba += CDIO_PREGAP_SECTORS;
> > + this_track->pregap = lba;
> > + break;
> > +
> > + case 1:
> > if (!b_first_index_for_track) {
> > lba += CDIO_PREGAP_SECTORS;
> > cdio_lba_to_msf(lba, &(this_track->start_msf));
> > @@ -709,6 +716,10 @@ parse_cuefile (_img_private_t *cd, const
> > }
> > }
> > this_track->num_indices++;
> > + break;
> > +
> > + default:
> > + break;
> > }
> > }
> > #endif
> > @@ -1163,6 +1174,7 @@ cdio_open_cue (const char *psz_cue_name)
> > _funcs.get_track_lba = _get_lba_track_bincue;
> > _funcs.get_track_msf = _get_track_msf_image;
> > _funcs.get_track_preemphasis = get_track_preemphasis_image,
> > + _funcs.get_track_pregap_lba = get_track_pregap_lba_image;
> > _funcs.lseek = _lseek_bincue;
> > _funcs.read = _read_bincue;
> > _funcs.read_audio_sectors = _read_audio_sectors_bincue;
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/cdrdao.c libcdio-0.80
> /lib/driver/image/cdrdao.c
> > --- libcdio-0.80-clean/lib/driver/image/cdrdao.c 2007-03-05 06:49:
> 24.000000000 -0500
> > +++ libcdio-0.80/lib/driver/image/cdrdao.c 2008-03-10 14:50:
> 45.000000000 -0400
> > @@ -843,7 +843,7 @@ parse_tocfile (_img_private_t *cd, const
> > if (0 <= i) {
> > if (NULL != (psz_field = strtok (NULL, " \t\n\r"))) {
> > if (NULL != cd)
> > - cd->tocent[i].pregap = cdio_mmssff_to_lba (psz_field);
> > + cd->tocent[i].silence = cdio_mmssff_to_lba (psz_field);
> > } else {
> > goto format_error;
> > }
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/nrg.c libcdio-0.80
> /lib/driver/image/nrg.c
> > --- libcdio-0.80-clean/lib/driver/image/nrg.c 2008-03-03 07:04:
> 34.000000000 -0500
> > +++ libcdio-0.80/lib/driver/image/nrg.c 2008-03-10 15:23:
> 42.000000000 -0400
> > @@ -371,24 +371,24 @@ parse_nrg (_img_private_t *p_env, const
> > case DAOX_ID: /* "DAOX" */
> > case DAOI_ID: /* "DAOI" */
> > {
> > + _daox_t *_xentries = NULL;
> > + _daoi_t *_ientries = NULL;
> > + _dao_common_t *_dao_common = (void *) chunk->data;
> > + int disc_mode = _dao_common->unknown[1];
> > track_format_t track_format;
> > - int disc_mode;
> > + int i;
> >
> > /* We include an extra 0 byte so these can be used as C
> strings.*/
> > - p_env->psz_mcn = calloc(1, CDIO_MCN_SIZE+1);
> > + p_env->psz_mcn = calloc(1, CDIO_MCN_SIZE+1);
> > + memcpy(p_env->psz_mcn, &(_dao_common->psz_mcn), CDIO_MCN_SIZE);
> > + p_env->psz_mcn[CDIO_MCN_SIZE] = '\0';
> >
> > if (DAOX_ID == opcode) {
> > - _daox_array_t *_entries = (void *) chunk->data;
> > - disc_mode = _entries->_unknown[1];
> > - p_env->dtyp = _entries->_unknown[19];
> > - memcpy(p_env->psz_mcn, &(_entries->psz_mcn), CDIO_MCN_SIZE);
> > - p_env->psz_mcn[CDIO_MCN_SIZE] = '\0';
> > + _xentries = (void *) chunk->data;
> > + p_env->dtyp = _xentries->track_info[0].common.unknown[0];
> > } else {
> > - _daoi_array_t *_entries = (void *) chunk->data;
> > - disc_mode = _entries->_unknown[1];
> > - p_env->dtyp = _entries->_unknown[19];
> > - memcpy(p_env->psz_mcn, &(_entries->psz_mcn), CDIO_MCN_SIZE);
> > - p_env->psz_mcn[CDIO_MCN_SIZE] = '\0';
> > + _ientries = (void *) chunk->data;
> > + p_env->dtyp = _ientries->track_info[0].common.unknown[0];
> > }
> >
> > p_env->is_dao = true;
> > @@ -430,7 +430,6 @@ parse_nrg (_img_private_t *p_env, const
> > track_format = TRACK_FORMAT_AUDIO;
> > }
> > if (0 == disc_mode) {
> > - int i;
> > for (i=0; i<p_env->gen.i_tracks; i++) {
> > cdtext_init (&(p_env->gen.cdtext_track[i]));
> > p_env->tocent[i].track_format= track_format;
> > @@ -446,7 +445,6 @@ parse_nrg (_img_private_t *p_env, const
> > }
> > }
> > } else if (2 == disc_mode) {
> > - int i;
> > for (i=0; i<p_env->gen.i_tracks; i++) {
> > cdtext_init (&(p_env->gen.cdtext_track[i]));
> > p_env->tocent[i].track_green = true;
> > @@ -473,6 +471,18 @@ parse_nrg (_img_private_t *p_env, const
> > "Don't know if mode 1, mode 2 or mixed: %x\n",
> > disc_mode);
> > }
> > +
> > + for (i=0; i<p_env->gen.i_tracks; i++) {
> > + if (!p_env->tocent[i].datasize) {
> > + continue;
> > + }
> > + if (DAOX_ID == opcode) {
> > + p_env->tocent[i].pregap =
> (uint64_from_be(_xentries->track_info[i].index0)) /
> (p_env->tocent[i].datasize);
> > + } else {
> > + p_env->tocent[i].pregap =
> (uint32_from_be(_ientries->track_info[i].index0)) /
> (p_env->tocent[i].datasize);
> > + }
> > + }
> > +
> > break;
> > }
> > case NERO_ID:
> > @@ -1243,13 +1253,14 @@ cdio_open_nrg (const char *psz_source)
> > _funcs.get_media_changed = get_media_changed_image;
> > _funcs.get_mcn = _get_mcn_image;
> > _funcs.get_num_tracks = _get_num_tracks_image;
> > - _funcs.get_track_channels = get_track_channels_generic,
> > - _funcs.get_track_copy_permit = get_track_copy_permit_image,
> > + _funcs.get_track_channels = get_track_channels_generic;
> > + _funcs.get_track_copy_permit = get_track_copy_permit_image;
> > _funcs.get_track_format = get_track_format_nrg;
> > _funcs.get_track_green = _get_track_green_nrg;
> > _funcs.get_track_lba = NULL; /* Will use generic routine via
> msf */
> > _funcs.get_track_msf = _get_track_msf_image;
> > - _funcs.get_track_preemphasis = get_track_preemphasis_generic,
> > + _funcs.get_track_preemphasis = get_track_preemphasis_generic;
> > + _funcs.get_track_pregap_lba = get_track_pregap_lba_image;
> > _funcs.lseek = _lseek_nrg;
> > _funcs.read = _read_nrg;
> > _funcs.read_audio_sectors = _read_audio_sectors_nrg;
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image/nrg.h libcdio-0.80
> /lib/driver/image/nrg.h
> > --- libcdio-0.80-clean/lib/driver/image/nrg.h 2006-01-14 04:45:
> 44.000000000 -0500
> > +++ libcdio-0.80/lib/driver/image/nrg.h 2008-03-08 23:16:
> 21.000000000 -0500
> > @@ -71,19 +71,48 @@ typedef struct {
> > uint32_t lsn GNUC_PACKED;
> > } _cuex_array_t;
> >
> > +/* New DAO[XI] Information from
> http://en.wikipedia.org/wiki/NRG_(file_format)<http://en.wikipedia.org/wiki/NRG_%28file_format%29>
> > +*/
> > +
> > typedef struct {
> > - uint32_t _unknown1 GNUC_PACKED;
> > - char psz_mcn[CDIO_MCN_SIZE];
> > - uint8_t _unknown[64-CDIO_MCN_SIZE-sizeof(uint32_t)];
> > + uint8_t zero[10];
> > + uint32_t sector_size GNUC_PACKED;
> > + uint8_t unknown[4];
> > +} _dao_array_common_t;
> > +
> > +typedef struct {
> > + _dao_array_common_t common;
> > + uint64_t index0 GNUC_PACKED;
> > + uint64_t index1 GNUC_PACKED;
> > + uint64_t end_of_track GNUC_PACKED;
> > } _daox_array_t;
> >
> > typedef struct {
> > - uint32_t _unknown1 GNUC_PACKED;
> > - char psz_mcn[CDIO_MCN_SIZE];
> > - uint8_t _unknown[64-CDIO_MCN_SIZE-sizeof(uint32_t)];
> > + _dao_array_common_t common;
> > + uint32_t index0 GNUC_PACKED;
> > + uint32_t index1 GNUC_PACKED;
> > + uint32_t end_of_track GNUC_PACKED;
> > } _daoi_array_t;
> >
> > typedef struct {
> > + uint32_t chunk_size_le GNUC_PACKED;
> > + char psz_mcn[CDIO_MCN_SIZE];
> > + uint8_t unknown[3];
> > + uint8_t first_track;
> > + uint8_t last_track;
> > +} _dao_common_t;
> > +
> > +typedef struct {
> > + _dao_common_t common;
> > + _daox_array_t track_info[EMPTY_ARRAY_SIZE];
> > +} _daox_t;
> > +
> > +typedef struct {
> > + _dao_common_t common;
> > + _daoi_array_t track_info[EMPTY_ARRAY_SIZE];
> > +} _daoi_t;
> > +
> > +typedef struct {
> > uint32_t id GNUC_PACKED;
> > uint32_t len GNUC_PACKED;
> > char data[EMPTY_ARRAY_SIZE];
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image.h libcdio-0.80/lib/driver/image.h
> > --- libcdio-0.80-clean/lib/driver/image.h 2005-02-16 23:57:
> 21.000000000 -0500
> > +++ libcdio-0.80/lib/driver/image.h 2008-03-10 14:50:01.000000000-0400
> > @@ -48,7 +48,8 @@ typedef struct {
> > msf_t start_msf;
> > lba_t start_lba;
> > int start_index;
> > - lba_t pregap; /**< pre-gap with zero audio data */
> > + lba_t pregap; /**< pre-gap */
> > + lba_t silence; /**< pre-gap with zero audio data */
> > int sec_count; /**< Number of sectors in this track.
> Does not
> > include pregap */
> > int num_indices;
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image_common.c libcdio-0.80
> /lib/driver/image_common.c
> > --- libcdio-0.80-clean/lib/driver/image_common.c 2005-02-17 02:03:
> 37.000000000 -0500
> > +++ libcdio-0.80/lib/driver/image_common.c 2008-03-10 15:34:
> 08.000000000 -0400
> > @@ -246,6 +246,30 @@ get_track_preemphasis_image(const void *
> > & PRE_EMPHASIS ) ? CDIO_TRACK_FLAG_TRUE :
> CDIO_TRACK_FLAG_FALSE;
> > }
> >
> > +/*! Return the starting LBA for the pregap for track number i_track.
> > + Track numbers start at 1.
> > + CDIO_INVALID_LBA is returned on error.
> > +*/
> > +lba_t
> > +get_track_pregap_lba_image(const void *p_user_data, track_t i_track)
> > +{
> > + const _img_private_t *p_env = p_user_data;
> > + lba_t pregap, start_lba;
> > +
> > + pregap = p_env->tocent[i_track-p_env->gen.i_first_track].pregap;
> > + start_lba = p_env->tocent[i_track-p_env->gen.i_first_track
> ].start_lba;
> > +
> > + /* avoid initializing pregap to CDIO_INVALID_LBA by letting calloc
> > + do the work. also, nero files have the pregap set equal
> > + to the start of the track when there is no pregap
> > + */
> > + if (!pregap || pregap == start_lba) {
> > + pregap = CDIO_INVALID_LBA;
> > + }
> > +
> > + return pregap;
> > +}
> > +
> > /*!
> > Read a data sector
> >
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/image_common.h libcdio-0.80
> /lib/driver/image_common.h
> > --- libcdio-0.80-clean/lib/driver/image_common.h 2005-02-17 02:03:
> 37.000000000 -0500
> > +++ libcdio-0.80/lib/driver/image_common.h 2008-03-09 21:03:
> 31.000000000 -0400
> > @@ -150,6 +150,13 @@ track_flag_t get_track_copy_permit_image
> > */
> > track_flag_t get_track_preemphasis_image(const void *p_user_data,
> > track_t i_track);
> > +
> > +/*! Return the starting LBA for the pregap for track number i_track.
> > + Track numbers start at 1.
> > + CDIO_INVALID_LBA is returned on error.
> > +*/
> > +lba_t get_track_pregap_lba_image(const void *p_user_data, track_t
> i_track);
> > +
> > /*!
> > Read a data sector
> >
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/libcdio.sym libcdio-0.80
> /lib/driver/libcdio.sym
> > --- libcdio-0.80-clean/lib/driver/libcdio.sym 2007-12-15 17:35:
> 53.000000000 -0500
> > +++ libcdio-0.80/lib/driver/libcdio.sym 2008-03-10 16:17:
> 27.000000000 -0400
> > @@ -80,6 +80,8 @@ cdio_get_track_format
> > cdio_get_track_green
> > cdio_get_track_last_lsn
> > cdio_get_track_lba
> > +cdio_get_track_pregap_lba
> > +cdio_get_track_pregap_lsn
> > cdio_get_track_lsn
> > cdio_get_track_msf
> > cdio_get_track_preemphasis
> > diff -upr -x version.h -x 'libiso9660*pc' -x 'libcdio*pc'
> libcdio-0.80-clean/lib/driver/track.c libcdio-0.80/lib/driver/track.c
> > --- libcdio-0.80-clean/lib/driver/track.c 2005-02-05 23:20:
> 25.000000000 -0500
> > +++ libcdio-0.80/lib/driver/track.c 2008-03-10 16:16:54.000000000-0400
> > @@ -229,7 +229,7 @@ cdio_get_track_lba(const CdIo_t *p_cdio,
> > i_track in cdio. Tracks numbers start at 1.
> > The "leadout" track is specified either by
> > using i_track LEADOUT_TRACK or the total tracks+1.
> > - CDIO_INVALID_LBA is returned on error.
> > + CDIO_INVALID_LSN is returned on error.
> > */
> > lsn_t
> > cdio_get_track_lsn(const CdIo_t *p_cdio, track_t i_track)
> > @@ -247,6 +247,34 @@ cdio_get_track_lsn(const CdIo_t *p_cdio,
> > }
> >
> > /*!
> > + Return the starting LBA for the pregap for track number
> > + i_track in cdio. Track numbers start at 1.
> > + CDIO_INVALID_LBA is returned on error.
> > +*/
> > +lba_t
> > +cdio_get_track_pregap_lba(const CdIo_t *p_cdio, track_t i_track)
> > +{
> > + if (p_cdio == NULL) return CDIO_INVALID_LBA;
> > +
> > + if (p_cdio->op.get_track_pregap_lba) {
> > + return p_cdio->op.get_track_pregap_lba (p_cdio->env, i_track);
> > + } else {
> > + return CDIO_INVALID_LBA;
> > + }
> > +}
> > +
> > +/*!
> > + Return the starting LSN for the pregap for track number
> > + i_track in cdio. Track numbers start at 1.
> > + CDIO_INVALID_LSN is returned on error.
> > +*/
> > +lsn_t
> > +cdio_get_track_pregap_lsn(const CdIo_t *p_cdio, track_t i_track)
> > +{
> > + return cdio_lba_to_lsn(cdio_get_track_pregap_lba(p_cdio, i_track));
> > +}
> > +
> > +/*!
> > Return the ending LSN for track number
> > i_track in cdio. CDIO_INVALID_LSN is returned on error.
> > */
>
>
>
- [Libcdio-devel] here's a patch that changes nothing..., (continued)
- [Libcdio-devel] here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., Robert William Fuller, 2008/03/08
- [Libcdio-devel] Re: here's a patch that changes nothing..., R. Bernstein, 2008/03/09
Re: [Libcdio-devel] libcdio freeze for 0.80 next weekend, Robert William Fuller, 2008/03/10
[Libcdio-devel] proposed pre-gap patch, Robert William Fuller, 2008/03/13
[Libcdio-devel] unexpected offset in bincue.c for audio sectors, Robert William Fuller, 2008/03/17