Skip to contentSkip to conversation

advertising: Timeout of 0 should not fire a callback

PATCH In review Troels Dalsgaard Hoffmeyer 2 participants 6 messages Apr 29, 2019 – Apr 30, 2019 Bluetooth drivers

lore
Share & tools
lore

Troels Dalsgaard Hoffmeyer

author
#0 lore raw

cover advertising: Timeout of 0 should not fire a callback

When an advertisement with a timeout of 0 was registered, the timeout callback was immediately called. The client received a signal that it was released even though it was still transmitted in the air. This seems unintended

Troels Dalsgaard Hoffmeyer (1):
advertising: Timeout of 0 should not fire a callback Setting a timeout
of 0 on an advertisement should let the advertisement run forever. The client was released immediately after, although the advertisement was still in the air.

 src/advertising.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH 1/1 advertising: Timeout of 0 should not fire a callback Setting a timeout of 0 on an advertisement should let the advertisement run forever. The client was released immediately after, although the advertisement was still in the air.

---
 src/advertising.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/advertising.c +4 -2
index 2f187edcf..26e24ee01 100644
@@ -587,8 +587,10 @@ static bool parse_timeout(DBusMessageIter *iter,
 	if (client->to_id)
 		g_source_remove(client->to_id);
 
-	client->to_id = g_timeout_add_seconds(client->timeout, client_timeout,
-								client);
+	if(client->timeout > 0) {
+		client->to_id = g_timeout_add_seconds(client->timeout, client_timeout,
+																		client);
+	}
 
 	return true;
 }
Signature · 1 line
2.17.1
maintainer
#2 lore raw

Re: [PATCH Bluez 1/1] advertising: Timeout of 0 should not fire a callback Setting a timeout of 0 on an advertisement should let the advertisement run forever. The client was released immediately after, although the advertisement was still in the air.

Hi Troels,

Quoted 1 line · Troels Dalsgaard Hoffmeyer

Please rework the commit message, the subject should be rather short and you should put some stuff in the description.

Quoted 11 lines
---
 src/advertising.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/advertising.c +0 -0
index 2f187edcf..26e24ee01 100644
@@ -587,8 +587,10 @@ static bool parse_timeout(DBusMessageIter *iter,
        if (client->to_id)
                g_source_remove(client->to_id);

src/advertising.c static bool parse_timeout(DBusMessageIter *iter, · line 596

Quoted 10 lines
src/advertising.c +4 -2
 
-       client->to_id = g_timeout_add_seconds(client->timeout, client_timeout,
-                                                               client);
+       if(client->timeout > 0) {
+               client->to_id = g_timeout_add_seconds(client->timeout, client_timeout,
+                                                                                                                                               client);
+       }
 
        return true;
 }

While I do agree we should handle timeout 0 as no timeout the reason you are hitting this in the first place is a misused of the API where you are setting a Timeout parameter when there shouldn't be one.

Signature · 1 line
Luiz Augusto von Dentz
author
#3 lore raw

cover Commit message fixup

In reply to Troels Dalsgaard Hoffmeyer

I didn't put a newline between the subject and the description.
I hope this commit message is better. It is my first time doing patches by email, and i'm still learning :)

I didn't know that setting the timeout to 0 would be a missuse.
I just modified the python advertisement example.
The problem i ran into was that creating and releasing these advertisments with timeout 0 could sometimes cause a segfault in bluetoothd

Troels Dalsgaard Hoffmeyer (1):
advertising: Timeout of 0 should not fire a callback

 src/advertising.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Signature · 1 line
2.17.1
author
#4 lore raw

PATCH 1/1 advertising: Timeout of 0 should not fire a callback

Setting a timeout of 0 on an advertisement should let the advertisement run forever. The client was released immediately after, although the advertisement was still in the air.

---
 src/advertising.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/advertising.c +4 -2
index 2f187edcf..890acd542 100644
@@ -587,8 +587,10 @@ static bool parse_timeout(DBusMessageIter *iter,
 	if (client->to_id)
 		g_source_remove(client->to_id);
 
-	client->to_id = g_timeout_add_seconds(client->timeout, client_timeout,
-								client);
+	if (client->timeout > 0) {
+		client->to_id = g_timeout_add_seconds(client->timeout, client_timeout,
+																		client);
+	}
 
 	return true;
 }
Signature · 1 line
2.17.1
maintainer Applied
#5 lore raw

Re: [PATCH Bluez 0/1] Commit message fixup

In reply to Troels Dalsgaard Hoffmeyer

Hi Troels,

Quoted 8 lines · Troels Dalsgaard Hoffmeyer

I didn't put a newline between the subject and the description.
I hope this commit message is better. It is my first time doing patches by email, and i'm still learning :)

I didn't know that setting the timeout to 0 would be a missuse.
I just modified the python advertisement example.
The problem i ran into was that creating and releasing these advertisments with timeout 0 could sometimes cause a segfault in bluetoothd

Quoted 8 lines

Troels Dalsgaard Hoffmeyer (1):
advertising: Timeout of 0 should not fire a callback

 src/advertising.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
2.17.1

Applied, thanks.

Signature · 1 line
Luiz Augusto von Dentz