[vlc-devel] [PATCH 1/2] Fix unused variable compilation warnings
Eduardo Vieira
eduardovra at gmail.com
Mon Dec 8 13:16:27 CET 2014
Hi,
Thanks for the comments.
2014-12-08 7:30 GMT-02:00 Rémi Denis-Courmont <remi at remlab.net>:
> Le 2014-12-08 04:06, Eduardo Vieira a écrit :
>>
>> ---
>> modules/access/zip/zipaccess.c | 1 +
>> modules/access/zip/zipstream.c | 1 +
>> modules/lua/demux.c | 1 +
>> modules/packetizer/h264.c | 1 -
>> src/input/es_out.c | 3 +++
>> src/input/es_out.h | 9 +++++++++
>> src/misc/picture.c | 1 +
>> src/video_output/display.c | 1 +
>> 8 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/modules/access/zip/zipaccess.c
>> b/modules/access/zip/zipaccess.c
>> index 2f2b4dc..24064a0 100644
>> --- a/modules/access/zip/zipaccess.c
>> +++ b/modules/access/zip/zipaccess.c
>> @@ -414,6 +414,7 @@ static uLong ZCALLBACK ZipIO_Write( void* opaque,
>> void* stream,
>> (void)opaque; (void)stream; (void)buf; (void)size;
>> int zip_access_cannot_write_this_should_not_happen = 0;
>> assert(zip_access_cannot_write_this_should_not_happen);
>> + VLC_UNUSED( zip_access_cannot_write_this_should_not_happen );
>> return 0;
>
>
> The defacto VLC style in such case is "assert(0)", which would solve the
> problem nicely. However, I suspect this code was meant to stay as its
> upstream to make future backport easier...
I looked it up, and it seems the code is this way from the first commit.
Can I keep the VLC_UNUSED call ?
>
>> }
>>
>> diff --git a/modules/access/zip/zipstream.c
>> b/modules/access/zip/zipstream.c
>> index ac29c7a..fa7a396 100644
>> --- a/modules/access/zip/zipstream.c
>> +++ b/modules/access/zip/zipstream.c
>> @@ -842,6 +842,7 @@ static uLong ZCALLBACK ZipIO_Write( void* opaque,
>> void* stream,
>> (void)opaque; (void)stream; (void)buf; (void)size;
>> int ERROR_zip_cannot_write_this_should_not_happen = 0;
>> assert( ERROR_zip_cannot_write_this_should_not_happen );
>> + VLC_UNUSED( ERROR_zip_cannot_write_this_should_not_happen );
>
>
> Ditto.
Not directly related but, do you know if the zip and rar access
modules will be replaced by the archive module (which uses libarchive)
?
It was unclear if these modules was written to do the same things.
>
>> return 0;
>> }
>>
>> diff --git a/modules/lua/demux.c b/modules/lua/demux.c
>> index eac4f24..f9c167b 100644
>> --- a/modules/lua/demux.c
>> +++ b/modules/lua/demux.c
>> @@ -75,6 +75,7 @@ static int vlclua_demux_read( lua_State *L )
>> lua_pushlstring( L, (const char *)p_read, i_read );
>> int i_seek = stream_Read( p_demux->s, NULL, i_read );
>> assert( i_read == i_seek );
>> + VLC_UNUSED( i_seek );
>
>
> This assertion is wrong. stream_Read() can fail at any time. So this patch
> is wrong.
The patch is not showing, but there's a call to stream_Peek right
above (in which i_read is set), and the return value is checked to be
> 0. Do you know if Read can fail even in this case ?
>
>> }
>> else
>> lua_pushnil( L );
>> diff --git a/modules/packetizer/h264.c b/modules/packetizer/h264.c
>> index c029171..76f7d76 100644
>> --- a/modules/packetizer/h264.c
>> +++ b/modules/packetizer/h264.c
>> @@ -556,7 +556,6 @@ static block_t *CreateAnnexbNAL( decoder_t
>> *p_dec, const uint8_t *p, int i_size
>> static void CreateDecodedNAL( uint8_t **pp_ret, int *pi_ret,
>> const uint8_t *src, int i_src )
>> {
>> - const uint8_t *end = &src[i_src];
>
>
> That seems like a slightly different problem than the rest of the patch.
Indeed. In this case, the variable was actually not used, and was not
within any ifdef or commented code block, so I thought it would be
safe to erase it.
>
>> uint8_t *dst = malloc( i_src );
>>
>> *pp_ret = dst;
>> diff --git a/src/input/es_out.c b/src/input/es_out.c
>> index 29fee99..ceada38 100644
>> --- a/src/input/es_out.c
>> +++ b/src/input/es_out.c
>> @@ -2565,6 +2565,7 @@ static int EsOutControlLocked( es_out_t *out,
>> int i_query, va_list args )
>> const mtime_t i_date = (mtime_t) va_arg( args, mtime_t );
>>
>> assert( !b_source_paused == !b_paused );
>> + VLC_UNUSED( b_source_paused );
>
>
> I'm not sure we can rely on void-casting variables to mute warnings. The
> variable is still pointless and some compilers or compiler versions may
> still be unhappy either way. That's why this issue and the ones below were
> never fixed.
There is an alternative solution by marking the variables using the
__attribute__((unused)). But I think it's GCC specific, so it will not
solve the problems you said.
I proposed solving the warnings this way because it seemed to be how
they were being handled:
https://mailman.videolan.org/pipermail/vlc-devel/2013-November/095500.html
>
>
>> EsOutChangePause( out, b_paused, i_date );
>>
>> return VLC_SUCCESS;
>> @@ -2576,6 +2577,7 @@ static int EsOutControlLocked( es_out_t *out,
>> int i_query, va_list args )
>> const int i_rate = (int)va_arg( args, int );
>>
>> assert( i_src_rate == i_rate );
>> + VLC_UNUSED( i_src_rate );
>> EsOutChangeRate( out, i_rate );
>>
>> return VLC_SUCCESS;
>> @@ -2586,6 +2588,7 @@ static int EsOutControlLocked( es_out_t *out,
>> int i_query, va_list args )
>> const mtime_t i_date = (mtime_t)va_arg( args, mtime_t );
>>
>> assert( i_date == -1 );
>> + VLC_UNUSED( i_date );
>> EsOutChangePosition( out );
>>
>> return VLC_SUCCESS;
>> diff --git a/src/input/es_out.h b/src/input/es_out.h
>> index 56cefed..21ec606 100644
>> --- a/src/input/es_out.h
>> +++ b/src/input/es_out.h
>> @@ -88,6 +88,7 @@ static inline void es_out_SetMode( es_out_t *p_out,
>> int i_mode )
>> {
>> int i_ret = es_out_Control( p_out, ES_OUT_SET_MODE, i_mode );
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> }
>> static inline mtime_t es_out_GetWakeup( es_out_t *p_out )
>> {
>> @@ -95,6 +96,7 @@ static inline mtime_t es_out_GetWakeup( es_out_t *p_out
>> )
>> int i_ret = es_out_Control( p_out, ES_OUT_GET_WAKE_UP, &i_wu );
>>
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> return i_wu;
>> }
>> static inline bool es_out_GetBuffering( es_out_t *p_out )
>> @@ -103,6 +105,7 @@ static inline bool es_out_GetBuffering( es_out_t
>> *p_out )
>> int i_ret = es_out_Control( p_out, ES_OUT_GET_BUFFERING, &b );
>>
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> return b;
>> }
>> static inline bool es_out_GetEmpty( es_out_t *p_out )
>> @@ -111,12 +114,14 @@ static inline bool es_out_GetEmpty( es_out_t *p_out
>> )
>> int i_ret = es_out_Control( p_out, ES_OUT_GET_EMPTY, &b );
>>
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> return b;
>> }
>> static inline void es_out_SetDelay( es_out_t *p_out, int i_cat,
>> mtime_t i_delay )
>> {
>> int i_ret = es_out_Control( p_out, ES_OUT_SET_DELAY, i_cat, i_delay
>> );
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> }
>> static inline int es_out_SetRecordState( es_out_t *p_out, bool b_record )
>> {
>> @@ -142,6 +147,7 @@ static inline void es_out_SetTimes( es_out_t
>> *p_out, double f_position, mtime_t
>> {
>> int i_ret = es_out_Control( p_out, ES_OUT_SET_TIMES, f_position,
>> i_time, i_length );
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> }
>> static inline void es_out_SetJitter( es_out_t *p_out,
>> mtime_t i_pts_delay, mtime_t
>> i_pts_jitter, int i_cr_average )
>> @@ -149,6 +155,7 @@ static inline void es_out_SetJitter( es_out_t *p_out,
>> int i_ret = es_out_Control( p_out, ES_OUT_SET_JITTER,
>> i_pts_delay, i_pts_jitter, i_cr_average
>> );
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> }
>> static inline int es_out_GetEsObjects( es_out_t *p_out, int i_id,
>> vlc_object_t **pp_decoder,
>> vout_thread_t **pp_vout, audio_output_t **pp_aout )
>> @@ -160,12 +167,14 @@ static inline int es_out_GetGroupForced(
>> es_out_t *p_out )
>> int i_group;
>> int i_ret = es_out_Control( p_out, ES_OUT_GET_GROUP_FORCED, &i_group
>> );
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> return i_group;
>> }
>> static inline void es_out_Eos( es_out_t *p_out )
>> {
>> int i_ret = es_out_Control( p_out, ES_OUT_SET_EOS );
>> assert( !i_ret );
>> + VLC_UNUSED( i_ret );
>> }
>>
>> es_out_t *input_EsOutNew( input_thread_t *, int i_rate );
>> diff --git a/src/misc/picture.c b/src/misc/picture.c
>> index 6aa5b05..b3908b7 100644
>> --- a/src/misc/picture.c
>> +++ b/src/misc/picture.c
>> @@ -286,6 +286,7 @@ picture_t *picture_Hold( picture_t *p_picture )
>> {
>> uintptr_t refs = atomic_fetch_add( &p_picture->gc.refcount, 1 );
>> assert( refs > 0 );
>> + VLC_UNUSED( refs );
>> return p_picture;
>> }
>>
>> diff --git a/src/video_output/display.c b/src/video_output/display.c
>> index 4c1b133..ec77b12 100644
>> --- a/src/video_output/display.c
>> +++ b/src/video_output/display.c
>> @@ -56,6 +56,7 @@ static picture_t *VideoBufferNew(filter_t *filter)
>> assert(vd->fmt.i_chroma == fmt->i_chroma &&
>> vd->fmt.i_width == fmt->i_width &&
>> vd->fmt.i_height == fmt->i_height);
>> + VLC_UNUSED( fmt );
>>
>> picture_pool_t *pool = vout_display_Pool(vd, 3);
>> if (!pool)
>
>
> --
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list