[vlc-devel] [PATCH 2/6] Standardise property marshalling for PropertiesChanged signal

Alex Merry dev at randomguy3.me.uk
Tue Jan 29 01:55:19 CET 2013


Re-use the AddProperty method (used in GetAllProperties) and use macros
to reduce repetition of code.  Now all the calls should be
error-checked.
---
 modules/control/dbus/dbus_player.c | 188 ++++++++++++++++---------------------
 modules/control/dbus/dbus_root.c   |  78 +++++++++------
 2 files changed, 129 insertions(+), 137 deletions(-)

diff --git a/modules/control/dbus/dbus_player.c b/modules/control/dbus/dbus_player.c
index 5b197f0..2f5afc2 100644
--- a/modules/control/dbus/dbus_player.c
+++ b/modules/control/dbus/dbus_player.c
@@ -39,7 +39,7 @@
 #include "dbus_player.h"
 #include "dbus_common.h"
 
-static void MarshalLoopStatus ( intf_thread_t *, DBusMessageIter * );
+static int MarshalLoopStatus ( intf_thread_t *, DBusMessageIter * );
 
 DBUS_METHOD( Position )
 { /* returns position in microseconds */
@@ -165,7 +165,7 @@ DBUS_METHOD( Seek )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalVolume( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     float f_vol = playlist_VolumeGet( p_intf->p_sys->p_playlist );
@@ -173,7 +173,10 @@ MarshalVolume( intf_thread_t *p_intf, DBusMessageIter *container )
         f_vol = 1.f; /* ? */
 
     double d_vol = f_vol;
-    dbus_message_iter_append_basic( container, DBUS_TYPE_DOUBLE, &d_vol );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_DOUBLE, &d_vol ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( VolumeGet )
@@ -302,7 +305,7 @@ DBUS_METHOD( OpenUri )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalCanPlay( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     playlist_t *p_playlist = p_intf->p_sys->p_playlist;
@@ -311,7 +314,10 @@ MarshalCanPlay( intf_thread_t *p_intf, DBusMessageIter *container )
     dbus_bool_t b_can_play = playlist_CurrentSize( p_playlist ) > 0;
     PL_UNLOCK;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_can_play );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_can_play ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( CanPlay )
@@ -332,7 +338,7 @@ DBUS_METHOD( CanPlay )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalCanPause( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     dbus_bool_t b_can_pause = FALSE;
@@ -345,8 +351,11 @@ MarshalCanPause( intf_thread_t *p_intf, DBusMessageIter *container )
         vlc_object_release( p_input );
     }
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN,
-                                    &b_can_pause );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN,
+                                         &b_can_pause ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( CanPause )
@@ -391,7 +400,7 @@ DBUS_METHOD( CanControl )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalCanSeek( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     dbus_bool_t b_can_seek = FALSE;
@@ -404,7 +413,10 @@ MarshalCanSeek( intf_thread_t *p_intf, DBusMessageIter *container )
         vlc_object_release( p_input );
     }
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_can_seek );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_can_seek ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( CanSeek )
@@ -425,11 +437,14 @@ DBUS_METHOD( CanSeek )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalShuffle( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     dbus_bool_t b_shuffle = var_GetBool( p_intf->p_sys->p_playlist, "random" );
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_shuffle );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_shuffle ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( ShuffleGet )
@@ -463,7 +478,7 @@ DBUS_METHOD( ShuffleSet )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalPlaybackStatus( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     input_thread_t *p_input;
@@ -489,8 +504,11 @@ MarshalPlaybackStatus( intf_thread_t *p_intf, DBusMessageIter *container )
     else
         psz_playback_status = PLAYBACK_STATUS_STOPPED;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_STRING,
-                                    &psz_playback_status );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_STRING,
+                                         &psz_playback_status ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( PlaybackStatus )
@@ -511,7 +529,7 @@ DBUS_METHOD( PlaybackStatus )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalRate( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     double d_rate;
@@ -524,7 +542,10 @@ MarshalRate( intf_thread_t *p_intf, DBusMessageIter *container )
     else
         d_rate = 1.0;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_DOUBLE, &d_rate );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_DOUBLE, &d_rate ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( RateGet )
