Re: [PATCH v4 3/9] supplicant: Do not wait for D-Bus reply if it's not used

[Patrik Flykt]

Hi,

On Wed, 2015-01-21 at 08:34 -0500, Hannu Mallat wrote:
 Don't wait for a reply from supplicant for D-Bus calls which don't
 have a callback function for processing the reply.

I think I'll have the same comment on this patch as on 4/9. Even though
the callback function is never set, for the overall call flow it seems
better to wait for the D-Bus method call to return - and do nothing with
it. Should someone need to add some code handling a particular result,
it will be easier to achieve that if a special condition need not be
remembered.

Cheers,

Patrik

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

[PATCH v4 3/9] supplicant: Do not wait for D-Bus reply if it's not used

[Hannu Mallat]

Don't wait for a reply from supplicant for D-Bus calls which don't
have a callback function for processing the reply.
---
 gsupplicant/dbus.c | 44 
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/gsupplicant/dbus.c b/gsupplicant/dbus.c
index 2957979..2d052a5 100644
--- a/gsupplicant/dbus.c
+++ b/gsupplicant/dbus.c
@@ -487,9 +487,10 @@ int supplicant_dbus_method_call(const char *path,
gpointer caller)
 {
struct method_call_data *method_call = NULL;
-   DBusMessage *message;
+   DBusMessage *message = NULL;
DBusMessageIter iter;
DBusPendingCall *call;
+   int result = 0;
 
if (!connection)
return -EINVAL;
@@ -497,16 +498,10 @@ int supplicant_dbus_method_call(const char *path,
if (!path || !interface || !method)
return -EINVAL;
 
-   method_call = g_try_new0(struct method_call_data, 1);
-   if (!method_call)
-   return -ENOMEM;
-
message = dbus_message_new_method_call(SUPPLICANT_SERVICE, path,
interface, method);
-   if (!message) {
-   g_free(method_call);
+   if (!message)
return -ENOMEM;
-   }
 
dbus_message_set_auto_start(message, FALSE);
 
@@ -514,17 +509,28 @@ int supplicant_dbus_method_call(const char *path,
if (setup)
setup(iter, user_data);
 
+   /* No need to wait for reply if there's no reply function */
+   if (!function) {
+   if (!dbus_connection_send(connection, message, NULL))
+   result = -ENOMEM;
+   goto cleanup;
+   }
+
+   method_call = g_try_new0(struct method_call_data, 1);
+   if (!method_call) {
+   result = -ENOMEM;
+   goto cleanup;
+   }
+
if (!dbus_connection_send_with_reply(connection, message,
call, TIMEOUT)) {
-   dbus_message_unref(message);
-   g_free(method_call);
-   return -EIO;
+   result = -ENOMEM;
+   goto cleanup;
}
 
if (!call) {
-   dbus_message_unref(message);
-   g_free(method_call);
-   return -EIO;
+   result = -EIO;
+   goto cleanup;
}
 
method_call-caller = caller;
@@ -536,9 +542,15 @@ int supplicant_dbus_method_call(const char *path,
dbus_pending_call_set_notify(call, method_call_reply, method_call,
method_call_free);
 
-   dbus_message_unref(message);
+   method_call = NULL;
 
-   return 0;
+cleanup:
+   if (message)
+   dbus_message_unref(message);
+
+   g_free(method_call);
+
+   return result;
 }
 
 void supplicant_dbus_property_append_basic(DBusMessageIter *iter,
-- 
1.9.1

___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman