Re: [Qemu-devel] [PATCH] Correct win32 timers deleting

[Stefan Weil]

Am 24.01.2011 15:24, schrieb Pavel Dovgaluk:
> This patch fixes resource leaks caused by quitting qemu with exit() 
> function
> on win32 host.
> Timer object should be freed not only at the end of the main function, but
> by every of the application exits.
>
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> ---
> qemu-timer.c | 14 +++++++++++---
> vl.c | 1 -
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-timer.c b/qemu-timer.c index 95814af..548f2e5 100644
> --- a/qemu-timer.c
> +++ b/qemu-timer.c
> @@ -972,7 +972,10 @@ static int win32_start_timer(struct qemu_alarm_timer
> *t)

Your patch is broken here.

> timeGetDevCaps(&tc, sizeof(tc));
>
> data->period = tc.wPeriodMin;
> - timeBeginPeriod(data->period);
> + if (timeBeginPeriod(data->period) != TIMERR_NOERROR) {
> + fprintf(stderr, "Failed to initialize win32 alarm timer\n");
> + return -1;
> + }
>
> flags = TIME_CALLBACK_FUNCTION;
> if (alarm_has_dynticks(t))
> @@ -990,6 +993,7 @@ static int win32_start_timer(struct 
> qemu_alarm_timer *t)
> fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
> GetLastError());
> timeEndPeriod(data->period);
> + data->period = 0;
> return -1;
> }
>
> @@ -1000,8 +1004,10 @@ static void win32_stop_timer(struct 
> qemu_alarm_timer
> *t) {
> struct qemu_alarm_win32 *data = t->priv;
>
> - timeKillEvent(data->timerId);
> - timeEndPeriod(data->period);
> + if (data->timerId)
> + timeKillEvent(data->timerId);
> + if (data->period)
> + timeEndPeriod(data->period);

CODING_STYLE requires {} for both if statements.

> }
>
> static void win32_rearm_timer(struct qemu_alarm_timer *t) @@ -1027,6
> +1033,7 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t)
> GetLastError());
>
> timeEndPeriod(data->period);
> + data->period = 0;
> exit(1);
> }
> }
> @@ -1061,6 +1068,7 @@ int init_timer_alarm(void)
> t->pending = 1;
> alarm_timer = t;
> qemu_add_vm_change_state_handler(alarm_timer_on_change_state_rearm, t);
> + atexit(quit_timers);
>
> return 0;
>
> diff --git a/vl.c b/vl.c
> index 0292184..c4b25b0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3118,7 +3118,6 @@ int main(int argc, char **argv, char **envp)
> os_setup_post();
>
> main_loop();
> - quit_timers();
> net_cleanup();
>
> return 0;


Please check your patch using scripts/checkpatch.pl.

Regards,
Stefan Weil