It hasn't been used in production and is very likely it will never be.
> @@ -401,14 +283,13 @@ get_properties_reply_cb (GObject *source, > > if (!check_query_reply_msg (reply_msg, NULL)) > { > - const gchar *buddy; > + GError err = { TP_ERRORS, TP_ERROR_NETWORK_ERROR, > + "Server returned an error" }; > > - buddy = lm_message_node_get_attribute ( > - wocky_stanza_get_top_node (reply_msg), "from"); > - g_assert (buddy != NULL); > + DEBUG ("Query failed: %s", error->message); > > - DEBUG ("PEP query failed. Let's try to search this buddy."); > - get_buddy_properties_from_search (ctx->conn, buddy, ctx->context); > + dbus_g_method_return_error (ctx->context, &err); > + g_error_free (error); > goto out; Fairly serious copypasta error: @error isn't set, so the DEBUG() is a null pointer dereference! The second argument of check_query_reply_msg is in fact a DBusGMethodInvocation, so I think you can streamline this to: if (!check_query_reply_msg (reply_msg, ctx->context)) { DEBUG ("Query failed"); goto out; } > @@ -446,18 +326,6 @@ olpc_buddy_info_get_properties (GabbleSvcOLPCBuddyInfo *iface, ... > - /* First check if we can find properties in a buddy view */ ... > /* Then try to query the PEP node */ The second comment doesn't make sense without the first; delete it.
(In reply to comment #1) > > @@ -401,14 +283,13 @@ get_properties_reply_cb (GObject *source, > > > > if (!check_query_reply_msg (reply_msg, NULL)) > > { > > - const gchar *buddy; > > + GError err = { TP_ERRORS, TP_ERROR_NETWORK_ERROR, > > + "Server returned an error" }; > > > > - buddy = lm_message_node_get_attribute ( > > - wocky_stanza_get_top_node (reply_msg), "from"); > > - g_assert (buddy != NULL); > > + DEBUG ("Query failed: %s", error->message); > > > > - DEBUG ("PEP query failed. Let's try to search this buddy."); > > - get_buddy_properties_from_search (ctx->conn, buddy, ctx->context); > > + dbus_g_method_return_error (ctx->context, &err); > > + g_error_free (error); > > goto out; > > Fairly serious copypasta error: @error isn't set, so the DEBUG() is a null > pointer dereference! > > The second argument of check_query_reply_msg is in fact a > DBusGMethodInvocation, so I think you can streamline this to: > > if (!check_query_reply_msg (reply_msg, ctx->context)) > { > DEBUG ("Query failed"); > goto out; > } True, check_query_reply_msg already calls dbus_g_method_return_error and as it also logs the error I'm omitting it from the caller. > > @@ -446,18 +326,6 @@ olpc_buddy_info_get_properties (GabbleSvcOLPCBuddyInfo *iface, > ... > > - /* First check if we can find properties in a buddy view */ > ... > > /* Then try to query the PEP node */ > > The second comment doesn't make sense without the first; delete it. Done.
review+, although it would have been easier to review if you hadn't squashed the fixes into the initial patch (I assume you didn't make any other changes than the ones I suggested).
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.