From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=44511 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PhPx6-0004we-VJ
	for qemu-devel@nongnu.org; Mon, 24 Jan 2011 12:11:42 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Stefan.Weil@weilnetz.de>) id 1PhPx4-0005ih-Vz
	for qemu-devel@nongnu.org; Mon, 24 Jan 2011 12:11:40 -0500
Received: from moutng.kundenserver.de ([212.227.17.8]:65009)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Stefan.Weil@weilnetz.de>) id 1PhPx4-0005ej-Ih
	for qemu-devel@nongnu.org; Mon, 24 Jan 2011 12:11:38 -0500
Message-ID: <4D3DB2A8.1010002@weilnetz.de>
Date: Mon, 24 Jan 2011 18:11:04 +0100
From: Stefan Weil <Stefan.Weil@weilnetz.de>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH] Correct win32 timers deleting
References: <001a01cbbbd2$5e0a4a10$1a1ede30$@Dovgaluk@ispras.ru>
In-Reply-To: <001a01cbbbd2$5e0a4a10$1a1ede30$@Dovgaluk@ispras.ru>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Pavel Dovgaluk <Pavel.Dovgaluk@ispras.ru>
Cc: qemu-devel@nongnu.org

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 <pavel.dovgaluk@gmail.com>
> ---
> 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