2021-03-16 17:26:17

by Frédéric Danis

Subject: [PATCH Bluez v5 1/4] shared/timeout: Add timeout_add_seconds abstraction

g_timeout_add_seconds() call doesn't ensure the time for the first call of
the timer if the delay is less or equal to 1 second.
In case of a 0 delay call g_idle_add() instead of g_timeout_add_seconds().
---
src/shared/tester.c | 16 +++++++++-------
src/shared/timeout-ell.c | 6 ++++++
src/shared/timeout-glib.c | 27 +++++++++++++++++++++++++++
src/shared/timeout-mainloop.c | 6 ++++++
src/shared/timeout.h | 3 +++
5 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/src/shared/tester.c b/src/shared/tester.c
index af33a79cd..c07cbc11c 100644
--- a/src/shared/tester.c
+++ b/src/shared/tester.c
@@ -36,6 +36,7 @@
#include "src/shared/util.h"
#include "src/shared/tester.h"
#include "src/shared/log.h"
+#include "src/shared/timeout.h"

#define COLOR_OFF "\x1B[0m"
#define COLOR_BLACK "\x1B[0;30m"
@@ -126,7 +127,7 @@ static void test_destroy(gpointer data)
struct test_case *test = data;

if (test->timeout_id > 0)
- g_source_remove(test->timeout_id);
+ timeout_remove(test->timeout_id);

if (test->teardown_id > 0)
g_source_remove(test->teardown_id);
@@ -429,7 +430,7 @@ static gboolean teardown_callback(gpointer user_data)
return FALSE;
}

-static gboolean test_timeout(gpointer user_data)
+static bool test_timeout(gpointer user_data)
{
struct test_case *test = user_data;

@@ -470,8 +471,9 @@ static void next_test_case(void)
test->start_time = g_timer_elapsed(test_timer, NULL);

if (test->timeout > 0)
- test->timeout_id = g_timeout_add_seconds(test->timeout,
- test_timeout, test);
+ test->timeout_id = timeout_add_seconds(test->timeout,
+ test_timeout, test,
+ NULL);

test->stage = TEST_STAGE_PRE_SETUP;

@@ -542,7 +544,7 @@ void tester_pre_setup_failed(void)
return;

if (test->timeout_id > 0) {
- g_source_remove(test->timeout_id);
+ timeout_remove(test->timeout_id);
test->timeout_id = 0;
}

@@ -583,7 +585,7 @@ void tester_setup_failed(void)
test->stage = TEST_STAGE_POST_TEARDOWN;

if (test->timeout_id > 0) {
- g_source_remove(test->timeout_id);
+ timeout_remove(test->timeout_id);
test->timeout_id = 0;
}

@@ -606,7 +608,7 @@ static void test_result(enum test_result result)
return;

if (test->timeout_id > 0) {
- g_source_remove(test->timeout_id);
+ timeout_remove(test->timeout_id);
test->timeout_id = 0;
}

diff --git a/src/shared/timeout-ell.c b/src/shared/timeout-ell.c
index 023364069..6416d8590 100644
--- a/src/shared/timeout-ell.c
+++ b/src/shared/timeout-ell.c
@@ -101,3 +101,9 @@ void timeout_remove(unsigned int id)
if (to)
l_timeout_remove(to);
}
+
+unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
+ void *user_data, timeout_destroy_func_t destroy)
+{
+ return timeout_add(timeout * 1000, func, user_data, destroy);
+}
diff --git a/src/shared/timeout-glib.c b/src/shared/timeout-glib.c
index 8bdb7a662..3268d480c 100644
--- a/src/shared/timeout-glib.c
+++ b/src/shared/timeout-glib.c
@@ -71,3 +71,30 @@ void timeout_remove(unsigned int id)
if (source)
g_source_destroy(source);
}
+
+unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
+ void *user_data, timeout_destroy_func_t destroy)
+{
+ struct timeout_data *data;
+ guint id;
+
+ data = g_try_new0(struct timeout_data, 1);
+ if (!data)
+ return 0;
+
+ data->func = func;
+ data->destroy = destroy;
+ data->user_data = user_data;
+
+ if (!timeout)
+ id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, timeout_callback,
+ data, timeout_destroy);
+ else
+ id = g_timeout_add_seconds_full(G_PRIORITY_DEFAULT, timeout,
+ timeout_callback, data,
+ timeout_destroy);
+ if (!id)
+ g_free(data);
+
+ return id;
+}
diff --git a/src/shared/timeout-mainloop.c b/src/shared/timeout-mainloop.c
index 5ffa65c2a..9be803cda 100644
--- a/src/shared/timeout-mainloop.c
+++ b/src/shared/timeout-mainloop.c
@@ -71,3 +71,9 @@ void timeout_remove(unsigned int id)