@@ -610,7 +631,7 @@ DBUS_METHOD( MaximumRate )
     REPLY_SEND;
 }
 
-static void
+static int
 MarshalLoopStatus( intf_thread_t *p_intf, DBusMessageIter *container )
 {
     const char *psz_loop_status;
@@ -624,8 +645,11 @@ MarshalLoopStatus( intf_thread_t *p_intf, DBusMessageIter *container )
     else
         psz_loop_status = LOOP_STATUS_NONE;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_STRING,
-                                    &psz_loop_status );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_STRING,
+                                         &psz_loop_status ))
+        return VLC_ENOMEM;
+
+    return VLC_SUCCESS;
 }
 
 DBUS_METHOD( LoopStatusGet )
@@ -868,6 +892,16 @@ int SeekedEmit( intf_thread_t * p_intf )
     return VLC_SUCCESS;
 }
 
+#define PROPERTY_MAPPING_BEGIN if( 0 ) {}
+#define PROPERTY_ENTRY( prop, signature ) \
+    else if( !strcmp( ppsz_properties[i], #prop ) ) \
+    { \
+        if( VLC_SUCCESS != AddProperty( (intf_thread_t*) p_intf, \
+                    &changed_properties, #prop, signature, Marshal##prop ) ) \
+            return DBUS_HANDLER_RESULT_NEED_MEMORY; \
+    }
+#define PROPERTY_MAPPING_END else { return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; }
+
 /**
  * PropertiesChangedSignal() synthetizes and sends the
  * org.freedesktop.DBus.Properties.PropertiesChanged signal
@@ -877,7 +911,7 @@ PropertiesChangedSignal( intf_thread_t    *p_intf,
                          vlc_dictionary_t *p_changed_properties )
 {
     DBusConnection  *p_conn = p_intf->p_sys->p_conn;
-    DBusMessageIter changed_properties, invalidated_properties, entry, variant;
+    DBusMessageIter changed_properties, invalidated_properties;
     const char *psz_interface_name = DBUS_MPRIS_PLAYER_INTERFACE;
     char **ppsz_properties = NULL;
     int i_properties = 0;
@@ -888,106 +922,48 @@ PropertiesChangedSignal( intf_thread_t    *p_intf,
 
     OUT_ARGUMENTS;
     ADD_STRING( &psz_interface_name );
-    dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "{sv}",
-                                      &changed_properties );
+
+    if (!dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "{sv}",
+                                           &changed_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
 
     i_properties = vlc_dictionary_keys_count( p_changed_properties );
     ppsz_properties = vlc_dictionary_all_keys( p_changed_properties );
 
     for( int i = 0; i < i_properties; i++ )
     {
-        dbus_message_iter_open_container( &changed_properties,
-                                          DBUS_TYPE_DICT_ENTRY, NULL,
-                                          &entry );
-
-        dbus_message_iter_append_basic( &entry, DBUS_TYPE_STRING,
-                                        &ppsz_properties[i] );
+        PROPERTY_MAPPING_BEGIN
+        PROPERTY_ENTRY( Metadata,       "a{sv}" )
+        PROPERTY_ENTRY( PlaybackStatus, "s"     )
+        PROPERTY_ENTRY( LoopStatus,     "s"     )
+        PROPERTY_ENTRY( Rate,           "d"     )
+        PROPERTY_ENTRY( Shuffle,        "b"     )
+        PROPERTY_ENTRY( Volume,         "d"     )
+        PROPERTY_ENTRY( CanSeek,        "b"     )
+        PROPERTY_ENTRY( CanPlay,        "b"     )
+        PROPERTY_ENTRY( CanPause,       "b"     )
+        PROPERTY_MAPPING_END
 
-        if( !strcmp( ppsz_properties[i], "Metadata" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "a{sv}",
-                                              &variant );
-            MarshalMetadata( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "PlaybackStatus" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "s",
-                                              &variant );
-            MarshalPlaybackStatus( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "LoopStatus" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "s",
-                                              &variant );
-            MarshalLoopStatus( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "Rate" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "d",
-                                              &variant );
-            MarshalRate( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "Shuffle" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "b",
-                                              &variant );
-            MarshalShuffle( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "Volume" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "d",
-                                              &variant );
-            MarshalVolume( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "CanSeek" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "b",
-                                              &variant );
-            MarshalCanSeek( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "CanPlay" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "b",
-                                              &variant );
-            MarshalCanPlay( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        else if( !strcmp( ppsz_properties[i], "CanPause" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "b",
-                                              &variant );
-            MarshalCanPause( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-        dbus_message_iter_close_container( &changed_properties, &entry );
         free( ppsz_properties[i] );
     }
 
-    dbus_message_iter_close_container( &args, &changed_properties );
-    dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "s",
-                                      &invalidated_properties );
-    dbus_message_iter_close_container( &args, &invalidated_properties );
+    if (!dbus_message_iter_close_container( &args, &changed_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
+    if (!dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "s",
+                                           &invalidated_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
+    if (!dbus_message_iter_close_container( &args, &invalidated_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
+
     free( ppsz_properties );
 
     SIGNAL_SEND;
 }
 
+#undef PROPERTY_MAPPING_BEGIN
+#undef PROPERTY_ADD
+#undef PROPERTY_MAPPING_END
+
 /*****************************************************************************
  * PropertiesChangedEmit: Emits the Seeked signal
  *****************************************************************************/
diff --git a/modules/control/dbus/dbus_root.c b/modules/control/dbus/dbus_root.c
index 3bcb36e..4a0ca4f 100644
--- a/modules/control/dbus/dbus_root.c
+++ b/modules/control/dbus/dbus_root.c
@@ -80,7 +80,9 @@ MarshalIdentity( intf_thread_t *p_intf, DBusMessageIter *container )
     VLC_UNUSED( p_intf );
     const char *psz_id = _("VLC media player");
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_STRING, &psz_id );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_STRING, &psz_id ))
+        return VLC_ENOMEM;
+
     return VLC_SUCCESS;
 }
 
