2013-08-19 14:32:50

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 0/8] Fix message order

From: Luiz Augusto von Dentz <[email protected]>

This set fixes a couple of issues introduced with the use of ObjectManager due
the processing of signals on idle to group changes, this has been causing
issues with external components such as ofono and pulseaudio that sometimes
receives connections from devices objects that have pending signals thus
cannot be properly processed or have to be assumed paired and with certain
UUIDs which is not ideal.

Luiz Augusto von Dentz (8):
gdbus: Fix not maintaining message order for signals
gdbus: Add g_dbus_send_message_with_reply
gdbus: Avoid calling dbus_connection_send*
gdbus: Fix emitting PropertiesChanged twice
core: Make use of g_dbus_send_message_with_reply
neard: Make use of g_dbus_send_message_with_reply
audio/media: Make use of g_dbus_send_message_with_reply
obexd: Make use of g_dbus_send_message*

gdbus/client.c | 14 ++---
gdbus/gdbus.h | 3 +
gdbus/object.c | 143 ++++++++++++++++++++++++------------------
obexd/plugins/bluetooth.c | 2 +-
obexd/plugins/pcsuite.c | 5 +-
obexd/plugins/syncevolution.c | 6 +-
obexd/src/manager.c | 3 +-
plugins/neard.c | 2 +-
profiles/audio/media.c | 2 +-
src/agent.c | 12 ++--
src/profile.c | 4 +-
11 files changed, 109 insertions(+), 87 deletions(-)

--
1.8.3.1



2013-08-20 12:28:04

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] build: Fix not checking readline.h for bluetooth-player

Hi,

On Mon, Aug 19, 2013 at 5:32 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> If --disable-client is passed to configure script is possible to build
> BlueZ without readline which would cause the following error:
>
> tools/bluetooth-player.c:36:31: fatal error: readline/readline.h: No such file or directory
> #include <readline/readline.h>
> ^
> ---
> Makefile.tools | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile.tools b/Makefile.tools
> index 3bb0e82..4bcb18e 100644
> --- a/Makefile.tools
> +++ b/Makefile.tools
> @@ -172,8 +172,7 @@ if EXPERIMENTAL
> noinst_PROGRAMS += tools/bdaddr tools/avinfo tools/avtest tools/scotest \
> tools/hcieventmask tools/hcisecfilter tools/hwdb \
> tools/btmgmt tools/btattach tools/btsnoop \
> - tools/btiotest tools/mpris-player \
> - tools/bluetooth-player
> + tools/btiotest tools/mpris-player
>
> tools_bdaddr_SOURCES = tools/bdaddr.c src/oui.h src/oui.c
> tools_bdaddr_LDADD = lib/libbluetooth-internal.la @UDEV_LIBS@
> @@ -199,17 +198,13 @@ tools_btiotest_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
> tools_mpris_player_SOURCES = tools/mpris-player.c
> tools_mpris_player_LDADD = gdbus/libgdbus-internal.la @GLIB_LIBS@ @DBUS_LIBS@
>
> -tools_bluetooth_player_SOURCES = tools/bluetooth-player.c \
> - client/display.h client/display.c
> -tools_bluetooth_player_LDADD = gdbus/libgdbus-internal.la \
> - @GLIB_LIBS@ @DBUS_LIBS@ -lreadline
> -
> EXTRA_DIST += tools/bdaddr.1
> endif
>
> if READLINE
> noinst_PROGRAMS += attrib/gatttool \
> - tools/obex-client-tool tools/obex-server-tool
> + tools/obex-client-tool tools/obex-server-tool \
> + tools/bluetooth-player
>
> attrib_gatttool_SOURCES = attrib/gatttool.c attrib/att.c attrib/gatt.c \
> attrib/gattrib.c btio/btio.c \
> @@ -226,6 +221,11 @@ tools_obex_client_tool_LDADD = lib/libbluetooth-internal.la \
> tools_obex_server_tool_SOURCES = $(gobex_sources) $(btio_sources) \
> tools/obex-server-tool.c
> tools_obex_server_tool_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
> +
> +tools_bluetooth_player_SOURCES = tools/bluetooth-player.c \
> + client/display.h client/display.c
> +tools_bluetooth_player_LDADD = gdbus/libgdbus-internal.la \
> + @GLIB_LIBS@ @DBUS_LIBS@ -lreadline
> endif
>
> if EXPERIMENTAL
> --
> 1.8.3.1

