[PATCH] Fix race condition on gatttool

[PATCH] Fix race condition on gatttool

[Sheldon Demario]

When the connect_cb() takes too long to be called the event_loop goes to idle
state and executes the callback too early
---
 attrib/gatttool.c |  128 +++++++++++++++++++++++++---------------------------
 1 files changed, 62 insertions(+), 66 deletions(-)

diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index be91967..cf052db 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -63,6 +63,7 @@ static gboolean opt_char_write_req = FALSE;
 static gboolean opt_interactive = FALSE;
 static GMainLoop *event_loop;
 static gboolean got_error = FALSE;
+static GSourceFunc operation;
 
 struct characteristic_data {
 	GAttrib *attrib;
@@ -70,13 +71,68 @@ struct characteristic_data {
 	uint16_t end;
 };
 
+static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
+{
+	GAttrib *attrib = user_data;
+	uint8_t opdu[ATT_MAX_MTU];
+	uint16_t handle, i, olen = 0;
+
+	handle = att_get_u16(&pdu[1]);
+
+	switch (pdu[0]) {
+	case ATT_OP_HANDLE_NOTIFY:
+		g_print("Notification handle = 0x%04x value: ", handle);
+		break;
+	case ATT_OP_HANDLE_IND:
+		g_print("Indication   handle = 0x%04x value: ", handle);
+		break;
+	default:
+		g_print("Invalid opcode\n");
+		return;
+	}
+
+	for (i = 3; i < len; i++)
+		g_print("%02x ", pdu[i]);
+
+	g_print("\n");
+
+	if (pdu[0] == ATT_OP_HANDLE_NOTIFY)
+		return;
+
+	olen = enc_confirmation(opdu, sizeof(opdu));
+
+	if (olen > 0)
+		g_attrib_send(attrib, 0, opdu[0], opdu, olen, NULL, NULL, NULL);
+}
+
+static gboolean listen_start(gpointer user_data)
+{
+	GAttrib *attrib = user_data;
+
+	g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, events_handler,
+							attrib, NULL);
+	g_attrib_register(attrib, ATT_OP_HANDLE_IND, events_handler,
+							attrib, NULL);
+
+	return FALSE;
+}
+
 static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 {
+	GAttrib *attrib;
+
 	if (err) {
 		g_printerr("%s\n", err->message);
 		got_error = TRUE;
 		g_main_loop_quit(event_loop);
 	}
+
+	attrib = g_attrib_new(io);
+
+	if (opt_listen)
+		g_idle_add(listen_start, attrib);
+
+	operation(attrib);
 }
 
 static void primary_all_cb(GSList *services, guint8 status, gpointer user_data)
@@ -120,52 +176,6 @@ done:
 	g_main_loop_quit(event_loop);
 }
 
-static void events_handler(const uint8_t *pdu, uint16_t len, gpointer user_data)
-{
-	GAttrib *attrib = user_data;
-	uint8_t opdu[ATT_MAX_MTU];
-	uint16_t handle, i, olen = 0;
-
-	handle = att_get_u16(&pdu[1]);
-
-	switch (pdu[0]) {
-	case ATT_OP_HANDLE_NOTIFY:
-		g_print("Notification handle = 0x%04x value: ", handle);
-		break;
-	case ATT_OP_HANDLE_IND:
-		g_print("Indication   handle = 0x%04x value: ", handle);
-		break;
-	default:
-		g_print("Invalid opcode\n");
-		return;
-	}
-
-	for (i = 3; i < len; i++)
-		g_print("%02x ", pdu[i]);
-
-	g_print("\n");
-
-	if (pdu[0] == ATT_OP_HANDLE_NOTIFY)
-		return;
-
-	olen = enc_confirmation(opdu, sizeof(opdu));
-
-	if (olen > 0)
-		g_attrib_send(attrib, 0, opdu[0], opdu, olen, NULL, NULL, NULL);
-}
-
-static gboolean listen_start(gpointer user_data)
-{
-	GAttrib *attrib = user_data;
-
-	g_attrib_register(attrib, ATT_OP_HANDLE_NOTIFY, events_handler,
-							attrib, NULL);
-	g_attrib_register(attrib, ATT_OP_HANDLE_IND, events_handler,
-							attrib, NULL);
-
-	return FALSE;
-}
-
 static gboolean primary(gpointer user_data)
 {
 	GAttrib *attrib = user_data;
@@ -527,9 +537,7 @@ int main(int argc, char *argv[])
 	GOptionContext *context;
 	GOptionGroup *gatt_group, *params_group, *char_rw_group;
 	GError *gerr = NULL;
-	GAttrib *attrib;
 	GIOChannel *chan;
-	GSourceFunc callback;
 
 	opt_sec_level = g_strdup("low");
 
@@ -570,17 +578,17 @@ int main(int argc, char *argv[])
 	}
 
 	if (opt_primary)
-		callback = primary;
+		operation = primary;
 	else if (opt_characteristics)
-		callback = characteristics;
+		operation = characteristics;
 	else if (opt_char_read)
-		callback = characteristics_read;
+		operation = characteristics_read;
 	else if (opt_char_write)
-		callback = characteristics_write;
+		operation = characteristics_write;
 	else if (opt_char_write_req)
-		callback = characteristics_write_req;
+		operation = characteristics_write_req;
 	else if (opt_char_desc)
-		callback = characteristics_desc;
+		operation = characteristics_desc;
 	else {
 		gchar *help = g_option_context_get_help(context, TRUE, NULL);
 		g_print("%s\n", help);
@@ -596,24 +604,12 @@ int main(int argc, char *argv[])
 		goto done;
 	}
 
-	attrib = g_attrib_new(chan);
-	g_io_channel_unref(chan);
-
 	event_loop = g_main_loop_new(NULL, FALSE);
 
-	if (opt_listen)
-		g_idle_add(listen_start, attrib);
-
-	g_idle_add(callback, attrib);
-
 	g_main_loop_run(event_loop);
 
-	g_attrib_unregister_all(attrib);
-
 	g_main_loop_unref(event_loop);
 
-	g_attrib_unref(attrib);
-
 done:
 	g_option_context_free(context);
 	g_free(opt_src);
-- 
1.7.1

Re: [PATCH] Fix race condition on gatttool

[Johan Hedberg]

Hi Sheldon,

On Mon, Apr 11, 2011, Sheldon Demario wrote:
> When the connect_cb() takes too long to be called the event_loop goes to idle
> state and executes the callback too early
> ---
>  attrib/gatttool.c |  128 +++++++++++++++++++++++++---------------------------
>  1 files changed, 62 insertions(+), 66 deletions(-)

Thanks, the patch has been pushed upstream. Could you please fix your
editor settings so that it doesn't produce lines longer than 74
characters for git commit messages? I had to manually edit it this time.

Johan

Re: [PATCH] Fix race condition on gatttool

[Sheldon Demario]

Johan,

On Thu, Apr 14, 2011 at 2:07 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Sheldon,
>
> On Mon, Apr 11, 2011, Sheldon Demario wrote:
>> When the connect_cb() takes too long to be called the event_loop goes to idle
>> state and executes the callback too early
>> ---
>>  attrib/gatttool.c |  128 +++++++++++++++++++++++++---------------------------
>>  1 files changed, 62 insertions(+), 66 deletions(-)
>
> Thanks, the patch has been pushed upstream. Could you please fix your
> editor settings so that it doesn't produce lines longer than 74
> characters for git commit messages? I had to manually edit it this time.

For sure. I was using the 80 chars limit.

>
> Johan
>