[vlc-devel] [PATCH 02/11] demux: wav: refactor chunk skip

[Thomas Guillem]

On Thu, Mar 12, 2020, at 17:34, Remi Denis-Courmont wrote:
> Le 2020-03-12 16:05, Thomas Guillem a écrit :
> > ---
> >  modules/demux/wav.c | 19 +++++++++++++------
> >  1 file changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/modules/demux/wav.c b/modules/demux/wav.c
> > index c68f5e916eb..4f483df10c7 100644
> > --- a/modules/demux/wav.c
> > +++ b/modules/demux/wav.c
> > @@ -116,6 +116,17 @@ static int Control( demux_t *p_demux, int
> > i_query, va_list args )
> >                                     i_query, args );
> >  }
> > 
> > +static int ChunkSkip( demux_t *p_demux, size_t i_size )
> > +{
> > +    if( vlc_stream_Read( p_demux->s, NULL, i_size ) != (ssize_t)i_size 
> > )
> > +        return VLC_EGENERIC;
> 
> Can you really assume that i_size <= SSIZE_MAX here even on 32-bits 
> platforms?
> 
> Also i_size += i_size & 1; would simplify.
> 

Thanks for your comment.

Is something like that OK ?

static int ChunkSkip( demux_t *p_demux, uint32_t i_size )
{
    ssize_t i_ret;
    i_size += i_size & 1;

#if (UINT32_MAX > SSIZE_MAX)
    if( unlikely( i_size > SSIZE_MAX ) )
    {
        i_ret = vlc_stream_Read( p_demux->s, NULL, SSIZE_MAX );
        if( i_ret < 0 || i_ret != SSIZE_MAX )
            return VLC_EGENERIC;
        i_size -= SSIZE_MAX;
        assert( i_size <= SSIZE_MAX );
    }
#endif

    i_ret = vlc_stream_Read( p_demux->s, NULL, i_size );
    return i_ret < 0 || i_ret != i_size ? VLC_EGENERIC : VLC_SUCCESS;
}

uint32_t instead of size_t since value come directly from GetDWLE()
I won't handle uint64_t skip since the 64bit data will never be skipped

> > +
> > +    if( (i_size & 1) && vlc_stream_Read( p_demux->s, NULL, 1 ) != 1 )
> > +        return VLC_EGENERIC;
> > +
> > +    return VLC_SUCCESS;
> > +}
> > +
> >  static int ChunkFind( demux_t *p_demux, const char *fcc, unsigned int
> > *pi_size )
> >  {
> >      const uint8_t *p_peek;
> > @@ -143,10 +154,7 @@ static int ChunkFind( demux_t *p_demux, const
> > char *fcc, unsigned int *pi_size )
> >              return VLC_SUCCESS;
> >          }
> > 
> > -        /* Skip chunk */
> > -        if( vlc_stream_Read( p_demux->s, NULL, 8 ) != 8 ||
> > -            vlc_stream_Read( p_demux->s, NULL, i_size ) != (int)i_size 
> > ||
> > -            ( (i_size & 1) && vlc_stream_Read( p_demux->s, NULL, 1 ) 
> > != 1 ) )
> > +        if( ChunkSkip( p_demux, i_size + 8 ) != VLC_SUCCESS )
> >              return VLC_EGENERIC;
> >      }
> >  }
> > @@ -302,8 +310,7 @@ static int Open( vlc_object_t * p_this )
> >              p_sys->i_data_size = (int64_t)1 << 62;
> >          else
> >              p_sys->i_data_size = i_data_size;
> > -        if( vlc_stream_Read( p_demux->s, NULL, i_size ) != (int)i_size 
> > ||
> > -            ( (i_size & 1) && vlc_stream_Read( p_demux->s, NULL, 1 ) 
> > != 1 ) )
> > +        if( ChunkSkip( p_demux, i_size ) != VLC_SUCCESS )
> >              goto error;
> >      }
> 
> -- 
> Rémi Denis-Courmont
>