diff mbox

[03/19] avoid dubiously clever code in win32_start_timer

Message ID 1261382970-23251-4-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Dec. 21, 2009, 8:09 a.m. UTC
The code is initializing an unsigned int to UINT_MAX using "-1", so that
the following always-true comparison seems to be always-false at a
first look.  Just remove the if.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

Comments

Paolo Bonzini Jan. 4, 2010, 6:39 p.m. UTC | #1
On 01/04/2010 08:34 PM, Anthony Liguori wrote:
> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
>> The code is initializing an unsigned int to UINT_MAX using "-1", so that
>> the following always-true comparison seems to be always-false at a
>> first look. Just remove the if.
>
> This really needs a better comment message as at first it wasn't obvious
> to me that this was correct.
>
> start_timer is only ever called once and as such, the check here is
> unnecessary. It's not incorrect, it's just more robust than it needs to be.
>
> I'm not sure it's really worth removing to be honest.

I didn't like the comparison with -1 being always true.  I'll update 
with a comment.

Paolo
Anthony Liguori Jan. 4, 2010, 7:34 p.m. UTC | #2
On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> The code is initializing an unsigned int to UINT_MAX using "-1", so that
> the following always-true comparison seems to be always-false at a
> first look.  Just remove the if.

This really needs a better comment message as at first it wasn't obvious 
to me that this was correct.

start_timer is only ever called once and as such, the check here is 
unnecessary.  It's not incorrect, it's just more robust than it needs to be.

I'm not sure it's really worth removing to be honest.

Regards,

Anthony Liguori
diff mbox

Patch

diff --git a/vl.c b/vl.c
index 97410ad..22ec53d 100644
--- a/vl.c
+++ b/vl.c
@@ -813,7 +813,7 @@  static struct qemu_alarm_timer *alarm_timer;
 struct qemu_alarm_win32 {
     MMRESULT timerId;
     unsigned int period;
-} alarm_win32_data = {0, -1};
+} alarm_win32_data = {0, 0};
 
 static int win32_start_timer(struct qemu_alarm_timer *t);
 static void win32_stop_timer(struct qemu_alarm_timer *t);
@@ -1550,9 +1550,7 @@  static int win32_start_timer(struct qemu_alarm_timer *t)
     memset(&tc, 0, sizeof(tc));
     timeGetDevCaps(&tc, sizeof(tc));
 
-    if (data->period < tc.wPeriodMin)
-        data->period = tc.wPeriodMin;
-
+    data->period = tc.wPeriodMin;
     timeBeginPeriod(data->period);
 
     flags = TIME_CALLBACK_FUNCTION;