[vlc-devel] [PATCH 1/2] player: fix unused var warning with NDEBUG

[Alexandre Janniaux]

Hi,

In that case, it might be better to explicitly mark the playlist
as unused, instead of relying on vlc_playlist_AssertLocked.

That way, a comment can even explain why it is not used while
being asked as a locked parameter in the function, and the intent
is clear.

Regards,
--
Alexandre Janniaux
Videolabs

On Wed, Sep 04, 2019 at 12:50:24PM +0200, Thomas Guillem wrote:
>
>
> On Wed, Sep 4, 2019, at 11:16, Alexandre Janniaux wrote:
> > Hi,
> >
> > As Steve, I think it shouldn't mark the variable as unused.
> > If you assert that player is locked but don't actually use it, it
> > should probably produce a warning for it.
>
> This is wrong for this case:
>
> void
> vlc_playlist_RemoveListener(vlc_playlist_t *playlist,
>                             vlc_playlist_listener_id *listener)
> {
>     vlc_playlist_AssertLocked(playlist);
>
>     vlc_list_remove(&listener->node);
>     free(listener);
> }
>
> The playlist is not touched but this code but be executed with the lock held.
>
> >
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Wed, Sep 04, 2019 at 09:14:13AM +0200, Thomas Guillem wrote:
> > > ---
> > >  src/player/player.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/src/player/player.h b/src/player/player.h
> > > index 36caa46299..60321cb8d0 100644
> > > --- a/src/player/player.h
> > > +++ b/src/player/player.h
> > > @@ -169,6 +169,7 @@ struct vlc_player_t
> > >      } destructor;
> > >  };
> > >
> > > +#ifndef NDEBUG
> > >  /*
> > >   * Assert that the player mutex is locked.
> > >   *
> > > @@ -181,6 +182,9 @@ vlc_player_assert_locked(vlc_player_t *player)
> > >      assert(player);
> > >      vlc_mutex_assert(&player->lock);
> > >  }
> > > +#else
> > > +#define vlc_player_assert_locked(x) VLC_UNUSED(x)
> > > +#endif
> > >
> > >  static inline struct vlc_player_input *
> > >  vlc_player_get_input_locked(vlc_player_t *player)
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel