<div dir="auto">Hello again,<div dir="auto">There is an earlier check doing the same goto. I can turn both into a static function call, for example, if that is preferred.</div><div dir="auto"><br></div><div dir="auto">See <a href="https://code.videolan.org/videolan/vlc/-/blob/master/modules/control/dbus/dbus_tracklist.c#L131">https://code.videolan.org/videolan/vlc/-/blob/master/modules/control/dbus/dbus_tracklist.c#L131</a><br></div><div dir="auto"><br></div><div dir="auto">Regards,</div><div dir="auto"> Jorge</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 19 Apr 2020, 07:04 Rémi Denis-Courmont, <<a href="mailto:remi@remlab.net">remi@remlab.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Le sunnuntaina 19. huhtikuuta 2020, 2.35.53 EEST Jorge Bellon-Castro a écrit :<br>
> From: Jorge Bellon Castro <<a href="mailto:jbelloncastro@gmail.com" target="_blank" rel="noreferrer">jbelloncastro@gmail.com</a>><br>
> <br>
> Avoid infinite loop and return DBus error message when handling<br>
> GetMetadata DBus function call containing invalid track ids.<br>
> ---<br>
>  modules/control/dbus/dbus_common.h    |  6 ++++++<br>
>  modules/control/dbus/dbus_tracklist.c | 15 +++++++++------<br>
>  2 files changed, 15 insertions(+), 6 deletions(-)<br>
> <br>
> diff --git a/modules/control/dbus/dbus_common.h<br>
> b/modules/control/dbus/dbus_common.h index 7cb82212bc..ee7d3a9d88 100644<br>
> --- a/modules/control/dbus/dbus_common.h<br>
> +++ b/modules/control/dbus/dbus_common.h<br>
> @@ -60,6 +60,12 @@<br>
>      dbus_message_unref( p_msg ); \<br>
>      return DBUS_HANDLER_RESULT_HANDLED<br>
> <br>
> +#define REPLY_ERROR( error, msg_format, ... ) \<br>
> +    dbus_message_unref( p_msg ); \<br>
> +    p_msg = dbus_message_new_error_printf( p_from, error, msg_format, \<br>
> +        __VA_ARGS__ ); \<br>
> +    if ( !p_msg ) return DBUS_HANDLER_RESULT_NEED_MEMORY;<br>
> +<br>
>  #define SIGNAL_INIT( interface, path, signal ) \<br>
>      DBusMessage *p_msg = dbus_message_new_signal( path, \<br>
>          interface, signal ); \<br>
> diff --git a/modules/control/dbus/dbus_tracklist.c<br>
> b/modules/control/dbus/dbus_tracklist.c index a2e4119c2b..1435bc5702 100644<br>
> --- a/modules/control/dbus/dbus_tracklist.c<br>
> +++ b/modules/control/dbus/dbus_tracklist.c<br>
> @@ -139,18 +139,21 @@ DBUS_METHOD( GetTracksMetadata )<br>
>          }<br>
>          vlc_playlist_Unlock(playlist);<br>
>          if (!id_valid)<br>
> -        {<br>
> -invalid_track_id:<br>
> -            msg_Err( (vlc_object_t*) p_this, "Invalid track id: %s",<br>
> -                                             psz_track_id );<br>
> -            continue;<br>
> -        }<br>
> +            goto invalid_track_id;<br>
<br>
If there's only one branch-to, you don't need to use goto.<br>
<br>
Otherwise, looks good.<br>
<br>
> <br>
>          dbus_message_iter_next( &track_ids );<br>
>      }<br>
> <br>
>      dbus_message_iter_close_container( &args, &meta );<br>
>      REPLY_SEND;<br>
> +<br>
> +invalid_track_id:<br>
> +    msg_Err( (vlc_object_t*) p_this, "Invalid track id: %s",<br>
> +            psz_track_id );<br>
> +    dbus_message_iter_abandon_container( &args, &meta );<br>
> +    REPLY_ERROR( DBUS_ERROR_UNKNOWN_OBJECT, "Invalid track id: %s",<br>
> +            psz_track_id );<br>
> +    REPLY_SEND;<br>
>  }<br>
> <br>
>  DBUS_METHOD( GoTo )<br>
<br>
<br>
-- <br>
Rémi Denis-Courmont<br>
Tapiolan uusi kaupunki, Uudenmaan tasavalta<br>
<br>
<br>
<br>
</blockquote></div>