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

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

[Troels Dalsgaard Hoffmeyer]

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(-)

-- 
2.17.1

[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.

[Troels Dalsgaard Hoffmeyer]

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

diff --git a/src/advertising.c b/src/advertising.c
index 2f187edcf..26e24ee01 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -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;
 }
-- 
2.17.1

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.

[Luiz Augusto von Dentz]

Hi Troels,

On Mon, Apr 29, 2019 at 2:16 PM Troels Dalsgaard Hoffmeyer
<troels.d.hoffmeyer@gmail.com> wrote:
>

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

> ---
>  src/advertising.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/advertising.c b/src/advertising.c
> index 2f187edcf..26e24ee01 100644
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -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;
>  }

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.

-- 
Luiz Augusto von Dentz

[PATCH Bluez 0/1] Commit message fixup

[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(-)

-- 
2.17.1

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

[Troels Dalsgaard Hoffmeyer]

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(-)

diff --git a/src/advertising.c b/src/advertising.c
index 2f187edcf..890acd542 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -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;
 }
-- 
2.17.1

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

[Luiz Augusto von Dentz]

Hi Troels,

On Tue, Apr 30, 2019 at 2:57 PM Troels Dalsgaard Hoffmeyer
<troels.d.hoffmeyer@gmail.com> wrote:
>
> 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(-)
>
> --
> 2.17.1

Applied, thanks.

-- 
Luiz Augusto von Dentz