[vlc-devel] [PATCH 1/1] Return DBus error message instead of infinite loop

[Jorge Bellon-Castro]

Hello again,
There is an earlier check doing the same goto. I can turn both into a
static function call, for example, if that is preferred.

See
https://code.videolan.org/videolan/vlc/-/blob/master/modules/control/dbus/dbus_tracklist.c#L131

Regards,
 Jorge

On Sun, 19 Apr 2020, 07:04 Rémi Denis-Courmont, <remi at remlab.net> wrote:

> Le sunnuntaina 19. huhtikuuta 2020, 2.35.53 EEST Jorge Bellon-Castro a
> écrit :
> > From: Jorge Bellon Castro <jbelloncastro at gmail.com>
> >
> > Avoid infinite loop and return DBus error message when handling
> > GetMetadata DBus function call containing invalid track ids.
> > ---
> >  modules/control/dbus/dbus_common.h    |  6 ++++++
> >  modules/control/dbus/dbus_tracklist.c | 15 +++++++++------
> >  2 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/modules/control/dbus/dbus_common.h
> > b/modules/control/dbus/dbus_common.h index 7cb82212bc..ee7d3a9d88 100644
> > --- a/modules/control/dbus/dbus_common.h
> > +++ b/modules/control/dbus/dbus_common.h
> > @@ -60,6 +60,12 @@
> >      dbus_message_unref( p_msg ); \
> >      return DBUS_HANDLER_RESULT_HANDLED
> >
> > +#define REPLY_ERROR( error, msg_format, ... ) \
> > +    dbus_message_unref( p_msg ); \
> > +    p_msg = dbus_message_new_error_printf( p_from, error, msg_format, \
> > +        __VA_ARGS__ ); \
> > +    if ( !p_msg ) return DBUS_HANDLER_RESULT_NEED_MEMORY;
> > +
> >  #define SIGNAL_INIT( interface, path, signal ) \
> >      DBusMessage *p_msg = dbus_message_new_signal( path, \
> >          interface, signal ); \
> > diff --git a/modules/control/dbus/dbus_tracklist.c
> > b/modules/control/dbus/dbus_tracklist.c index a2e4119c2b..1435bc5702
> 100644
> > --- a/modules/control/dbus/dbus_tracklist.c
> > +++ b/modules/control/dbus/dbus_tracklist.c
> > @@ -139,18 +139,21 @@ DBUS_METHOD( GetTracksMetadata )
> >          }
> >          vlc_playlist_Unlock(playlist);
> >          if (!id_valid)
> > -        {
> > -invalid_track_id:
> > -            msg_Err( (vlc_object_t*) p_this, "Invalid track id: %s",
> > -                                             psz_track_id );
> > -            continue;
> > -        }
> > +            goto invalid_track_id;
>
> If there's only one branch-to, you don't need to use goto.
>
> Otherwise, looks good.
>
> >
> >          dbus_message_iter_next( &track_ids );
> >      }
> >
> >      dbus_message_iter_close_container( &args, &meta );
> >      REPLY_SEND;
> > +
> > +invalid_track_id:
> > +    msg_Err( (vlc_object_t*) p_this, "Invalid track id: %s",
> > +            psz_track_id );
> > +    dbus_message_iter_abandon_container( &args, &meta );
> > +    REPLY_ERROR( DBUS_ERROR_UNKNOWN_OBJECT, "Invalid track id: %s",
> > +            psz_track_id );
> > +    REPLY_SEND;
> >  }
> >
> >  DBUS_METHOD( GoTo )
>
>
> --
> Rémi Denis-Courmont
> Tapiolan uusi kaupunki, Uudenmaan tasavalta
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200419/1be7a516/attachment.html>