mainloop_remove_timeout((int) id);
}
+
+unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
+ void *user_data, timeout_destroy_func_t destroy)
+{
+ return timeout_add(timeout * 1000, func, user_data, destroy);
+}
diff --git a/src/shared/timeout.h b/src/shared/timeout.h
index 7e22345dd..0945c3318 100644
--- a/src/shared/timeout.h
+++ b/src/shared/timeout.h
@@ -16,3 +16,6 @@ typedef void (*timeout_destroy_func_t)(void *user_data);
unsigned int timeout_add(unsigned int timeout, timeout_func_t func,
void *user_data, timeout_destroy_func_t destroy);
void timeout_remove(unsigned int id);
+
+unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
+ void *user_data, timeout_destroy_func_t destroy);
--
2.18.0

2021-03-16 21:16:35

by bluez.test.bot

Subject: RE: adapter: Fix discovery trigger for 0 second delay

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=449327

---Test result---

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

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

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

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



---
Regards,
Linux Bluetooth

2021-03-16 21:20:25

by Luiz Augusto von Dentz

Subject: Re: [PATCH Bluez v5 1/4] shared/timeout: Add timeout_add_seconds abstraction

Hi Frédéric,

On Tue, Mar 16, 2021 at 10:26 AM Frédéric Danis
<[email protected]> wrote:
>
> g_timeout_add_seconds() call doesn't ensure the time for the first call of
> the timer if the delay is less or equal to 1 second.
> In case of a 0 delay call g_idle_add() instead of g_timeout_add_seconds().
> ---
> src/shared/tester.c | 16 +++++++++-------
> src/shared/timeout-ell.c | 6 ++++++
> src/shared/timeout-glib.c | 27 +++++++++++++++++++++++++++
> src/shared/timeout-mainloop.c | 6 ++++++
> src/shared/timeout.h | 3 +++
> 5 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/src/shared/tester.c b/src/shared/tester.c
> index af33a79cd..c07cbc11c 100644
> --- a/src/shared/tester.c
> +++ b/src/shared/tester.c
> @@ -36,6 +36,7 @@
> #include "src/shared/util.h"
> #include "src/shared/tester.h"
> #include "src/shared/log.h"
> +#include "src/shared/timeout.h"
>
> #define COLOR_OFF "\x1B[0m"
> #define COLOR_BLACK "\x1B[0;30m"
> @@ -126,7 +127,7 @@ static void test_destroy(gpointer data)
> struct test_case *test = data;
>
> if (test->timeout_id > 0)
> - g_source_remove(test->timeout_id);
> + timeout_remove(test->timeout_id);
>
> if (test->teardown_id > 0)
> g_source_remove(test->teardown_id);
> @@ -429,7 +430,7 @@ static gboolean teardown_callback(gpointer user_data)
> return FALSE;
> }
>
> -static gboolean test_timeout(gpointer user_data)
> +static bool test_timeout(gpointer user_data)
> {
> struct test_case *test = user_data;
>
> @@ -470,8 +471,9 @@ static void next_test_case(void)
> test->start_time = g_timer_elapsed(test_timer, NULL);
>
> if (test->timeout > 0)
> - test->timeout_id = g_timeout_add_seconds(test->timeout,
> - test_timeout, test);
> + test->timeout_id = timeout_add_seconds(test->timeout,
> + test_timeout, test,
> + NULL);
>
> test->stage = TEST_STAGE_PRE_SETUP;
>
> @@ -542,7 +544,7 @@ void tester_pre_setup_failed(void)
> return;
>
> if (test->timeout_id > 0) {
> - g_source_remove(test->timeout_id);
> + timeout_remove(test->timeout_id);
> test->timeout_id = 0;
> }
>
> @@ -583,7 +585,7 @@ void tester_setup_failed(void)
> test->stage = TEST_STAGE_POST_TEARDOWN;
>
> if (test->timeout_id > 0) {
> - g_source_remove(test->timeout_id);
> + timeout_remove(test->timeout_id);
> test->timeout_id = 0;
> }
>
> @@ -606,7 +608,7 @@ static void test_result(enum test_result result)
> return;
>
> if (test->timeout_id > 0) {
> - g_source_remove(test->timeout_id);
> + timeout_remove(test->timeout_id);
> test->timeout_id = 0;
> }
>
> diff --git a/src/shared/timeout-ell.c b/src/shared/timeout-ell.c
> index 023364069..6416d8590 100644
> --- a/src/shared/timeout-ell.c
> +++ b/src/shared/timeout-ell.c
> @@ -101,3 +101,9 @@ void timeout_remove(unsigned int id)
> if (to)
> l_timeout_remove(to);
> }
> +
> +unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
> + void *user_data, timeout_destroy_func_t destroy)
> +{
> + return timeout_add(timeout * 1000, func, user_data, destroy);
> +}
> diff --git a/src/shared/timeout-glib.c b/src/shared/timeout-glib.c
> index 8bdb7a662..3268d480c 100644
> --- a/src/shared/timeout-glib.c
> +++ b/src/shared/timeout-glib.c
> @@ -71,3 +71,30 @@ void timeout_remove(unsigned int id)
> if (source)
> g_source_destroy(source);
> }
> +
> +unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
> + void *user_data, timeout_destroy_func_t destroy)
> +{
> + struct timeout_data *data;
> + guint id;
> +
> + data = g_try_new0(struct timeout_data, 1);
> + if (!data)
> + return 0;
> +
> + data->func = func;
> + data->destroy = destroy;
> + data->user_data = user_data;
> +
> + if (!timeout)
> + id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, timeout_callback,
> + data, timeout_destroy);
> + else
> + id = g_timeout_add_seconds_full(G_PRIORITY_DEFAULT, timeout,
> + timeout_callback, data,
> + timeout_destroy);
> + if (!id)
> + g_free(data);
> +
> + return id;
> +}
> diff --git a/src/shared/timeout-mainloop.c b/src/shared/timeout-mainloop.c
> index 5ffa65c2a..9be803cda 100644
> --- a/src/shared/timeout-mainloop.c
> +++ b/src/shared/timeout-mainloop.c
> @@ -71,3 +71,9 @@ void timeout_remove(unsigned int id)
>
> mainloop_remove_timeout((int) id);
> }
> +
> +unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
> + void *user_data, timeout_destroy_func_t destroy)
> +{
> + return timeout_add(timeout * 1000, func, user_data, destroy);
> +}
> diff --git a/src/shared/timeout.h b/src/shared/timeout.h
> index 7e22345dd..0945c3318 100644
> --- a/src/shared/timeout.h
> +++ b/src/shared/timeout.h
> @@ -16,3 +16,6 @@ typedef void (*timeout_destroy_func_t)(void *user_data);
> unsigned int timeout_add(unsigned int timeout, timeout_func_t func,
> void *user_data, timeout_destroy_func_t destroy);
> void timeout_remove(unsigned int id);
> +
> +unsigned int timeout_add_seconds(unsigned int timeout, timeout_func_t func,
> + void *user_data, timeout_destroy_func_t destroy);
> --
> 2.18.0

Applied, note that I changed the commit message a little bit.

--
Luiz Augusto von Dentz