@@ -119,7 +121,9 @@ MarshalCanSetFullscreen( intf_thread_t *p_intf, DBusMessageIter *container )
         }
     }
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret ))
+        return VLC_ENOMEM;
+
     return VLC_SUCCESS;
 }
 
@@ -149,8 +153,10 @@ MarshalFullscreen( intf_thread_t *p_intf, DBusMessageIter *container )
     else
         b_fullscreen = FALSE;
 
-    dbus_message_iter_append_basic( container,
-            DBUS_TYPE_BOOLEAN, &b_fullscreen );
+    if (!dbus_message_iter_append_basic( container,
+            DBUS_TYPE_BOOLEAN, &b_fullscreen ))
+        return VLC_ENOMEM;
+
     return VLC_SUCCESS;
 }
 
@@ -202,7 +208,9 @@ MarshalCanQuit( intf_thread_t *p_intf, DBusMessageIter *container )
     VLC_UNUSED( p_intf );
     const dbus_bool_t b_ret = TRUE;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret ))
+        return VLC_ENOMEM;
+
     return VLC_SUCCESS;
 }
 
@@ -228,7 +236,9 @@ MarshalCanRaise( intf_thread_t *p_intf, DBusMessageIter *container )
     VLC_UNUSED( p_intf );
     const dbus_bool_t b_ret = FALSE;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret ))
+        return VLC_ENOMEM;
+
     return VLC_SUCCESS;
 }
 
@@ -254,7 +264,9 @@ MarshalHasTrackList( intf_thread_t *p_intf, DBusMessageIter *container )
     VLC_UNUSED( p_intf );
     const dbus_bool_t b_ret = FALSE;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_BOOLEAN, &b_ret ))
+        return VLC_ENOMEM;
+
     return VLC_SUCCESS;
 }
 
@@ -280,7 +292,9 @@ MarshalDesktopEntry( intf_thread_t *p_intf, DBusMessageIter *container )
     VLC_UNUSED( p_intf );
     const char* psz_ret = PACKAGE;
 
-    dbus_message_iter_append_basic( container, DBUS_TYPE_STRING, &psz_ret );
+    if (!dbus_message_iter_append_basic( container, DBUS_TYPE_STRING, &psz_ret ))
+        return VLC_ENOMEM;
+
     return VLC_SUCCESS;
 }
 
@@ -555,12 +569,22 @@ handle_root ( DBusConnection *p_conn, DBusMessage *p_from, void *p_this )
  * org.freedesktop.DBus.Properties.PropertiesChanged signal
  */
 
+#define PROPERTY_MAPPING_BEGIN if( 0 ) {}
+#define PROPERTY_ENTRY( prop, signature ) \
+    else if( !strcmp( ppsz_properties[i], #prop ) ) \
+    { \
+        if( VLC_SUCCESS != AddProperty( (intf_thread_t*) p_intf, \
+                    &changed_properties, #prop, signature, Marshal##prop ) ) \
+            return DBUS_HANDLER_RESULT_NEED_MEMORY; \
+    }
+#define PROPERTY_MAPPING_END else { return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; }
+
 static DBusHandlerResult
 PropertiesChangedSignal( intf_thread_t    *p_intf,
                          vlc_dictionary_t *p_changed_properties )
 {
     DBusConnection  *p_conn = p_intf->p_sys->p_conn;
-    DBusMessageIter changed_properties, invalidated_properties, entry, variant;
+    DBusMessageIter changed_properties, invalidated_properties;
     const char *psz_interface_name = DBUS_MPRIS_ROOT_INTERFACE;
     char **ppsz_properties = NULL;
     int i_properties = 0;
@@ -571,40 +595,32 @@ PropertiesChangedSignal( intf_thread_t    *p_intf,
 
     OUT_ARGUMENTS;
     ADD_STRING( &psz_interface_name );
-    dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "{sv}",
-                                      &changed_properties );
+
+    if (!dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "{sv}",
+                                           &changed_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
 
     i_properties = vlc_dictionary_keys_count( p_changed_properties );
     ppsz_properties = vlc_dictionary_all_keys( p_changed_properties );
 
     for( int i = 0; i < i_properties; i++ )
     {
-        dbus_message_iter_open_container( &changed_properties,
-                                          DBUS_TYPE_DICT_ENTRY, NULL,
-                                          &entry );
-
-        dbus_message_iter_append_basic( &entry, DBUS_TYPE_STRING,
-                                        &ppsz_properties[i] );
+        PROPERTY_MAPPING_BEGIN
+        PROPERTY_ENTRY( Fullscreen, "b" )
+        PROPERTY_MAPPING_END
 
-        if( !strcmp( ppsz_properties[i], "Fullscreen" ) )
-        {
-            dbus_message_iter_open_container( &entry,
-                                              DBUS_TYPE_VARIANT, "b",
-                                              &variant );
-            MarshalFullscreen( p_intf, &variant );
-            dbus_message_iter_close_container( &entry, &variant );
-        }
-
-        dbus_message_iter_close_container( &changed_properties, &entry );
         free( ppsz_properties[i] );
     }
 
-    dbus_message_iter_close_container( &args, &changed_properties );
+    if (!dbus_message_iter_close_container( &args, &changed_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
 
-    dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "s",
-                                      &invalidated_properties );
+    if (!dbus_message_iter_open_container( &args, DBUS_TYPE_ARRAY, "s",
+                                           &invalidated_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
+    if (!dbus_message_iter_close_container( &args, &invalidated_properties ))
+        return DBUS_HANDLER_RESULT_NEED_MEMORY;
 
-    dbus_message_iter_close_container( &args, &invalidated_properties );
     free( ppsz_properties );
 
     SIGNAL_SEND;
-- 
1.8.1.1




More information about the vlc-devel mailing list