diff mbox series

[2/2] qemu/timer: Sanity check timer_list in timer_init_full()

Message ID 20250125182425.59708-3-philmd@linaro.org
State New
Headers show
Series qemu/timer: Clarify QEMUTimer new/free API | expand

Commit Message

Philippe Mathieu-Daudé Jan. 25, 2025, 6:24 p.m. UTC
Ensure we are not re-initializing a QEMUTimer already added
to an active list. timer_init*() functions expect either
a recently created and zeroed QEMUTimer, or one previously
free'd with timer_free().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/timer.h | 2 +-
 util/qemu-timer.c    | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Daniel P. Berrangé Feb. 6, 2025, 10:40 a.m. UTC | #1
On Sat, Jan 25, 2025 at 07:24:25PM +0100, Philippe Mathieu-Daudé wrote:
> Ensure we are not re-initializing a QEMUTimer already added
> to an active list. timer_init*() functions expect either
> a recently created and zeroed QEMUTimer, or one previously
> free'd with timer_free().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/qemu/timer.h | 2 +-
>  util/qemu-timer.c    | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
Michael Tokarev Feb. 9, 2025, 9:37 a.m. UTC | #2
25.01.2025 21:24, Philippe Mathieu-Daudé wrote:

> - * You need not call an explicit deinit call. Simply make
> + * You need not call an explicit timer_deinit() call. Simply make
>    * sure it is not on a list with timer_del.

Reworded this as "You need not call timer_deinit() explicitly. Simply make..."

and applied to trivial-patches tree.

Thanks,

/mjt
Michael Tokarev Feb. 9, 2025, 9:41 a.m. UTC | #3
09.02.2025 12:37, Michael Tokarev wrote:
> 25.01.2025 21:24, Philippe Mathieu-Daudé wrote:
> 
>> - * You need not call an explicit deinit call. Simply make
>> + * You need not call an explicit timer_deinit() call. Simply make
>>    * sure it is not on a list with timer_del.
> 
> Reworded this as "You need not call timer_deinit() explicitly. Simply make..."

"You don't need to call timer_deinit() explicitly.", actually.

This breaks quite a lot of CI tests: https://gitlab.com/mjt0k/qemu/-/pipelines/1662551717
Philippe Mathieu-Daudé Feb. 9, 2025, 5:41 p.m. UTC | #4
On 9/2/25 10:41, Michael Tokarev wrote:
> 09.02.2025 12:37, Michael Tokarev wrote:
>> 25.01.2025 21:24, Philippe Mathieu-Daudé wrote:
>>
>>> - * You need not call an explicit deinit call. Simply make
>>> + * You need not call an explicit timer_deinit() call. Simply make
>>>    * sure it is not on a list with timer_del.
>>
>> Reworded this as "You need not call timer_deinit() explicitly. Simply 
>> make..."
> 
> "You don't need to call timer_deinit() explicitly.", actually.
> 
> This breaks quite a lot of CI tests: https://gitlab.com/mjt0k/qemu/-/ 
> pipelines/1662551717

Do sorry, I only tested on a specific config and missed all the
other errors :/
diff mbox series

Patch

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index abd2204f3be..4717693f950 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -407,7 +407,7 @@  int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg);
  * (or default timer list group, if NULL).
  * The caller is responsible for allocating the memory.
  *
- * You need not call an explicit deinit call. Simply make
+ * You need not call an explicit timer_deinit() call. Simply make
  * sure it is not on a list with timer_del.
  */
 void timer_init_full(QEMUTimer *ts,
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 0e8a453eaa1..058cae6e487 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -354,6 +354,7 @@  void timer_init_full(QEMUTimer *ts,
     if (!timer_list_group) {
         timer_list_group = &main_loop_tlg;
     }
+    assert(ts->timer_list == NULL);
     ts->timer_list = timer_list_group->tl[type];
     ts->cb = cb;
     ts->opaque = opaque;