I went ahead and applied this one.


--
Luiz Augusto von Dentz

2013-08-19 23:59:12

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/8] gdbus: Fix not maintaining message order for signals

On Mon, Aug 19, 2013 at 11:32 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> In some cases the order of the messages is altered when a message is
> sent without processing the pending signals first, currently this affect

Here and in every commit message: I think it should be made clear that
we are talking about properties and object manager signals, not
generic signals. Other signals are not affected by the out-of-order
issue.


> client_check_order unit test:
>
> /gdbus/client_check_order: **
> ERROR:unit/test-gdbus-client.c:795:property_check_order: assertion failed: (g_strcmp0(string, "value1") == 0)
>
> As can be observed the value of the property is not yet updated because the
> signal it is still pending, once this fix is applied the test pass:
>
> /gdbus/client_check_order: OK
>
> Note that the flushing only works when g_dbus_send_message is used so
> places where dbus_connection_send and other variants are called directly
> may still change the order.
> ---
> gdbus/object.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 2f8ef45..b2ca1c2 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -1494,6 +1494,35 @@ DBusMessage *g_dbus_create_reply(DBusMessage *message, int type, ...)
> return reply;
> }
>
> +static void g_dbus_flush_object(struct generic_data *data)
> +{
> + GSList *l;
> +
> + if (data->process_id > 0) {
> + g_source_remove(data->process_id);
> + data->process_id = 0;
> + }
> +
> + process_changes(data);
> +
> + for (l = data->objects; l; l = l->next)
> + g_dbus_flush_object(l->data);

wouldn't it be better swap these 2 so we flush the pending messages
from children first, then from the parent? Then we can even guarantee
this ordering by moving this code to process_changes().

> +}
> +
> +static void g_dbus_flush(DBusConnection *connection)
> +{
> + static gboolean flushing = FALSE;

this static var makes no sense with multiple connections, does it?


> +
> + if (flushing || root == NULL || root->conn != connection)
> + return;
> +
> + flushing = TRUE;
> +
> + g_dbus_flush_object(root);
> +
> + flushing = FALSE;
> +}
> +
> gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
> {
> dbus_bool_t result = FALSE;
> @@ -1510,6 +1539,9 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
> goto out;
> }
>
> + /* Flush pending signal to guarantee message order */
> + g_dbus_flush(connection);
> +
> result = dbus_connection_send(connection, message, NULL);
>
> out:
> --

Otherwise looks good.

Lucas De Marchi

2013-08-19 14:32:58

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 7/8] audio/media: Make use of g_dbus_send_message_with_reply

From: Luiz Augusto von Dentz <[email protected]>

