Attachment 45233 Details for Bug 35767 – invoke_object_method: if marshalling an out argument fails, discard message

[patch] invoke_object_method: if marshalling an out argument fails, discard message

0005-invoke_object_method-if-marshalling-an-out-argument-.patch (text/plain), 3.64 KB, created by Simon McVittie on 2011-04-04 08:55:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-04-04 08:55:41 UTC

Size: 3.64 KB

patch

obsolete

>From 1e1d8069dbf8214223aa98d278412819e3a958a8 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Mon, 4 Apr 2011 16:52:58 +0100
>Subject: [PATCH 5/5] invoke_object_method: if marshalling an out argument fails, discard message
>
>We shouldn't report this as OOM, because it probably wasn't (it's much
>more likely to be programming error); we should also continue through the
>arguments, so that we don't leak them.
>
>By abandoning the message as soon as we detect a programming error,
>we can use reply == NULL as an indicator of whether to keep appending.
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=35767
>---
> dbus/dbus-gobject.c |   47 ++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 36 insertions(+), 11 deletions(-)
>
>diff --git a/dbus/dbus-gobject.c b/dbus/dbus-gobject.c
>index ab94957..2688fd2 100644
>--- a/dbus/dbus-gobject.c
>+++ b/dbus/dbus-gobject.c
>@@ -1770,9 +1770,9 @@ invoke_object_method (GObject         *object,
> 		      NULL, method->function);
>   if (is_async)
>     {
>-      result = DBUS_HANDLER_RESULT_HANDLED;
>       goto done;
>     }
>+
>   if (retval_signals_error)
>     had_error = _dbus_gvalue_signals_error (&return_value);
>   else
>@@ -1801,9 +1801,22 @@ invoke_object_method (GObject         *object,
> 
>       /* First, append the return value, unless it's synthetic */
>       if (have_retval && !retval_is_synthetic)
>-	{ 
>-	  if (send_reply && !_dbus_gvalue_marshal (&iter, &return_value))
>-	    goto nomem;
>+	{
>+          if (reply != NULL && !_dbus_gvalue_marshal (&iter, &return_value))
>+            {
>+              gchar *desc = g_strdup_value_contents (&return_value);
>+
>+              g_critical ("unable to append retval of type %s for %s: %s",
>+                  G_VALUE_TYPE_NAME (&return_value),
>+                  method_name_from_object_info (object_info, method),
>+                  desc);
>+              g_free (desc);
>+              /* the reply is now unusable but we still need to free
>+               * everything */
>+              dbus_message_unref (reply);
>+              reply = NULL;
>+            }
>+
> 	  if (!retval_is_constant)
> 	    g_value_unset (&return_value);
> 	}
>@@ -1853,9 +1866,22 @@ invoke_object_method (GObject         *object,
> 	      g_value_set_static_boxed (&gvalue, out_param_gvalues->values + out_param_gvalue_pos);
> 	      out_param_gvalue_pos++;
> 	    }
>-	      
>-	  if (send_reply && !_dbus_gvalue_marshal (&iter, &gvalue))
>-	    goto nomem;
>+
>+          if (reply && !_dbus_gvalue_marshal (&iter, &gvalue))
>+            {
>+              gchar *desc = g_strdup_value_contents (&gvalue);
>+
>+              g_critical ("unable to append OUT arg of type %s for %s: %s",
>+                  G_VALUE_TYPE_NAME (&gvalue),
>+                  method_name_from_object_info (object_info, method),
>+                  desc);
>+              g_free (desc);
>+              /* the reply is now unusable but we still need to free
>+               * everything */
>+              dbus_message_unref (reply);
>+              reply = NULL;
>+            }
>+
> 	  /* Here we actually free the allocated value; we
> 	   * took ownership of it with _dbus_gvalue_take, unless
> 	   * an annotation has specified this value as constant.
>@@ -1875,9 +1901,11 @@ invoke_object_method (GObject         *object,
>       dbus_message_unref (reply);
>     }
> 
>+done:
>   result = DBUS_HANDLER_RESULT_HANDLED;
>- done:
>+
>   g_free (in_signature);
>+
>   if (!is_async)
>     {
>       g_array_free (out_param_values, TRUE);
>@@ -1889,9 +1917,6 @@ invoke_object_method (GObject         *object,
> 
>   g_value_array_free (value_array);
>   return result;
>- nomem:
>-  result = DBUS_HANDLER_RESULT_NEED_MEMORY;
>-  goto done;
> }
> 
> static gboolean
>-- 
>1.7.4.1
>

Actions: View | Splinter Review

Attachments on bug 35767: 45220 | 45229 | 45230 | 45231 | 45232 | 45233 | 50325 | 50328

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.