2020-10-05 04:20:21

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ] shared/timeout-ell: Fix timeout wrapper implementation

This fixes the following issues:
- Correct user data is passed around to l_timeout_create():
locally allocated timeout data is a valid "user data" to
associate with a newly created timeout. Previously, user_data
passed as an argument to timeout_add() was incorrectly used as
an argument to l_timeout_create()
- To maintain common API and work around the issue when the conversion
of a pointer to an unsigned int truncates the initial value, a queue
of active timeouts is maintained where pointer each l_timeout structure
is associate with a unique id. This id is returned when timeout_create()
API is called and can be subsequently used with timeout_remove().
---
src/shared/timeout-ell.c | 50 ++++++++++++++++++++++++++++++++++++----
1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/src/shared/timeout-ell.c b/src/shared/timeout-ell.c
index c2da387e2..023364069 100644
--- a/src/shared/timeout-ell.c
+++ b/src/shared/timeout-ell.c
@@ -12,13 +12,23 @@

#include "timeout.h"

+static struct l_queue *timeout_q;
+
struct timeout_data {
timeout_func_t func;
timeout_destroy_func_t destroy;
- unsigned int timeout;
void *user_data;
+ unsigned int timeout;
};

+static bool match_id(const void *a, const void *b)
+{
+ unsigned int to_id = L_PTR_TO_UINT(a);
+ unsigned int id = L_PTR_TO_UINT(b);
+
+ return (to_id == id);
+}
+
static void timeout_callback(struct l_timeout *timeout, void *user_data)
{
struct timeout_data *data = user_data;
@@ -43,7 +53,12 @@ unsigned int timeout_add(unsigned int timeout, timeout_func_t func,
void *user_data, timeout_destroy_func_t destroy)
{
struct timeout_data *data;
- uint32_t id;
+ unsigned int id = 0;
+ struct l_timeout *to;
+ int tries = 0;
+
+ if (!timeout_q)
+ timeout_q = l_queue_new();

data = l_new(struct timeout_data, 1);

@@ -52,12 +67,37 @@ unsigned int timeout_add(unsigned int timeout, timeout_func_t func,
data->user_data = user_data;
data->timeout = timeout;

- id = L_PTR_TO_UINT(l_timeout_create(timeout, timeout_callback,
- user_data, timeout_destroy));
+ while (id == 0 && tries < 3) {
+ to = l_timeout_create(timeout, timeout_callback,
+ data, timeout_destroy);
+ if (!to)
+ break;
+
+ tries++;
+ id = L_PTR_TO_UINT(to);
+
+ if (id == 0 ||
+ l_queue_find(timeout_q, match_id, L_UINT_TO_PTR(id))) {
+
+ l_timeout_remove(to);
+ continue;
+ }
+
+ l_queue_push_tail(timeout_q, to);
+ }
+
+ if (id == 0)
+ l_free(data);
+
return id;
}

void timeout_remove(unsigned int id)
{
- l_timeout_remove(L_UINT_TO_PTR(id));
+ struct l_timeout *to;
+
+ to = l_queue_remove_if(timeout_q, match_id, L_UINT_TO_PTR(id));
+
+ if (to)
+ l_timeout_remove(to);
}
--
2.26.2


2020-10-05 04:42:12

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ] shared/timeout-ell: Fix timeout wrapper implementation

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=359581

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth

2020-10-20 07:41:48

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] shared/timeout-ell: Fix timeout wrapper implementation

Hi Inga,

On Mon, Oct 19, 2020 at 1:00 PM Stotland, Inga <[email protected]> wrote:
>
> Another ping
>
> On Fri, 2020-10-09 at 21:03 -0700, Inga Stotland wrote:
>
> Gentle ping
>
> On Sun, 2020-10-04 at 21:19 -0700, Inga Stotland wrote:
>
> This fixes the following issues:
>
> - Correct user data is passed around to l_timeout_create():
>
> locally allocated timeout data is a valid "user data" to
>
> associate with a newly created timeout. Previously, user_data
>
> passed as an argument to timeout_add() was incorrectly used as
>
> an argument to l_timeout_create()
>
> - To maintain common API and work around the issue when the conversion
>
> of a pointer to an unsigned int truncates the initial value, a queue
>
> of active timeouts is maintained where pointer each l_timeout structure
>
> is associate with a unique id. This id is returned when timeout_create()
>
> API is called and can be subsequently used with timeout_remove().
>
> ---
>
> src/shared/timeout-ell.c | 50 ++++++++++++++++++++++++++++++++++++----
>
> 1 file changed, 45 insertions(+), 5 deletions(-)
>
>
> diff --git a/src/shared/timeout-ell.c b/src/shared/timeout-ell.c
>
> index c2da387e2..023364069 100644
>
> --- a/src/shared/timeout-ell.c
>
> +++ b/src/shared/timeout-ell.c
>
> @@ -12,13 +12,23 @@
>
>
>
> #include "timeout.h"
>
>
>
> +static struct l_queue *timeout_q;
>
> +
>
> struct timeout_data {
>
> timeout_func_t func;
>
> timeout_destroy_func_t destroy;
>
> - unsigned int timeout;
>
> void *user_data;
>
> + unsigned int timeout;
>
> };
>
>
>
> +static bool match_id(const void *a, const void *b)
>
> +{
>
> + unsigned int to_id = L_PTR_TO_UINT(a);
>
> + unsigned int id = L_PTR_TO_UINT(b);
>
> +
>
> + return (to_id == id);
>
> +}
>
> +
>
> static void timeout_callback(struct l_timeout *timeout, void *user_data)
>
> {
>
> struct timeout_data *data = user_data;
>
> @@ -43,7 +53,12 @@ unsigned int timeout_add(unsigned int timeout, timeout_func_t func,
>
> void *user_data, timeout_destroy_func_t destroy)
>
> {
>
> struct timeout_data *data;
>
> - uint32_t id;
>
> + unsigned int id = 0;
>
> + struct l_timeout *to;
>
> + int tries = 0;
>
> +
>
> + if (!timeout_q)
>
> + timeout_q = l_queue_new();
>
>
>
> data = l_new(struct timeout_data, 1);
>
>
>
> @@ -52,12 +67,37 @@ unsigned int timeout_add(unsigned int timeout, timeout_func_t func,
>
> data->user_data = user_data;
>
> data->timeout = timeout;
>
>
>
> - id = L_PTR_TO_UINT(l_timeout_create(timeout, timeout_callback,
>
> - user_data, timeout_destroy));
>
> + while (id == 0 && tries < 3) {
>
> + to = l_timeout_create(timeout, timeout_callback,
>
> + data, timeout_destroy);
>
> + if (!to)
>
> + break;
>
> +
>
> + tries++;
>
> + id = L_PTR_TO_UINT(to);
>
> +
>
> + if (id == 0 ||
>
> + l_queue_find(timeout_q, match_id, L_UINT_TO_PTR(id))) {
>
> +
>
> + l_timeout_remove(to);
>
> + continue;
>
> + }
>
> +
>
> + l_queue_push_tail(timeout_q, to);
>
> + }
>
> +
>
> + if (id == 0)
>
> + l_free(data);
>
> +
>
> return id;
>
> }
>
>
>
> void timeout_remove(unsigned int id)
>
> {
>
> - l_timeout_remove(L_UINT_TO_PTR(id));
>
> + struct l_timeout *to;
>
> +
>
> + to = l_queue_remove_if(timeout_q, match_id, L_UINT_TO_PTR(id));
>
> +
>
> + if (to)
>
> + l_timeout_remove(to);
>
> }

Applied, thanks.

--
Luiz Augusto von Dentz