This replaces dbus_connection_send_with_reply with
g_dbus_send_message_with_reply which does not alter message order.
---
profiles/audio/media.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 5a06b99..9c72b8d 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -325,7 +325,7 @@ static gboolean media_endpoint_async_call(DBusMessage *msg,
request = g_new0(struct endpoint_request, 1);

/* Timeout should be less than avdtp request timeout (4 seconds) */
- if (dbus_connection_send_with_reply(btd_get_dbus_connection(),
+ if (g_dbus_send_message_with_reply(btd_get_dbus_connection(),
msg, &request->call,
REQUEST_TIMEOUT) == FALSE) {
error("D-Bus send failed");
--
1.8.3.1


2013-08-19 14:32:59

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 8/8] obexd: Make use of g_dbus_send_message*

From: Luiz Augusto von Dentz <[email protected]>

This replaces dbus_connection_send* with g_dbus_send_message* which do
not alter message order.
---
obexd/plugins/bluetooth.c | 2 +-
obexd/plugins/pcsuite.c | 5 ++---
obexd/plugins/syncevolution.c | 6 +++---
obexd/src/manager.c | 3 +--
4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/obexd/plugins/bluetooth.c b/obexd/plugins/bluetooth.c
index e4d518e..10f5e4f 100644
--- a/obexd/plugins/bluetooth.c
+++ b/obexd/plugins/bluetooth.c
@@ -317,7 +317,7 @@ static int register_profile(struct bluetooth_profile *profile)
}
dbus_message_iter_close_container(&iter, &opt);

- if (!dbus_connection_send_with_reply(connection, msg, &call, -1)) {
+ if (!g_dbus_send_message_with_reply(connection, msg, &call, -1)) {
ret = -1;
unregister_profile(profile);
goto failed;
diff --git a/obexd/plugins/pcsuite.c b/obexd/plugins/pcsuite.c
index 4ce2fe4..6460aa9 100644
--- a/obexd/plugins/pcsuite.c
+++ b/obexd/plugins/pcsuite.c
@@ -351,7 +351,7 @@ static gboolean send_backup_dbus_message(const char *oper,
DBUS_TYPE_INVALID);

if (strcmp(oper, "open") == 0) {
- ret = dbus_connection_send_with_reply(conn, msg, &pending_call,
+ ret = g_dbus_send_message_with_reply(conn, msg, &pending_call,
BACKUP_DBUS_TIMEOUT);
dbus_message_unref(msg);
if (ret) {
@@ -363,8 +363,7 @@ static gboolean send_backup_dbus_message(const char *oper,
} else
dbus_connection_unref(conn);
} else {
- ret = dbus_connection_send(conn, msg, NULL);
- dbus_message_unref(msg);
+ g_dbus_send_message(conn, msg);
dbus_connection_unref(conn);
}

diff --git a/obexd/plugins/syncevolution.c b/obexd/plugins/syncevolution.c
index 2d25d28..edc00c8 100644
--- a/obexd/plugins/syncevolution.c
+++ b/obexd/plugins/syncevolution.c
@@ -314,7 +314,7 @@ static int synce_close(void *object)
dbus_message_append_args(msg, DBUS_TYPE_BOOLEAN, &normal,
DBUS_TYPE_STRING, &error, DBUS_TYPE_INVALID);

- dbus_connection_send_with_reply(context->dbus_conn, msg, &call, -1);
+ g_dbus_send_message_with_reply(context->dbus_conn, msg, &call, -1);
dbus_pending_call_set_notify(call, close_cb, NULL, NULL);
dbus_message_unref(msg);
dbus_pending_call_unref(call);
@@ -380,7 +380,7 @@ static ssize_t synce_read(void *object, void *buf, size_t count)
dbus_message_append_args(msg, DBUS_TYPE_BOOLEAN, &authenticate,
DBUS_TYPE_STRING, &session, DBUS_TYPE_INVALID);

- if (!dbus_connection_send_with_reply(conn, msg, &call, -1)) {
+ if (!g_dbus_send_message_with_reply(conn, msg, &call, -1)) {
error("D-Bus call to %s failed.", SYNCE_SERVER_INTERFACE);
dbus_message_unref(msg);
return -EPERM;
@@ -424,7 +424,7 @@ static ssize_t synce_write(void *object, const void *buf, size_t count)
dbus_message_append_args(msg, DBUS_TYPE_STRING, &type,
DBUS_TYPE_INVALID);

- if (!dbus_connection_send_with_reply(context->dbus_conn, msg,
+ if (!g_dbus_send_message_with_reply(context->dbus_conn, msg,
&call, -1)) {
error("D-Bus call to %s failed.", SYNCE_CONN_INTERFACE);
dbus_message_unref(msg);
diff --git a/obexd/src/manager.c b/obexd/src/manager.c
index dbfbef8..f64b7b9 100644
--- a/obexd/src/manager.c
+++ b/obexd/src/manager.c
@@ -776,8 +776,7 @@ int manager_request_authorization(struct obex_transfer *transfer, int32_t time,
dbus_message_append_args(msg, DBUS_TYPE_OBJECT_PATH, &transfer->path,
DBUS_TYPE_INVALID);

- if (!dbus_connection_send_with_reply(connection,
- msg, &call, TIMEOUT)) {
+ if (!g_dbus_send_message_with_reply(connection, msg, &call, TIMEOUT)) {
dbus_message_unref(msg);
return -EPERM;
}
--
1.8.3.1


2013-08-19 14:32:56

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 5/8] core: Make use of g_dbus_send_message_with_reply

From: Luiz Augusto von Dentz <[email protected]>

This replaces dbus_connection_send_with_reply with
g_dbus_send_message_with_reply which does not alter message order.
---
src/agent.c | 12 ++++++------
src/profile.c | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/agent.c b/src/agent.c
index b9e6e8c..7880ba6 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -370,7 +370,7 @@ static int agent_call_authorize_service(struct agent_request *req,
DBUS_TYPE_STRING, &uuid,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(btd_get_dbus_connection(),
+ if (g_dbus_send_message_with_reply(btd_get_dbus_connection(),
req->msg, &req->call,
REQUEST_TIMEOUT) == FALSE) {
error("D-Bus send failed");
@@ -480,7 +480,7 @@ static int pincode_request_new(struct agent_request *req, const char *device_pat
dbus_message_append_args(req->msg, DBUS_TYPE_OBJECT_PATH, &device_path,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(btd_get_dbus_connection(), req->msg,
+ if (g_dbus_send_message_with_reply(btd_get_dbus_connection(), req->msg,
&req->call, REQUEST_TIMEOUT) == FALSE) {
error("D-Bus send failed");
return -EIO;
@@ -574,7 +574,7 @@ static int passkey_request_new(struct agent_request *req,
dbus_message_append_args(req->msg, DBUS_TYPE_OBJECT_PATH, &device_path,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(btd_get_dbus_connection(), req->msg,
+ if (g_dbus_send_message_with_reply(btd_get_dbus_connection(), req->msg,
&req->call, REQUEST_TIMEOUT) == FALSE) {
error("D-Bus send failed");
return -EIO;
@@ -632,7 +632,7 @@ static int confirmation_request_new(struct agent_request *req,
DBUS_TYPE_UINT32, &passkey,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(btd_get_dbus_connection(), req->msg,
+ if (g_dbus_send_message_with_reply(btd_get_dbus_connection(), req->msg,
&req->call, REQUEST_TIMEOUT) == FALSE) {
error("D-Bus send failed");
return -EIO;
@@ -689,7 +689,7 @@ static int authorization_request_new(struct agent_request *req,
DBUS_TYPE_OBJECT_PATH, &device_path,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(btd_get_dbus_connection(), req->msg,
+ if (g_dbus_send_message_with_reply(btd_get_dbus_connection(), req->msg,
&req->call, REQUEST_TIMEOUT) == FALSE) {
error("D-Bus send failed");
return -EIO;
@@ -823,7 +823,7 @@ static int display_pincode_request_new(struct agent_request *req,
DBUS_TYPE_STRING, &pincode,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(btd_get_dbus_connection(), req->msg,
+ if (g_dbus_send_message_with_reply(btd_get_dbus_connection(), req->msg,
&req->call, REQUEST_TIMEOUT) == FALSE) {
error("D-Bus send failed");
return -EIO;
diff --git a/src/profile.c b/src/profile.c
index 90c3535..57aead7 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -938,7 +938,7 @@ static bool send_new_connection(struct ext_profile *ext, struct ext_io *conn)

dbus_message_iter_close_container(&iter, &dict);

- if (!dbus_connection_send_with_reply(btd_get_dbus_connection(),
+ if (!g_dbus_send_message_with_reply(btd_get_dbus_connection(),
msg, &conn->pending, -1)) {
error("%s: sending NewConnection failed", ext->name);
dbus_message_unref(msg);
@@ -1682,7 +1682,7 @@ static int send_disconn_req(struct ext_profile *ext, struct ext_io *conn)
dbus_message_append_args(msg, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID);

- if (!dbus_connection_send_with_reply(btd_get_dbus_connection(),
+ if (!g_dbus_send_message_with_reply(btd_get_dbus_connection(),
msg, &conn->pending, -1)) {
error("%s: sending RequestDisconnection failed", ext->name);
dbus_message_unref(msg);
--
1.8.3.1


2013-08-19 14:32:57

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 6/8] neard: Make use of g_dbus_send_message_with_reply

From: Luiz Augusto von Dentz <[email protected]>

This replaces dbus_connection_send_with_reply with
g_dbus_send_message_with_reply which does not alter message order.
---
plugins/neard.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/neard.c b/plugins/neard.c
index e4a4d71..ea91c4d 100644
--- a/plugins/neard.c
+++ b/plugins/neard.c
@@ -158,7 +158,7 @@ static void register_agent(bool append_carrier)
dbus_message_append_args(message, DBUS_TYPE_STRING, &carrier,
DBUS_TYPE_INVALID);

- if (!dbus_connection_send_with_reply(btd_get_dbus_connection(),
+ if (!g_dbus_send_message_with_reply(btd_get_dbus_connection(),
message, &call, -1)) {
dbus_message_unref(message);
error("D-Bus send failed");
--
1.8.3.1


2013-08-19 14:32:54

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 3/8] gdbus: Avoid calling dbus_connection_send*

From: Luiz Augusto von Dentz <[email protected]>

dbus_connection_send* should not be called directly except by
g_dbus_send_message.
---
gdbus/client.c | 14 ++++-----
gdbus/object.c | 92 ++++++++++++++++++++++------------------------------------
2 files changed, 42 insertions(+), 64 deletions(-)

diff --git a/gdbus/client.c b/gdbus/client.c
index d80e252..8ebfaad 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -100,7 +100,7 @@ static gboolean modify_match(DBusConnection *conn, const char *member,
dbus_message_append_args(msg, DBUS_TYPE_STRING, &rule,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(conn, msg, &call, -1) == FALSE) {
+ if (g_dbus_send_message_with_reply(conn, msg, &call, -1) == FALSE) {
dbus_message_unref(msg);
return FALSE;
}
@@ -319,7 +319,7 @@ static void get_all_properties(GDBusProxy *proxy)
dbus_message_append_args(msg, DBUS_TYPE_STRING, &proxy->interface,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(client->dbus_conn, msg,
+ if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
&call, -1) == FALSE) {
dbus_message_unref(msg);
return;
@@ -575,7 +575,7 @@ gboolean g_dbus_proxy_refresh_property(GDBusProxy *proxy, const char *name)
&proxy->interface);
dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &name);

- if (dbus_connection_send_with_reply(client->dbus_conn, msg,
+ if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
&call, -1) == FALSE) {
dbus_message_unref(msg);
refresh_property_free(data);
@@ -668,7 +668,7 @@ gboolean g_dbus_proxy_set_property_basic(GDBusProxy *proxy,
dbus_message_iter_append_basic(&variant, type, value);
dbus_message_iter_close_container(&iter, &variant);

- if (dbus_connection_send_with_reply(client->dbus_conn, msg,
+ if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
&call, -1) == FALSE) {
dbus_message_unref(msg);
g_free(data);
@@ -742,7 +742,7 @@ gboolean g_dbus_proxy_method_call(GDBusProxy *proxy, const char *method,
setup(&iter, data->user_data);
}

- if (dbus_connection_send_with_reply(client->dbus_conn, msg,
+ if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
&call, METHOD_CALL_TIMEOUT) == FALSE) {
dbus_message_unref(msg);
g_free(data);
@@ -1038,7 +1038,7 @@ static void get_managed_objects(GDBusClient *client)

dbus_message_append_args(msg, DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(client->dbus_conn, msg,
+ if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
&client->get_objects_call, -1) == FALSE) {
dbus_message_unref(msg);
return;
@@ -1102,7 +1102,7 @@ static void get_name_owner(GDBusClient *client, const char *name)
dbus_message_append_args(msg, DBUS_TYPE_STRING, &name,
DBUS_TYPE_INVALID);

- if (dbus_connection_send_with_reply(client->dbus_conn, msg,
+ if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
&client->pending_call, -1) == FALSE) {
dbus_message_unref(msg);
return;
diff --git a/gdbus/object.c b/gdbus/object.c
index 5d43036..92f57a0 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -271,8 +271,7 @@ static DBusHandlerResult process_message(DBusConnection *connection,
if (reply == NULL)
return DBUS_HANDLER_RESULT_NEED_MEMORY;

- dbus_connection_send(connection, reply, NULL);
- dbus_message_unref(reply);
+ g_dbus_send_message(connection, reply);

return DBUS_HANDLER_RESULT_HANDLED;
}
@@ -312,19 +311,14 @@ void g_dbus_pending_error_valist(DBusConnection *connection,

for (list = pending_security; list; list = list->next) {
struct security_data *secdata = list->data;
- DBusMessage *reply;

if (secdata->pending != pending)
continue;

pending_security = g_slist_remove(pending_security, secdata);

- reply = g_dbus_create_error_valist(secdata->message,
+ g_dbus_send_error_valist(connection, secdata->message,
name, format, args);
- if (reply != NULL) {
- dbus_connection_send(connection, reply, NULL);
- dbus_message_unref(reply);
- }

dbus_message_unref(secdata->message);
g_free(secdata);
@@ -469,18 +463,13 @@ void g_dbus_pending_property_error_valist(GDBusPendingReply id,
va_list args)
{
struct property_data *propdata;
- DBusMessage *reply;

propdata = remove_pending_property_data(id);
if (propdata == NULL)
return;

- reply = g_dbus_create_error_valist(propdata->message, name, format,
- args);
- if (reply != NULL) {
- dbus_connection_send(propdata->conn, reply, NULL);
- dbus_message_unref(reply);
- }
+ g_dbus_send_error_valist(propdata->conn, propdata->message, name,
+ format, args);

dbus_message_unref(propdata->message);
g_free(propdata);
@@ -1319,45 +1308,6 @@ static gboolean check_signal(DBusConnection *conn, const char *path,
return FALSE;
}

-static dbus_bool_t emit_signal_valist(DBusConnection *conn,
- const char *path,
- const char *interface,
- const char *name,
- int first,
- va_list var_args)
-{
- DBusMessage *signal;
- dbus_bool_t ret;
- const GDBusArgInfo *args;
-
- if (!check_signal(conn, path, interface, name, &args))
- return FALSE;
-
- signal = dbus_message_new_signal(path, interface, name);
- if (signal == NULL) {
- error("Unable to allocate new %s.%s signal", interface, name);
- return FALSE;
- }
-
- ret = dbus_message_append_args_valist(signal, first, var_args);
- if (!ret)
- goto fail;
-
- if (g_dbus_args_have_signature(args, signal) == FALSE) {
- error("%s.%s: got unexpected signature '%s'", interface, name,
- dbus_message_get_signature(signal));
- ret = FALSE;
- goto fail;
- }
-
- ret = dbus_connection_send(conn, signal, NULL);
-
-fail:
- dbus_message_unref(signal);
-
- return ret;
-}
-
gboolean g_dbus_register_interface(DBusConnection *connection,
const char *path, const char *name,
const GDBusMethodTable *methods,
@@ -1634,7 +1584,7 @@ gboolean g_dbus_emit_signal(DBusConnection *connection,

va_start(args, type);

- result = emit_signal_valist(connection, path, interface,
+ result = g_dbus_emit_signal_valist(connection, path, interface,
name, type, args);

va_end(args);
@@ -1646,8 +1596,36 @@ gboolean g_dbus_emit_signal_valist(DBusConnection *connection,
const char *path, const char *interface,
const char *name, int type, va_list args)
{
- return emit_signal_valist(connection, path, interface,
- name, type, args);
+ DBusMessage *signal;
+ dbus_bool_t ret;
+ const GDBusArgInfo *args_info;
+
+ if (!check_signal(connection, path, interface, name, &args_info))
+ return FALSE;
+
+ signal = dbus_message_new_signal(path, interface, name);
+ if (signal == NULL) {
+ error("Unable to allocate new %s.%s signal", interface, name);
+ return FALSE;
+ }
+
+ ret = dbus_message_append_args_valist(signal, type, args);
+ if (!ret)
+ goto fail;
+
+ if (g_dbus_args_have_signature(args_info, signal) == FALSE) {
+ error("%s.%s: got unexpected signature '%s'", interface, name,
+ dbus_message_get_signature(signal));
+ ret = FALSE;
+ goto fail;
+ }
+
+ return g_dbus_send_message(connection, signal);
+
+fail:
+ dbus_message_unref(signal);
+
+ return ret;
}

static void process_properties_from_interface(struct generic_data *data,
--
1.8.3.1


2013-08-19 14:32:55

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 4/8] gdbus: Fix emitting PropertiesChanged twice

From: Luiz Augusto von Dentz <[email protected]>

This fixes double emission of PropertiesChanged introduced by flushing
changes, the flushing can happen during the pending processing so the
pending_prop flag needs to be updated in the beginning and the list of
properties can be freed before g_dbus_send_message as it is not required
anymore.
---
gdbus/object.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 92f57a0..ae73061 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1636,6 +1636,8 @@ static void process_properties_from_interface(struct generic_data *data,
DBusMessageIter iter, dict, array;
GSList *invalidated;

+ data->pending_prop = FALSE;
+
if (iface->pending_prop == NULL)
return;

@@ -1685,10 +1687,10 @@ static void process_properties_from_interface(struct generic_data *data,
g_slist_free(invalidated);
dbus_message_iter_close_container(&iter, &array);

- g_dbus_send_message(data->conn, signal);
-
g_slist_free(iface->pending_prop);
iface->pending_prop = NULL;
+
+ g_dbus_send_message(data->conn, signal);
}

static void process_property_changes(struct generic_data *data)
@@ -1700,8 +1702,6 @@ static void process_property_changes(struct generic_data *data)

process_properties_from_interface(data, iface);
}
-
- data->pending_prop = FALSE;
}

void g_dbus_emit_property_changed(DBusConnection *connection,
--
1.8.3.1


2013-08-19 14:32:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 2/8] gdbus: Add g_dbus_send_message_with_reply

From: Luiz Augusto von Dentz <[email protected]>

g_dbus_send_message_with_reply flushes pending signals before calling
dbus_connection_send_with_reply so it does not alter the message order
---
gdbus/gdbus.h | 3 +++
gdbus/object.c | 11 +++++++++++
2 files changed, 14 insertions(+)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 8b13393..9542109 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -250,6 +250,9 @@ DBusMessage *g_dbus_create_reply_valist(DBusMessage *message,
int type, va_list args);

gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message);
+gboolean g_dbus_send_message_with_reply(DBusConnection *connection,
+ DBusMessage *message,
+ DBusPendingCall **call, int timeout);
gboolean g_dbus_send_error(DBusConnection *connection, DBusMessage *message,
const char *name, const char *format, ...)
__attribute__((format(printf, 4, 5)));
diff --git a/gdbus/object.c b/gdbus/object.c
index b2ca1c2..5d43036 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1550,6 +1550,17 @@ out:
return result;
}

+gboolean g_dbus_send_message_with_reply(DBusConnection *connection,
+ DBusMessage *message,
+ DBusPendingCall **call, int timeout)
+{
+ /* Flush pending signal to guarantee message order */
+ g_dbus_flush(connection);
+
+ return dbus_connection_send_with_reply(connection, message, call,
+ timeout);
+}
+
gboolean g_dbus_send_error_valist(DBusConnection *connection,
DBusMessage *message, const char *name,
const char *format, va_list args)
--
1.8.3.1


2013-08-19 14:32:52

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 1/8] gdbus: Fix not maintaining message order for signals

From: Luiz Augusto von Dentz <[email protected]>

In some cases the order of the messages is altered when a message is
sent without processing the pending signals first, currently this affect
client_check_order unit test:

/gdbus/client_check_order: **
ERROR:unit/test-gdbus-client.c:795:property_check_order: assertion failed: (g_strcmp0(string, "value1") == 0)

As can be observed the value of the property is not yet updated because the
signal it is still pending, once this fix is applied the test pass:

/gdbus/client_check_order: OK

Note that the flushing only works when g_dbus_send_message is used so
places where dbus_connection_send and other variants are called directly
may still change the order.
---
gdbus/object.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index 2f8ef45..b2ca1c2 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1494,6 +1494,35 @@ DBusMessage *g_dbus_create_reply(DBusMessage *message, int type, ...)
return reply;
}

+static void g_dbus_flush_object(struct generic_data *data)
+{
+ GSList *l;
+
+ if (data->process_id > 0) {
+ g_source_remove(data->process_id);
+ data->process_id = 0;
+ }
+
+ process_changes(data);
+
+ for (l = data->objects; l; l = l->next)
+ g_dbus_flush_object(l->data);
+}
+
+static void g_dbus_flush(DBusConnection *connection)
+{
+ static gboolean flushing = FALSE;
+
+ if (flushing || root == NULL || root->conn != connection)
+ return;
+
+ flushing = TRUE;
+
+ g_dbus_flush_object(root);
+
+ flushing = FALSE;
+}
+
gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
{
dbus_bool_t result = FALSE;
@@ -1510,6 +1539,9 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
goto out;
}

+ /* Flush pending signal to guarantee message order */
+ g_dbus_flush(connection);
+
result = dbus_connection_send(connection, message, NULL);

out:
--
1.8.3.1


2013-08-19 14:32:51

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ] build: Fix not checking readline.h for bluetooth-player

From: Luiz Augusto von Dentz <[email protected]>

If --disable-client is passed to configure script is possible to build
BlueZ without readline which would cause the following error:

tools/bluetooth-player.c:36:31: fatal error: readline/readline.h: No such file or directory
#include <readline/readline.h>
^
---
Makefile.tools | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Makefile.tools b/Makefile.tools
index 3bb0e82..4bcb18e 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -172,8 +172,7 @@ if EXPERIMENTAL
noinst_PROGRAMS += tools/bdaddr tools/avinfo tools/avtest tools/scotest \
tools/hcieventmask tools/hcisecfilter tools/hwdb \
tools/btmgmt tools/btattach tools/btsnoop \
- tools/btiotest tools/mpris-player \
- tools/bluetooth-player
+ tools/btiotest tools/mpris-player

tools_bdaddr_SOURCES = tools/bdaddr.c src/oui.h src/oui.c
tools_bdaddr_LDADD = lib/libbluetooth-internal.la @UDEV_LIBS@
@@ -199,17 +198,13 @@ tools_btiotest_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
tools_mpris_player_SOURCES = tools/mpris-player.c
tools_mpris_player_LDADD = gdbus/libgdbus-internal.la @GLIB_LIBS@ @DBUS_LIBS@

-tools_bluetooth_player_SOURCES = tools/bluetooth-player.c \
- client/display.h client/display.c
-tools_bluetooth_player_LDADD = gdbus/libgdbus-internal.la \
- @GLIB_LIBS@ @DBUS_LIBS@ -lreadline
-
EXTRA_DIST += tools/bdaddr.1
endif

if READLINE
noinst_PROGRAMS += attrib/gatttool \
- tools/obex-client-tool tools/obex-server-tool
+ tools/obex-client-tool tools/obex-server-tool \
+ tools/bluetooth-player

attrib_gatttool_SOURCES = attrib/gatttool.c attrib/att.c attrib/gatt.c \
attrib/gattrib.c btio/btio.c \
@@ -226,6 +221,11 @@ tools_obex_client_tool_LDADD = lib/libbluetooth-internal.la \
tools_obex_server_tool_SOURCES = $(gobex_sources) $(btio_sources) \
tools/obex-server-tool.c
tools_obex_server_tool_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
+
+tools_bluetooth_player_SOURCES = tools/bluetooth-player.c \
+ client/display.h client/display.c
+tools_bluetooth_player_LDADD = gdbus/libgdbus-internal.la \
+ @GLIB_LIBS@ @DBUS_LIBS@ -lreadline
endif

if EXPERIMENTAL
--
1.8.3.1