[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] rtc: fix 12-hour mode
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] rtc: fix 12-hour mode |
Date: |
Sun, 01 Jan 2012 16:53:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111115 Thunderbird/8.0 |
On 11/21/2011 08:00 PM, Paolo Bonzini wrote:
> Hours in 12-hour mode are in the 1-12 range, not 0-11.
>
> @@ -320,7 +324,8 @@ static void rtc_copy_date(RTCState *s)
> s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, tm->tm_hour);
> } else {
> /* 12 hour format */
> - s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, tm->tm_hour % 12);
> + int h = (tm->tm_hour % 12) ? tm->tm_hour % 12 : 12;
> + s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, h);
> if (tm->tm_hour >= 12)
> s->cmos_data[RTC_HOURS] |= 0x80;
> }
Nitpick, don't update patch on this account:
I dislike seeing int-to-bool conversion on anything that is not a
true/false or count value. Things like if (has_some_property) or if
(!nr_items) read well and are easily understood. But here, you're not
checking for "are there any tm_hours, if yes, use them, if not, use
12". You're testing against the value 0 which has a special encoding in
12 hour mode.
The is usually manifested in
if (!strcmp(a, b)) ...
strcmp() does not return a bool or a count, and in fact it reads exactly
the opposite of the intent: "if not string compare". strcmp() returns
an enumeration, or perhaps a mapping of string trichotomy to integer
trichotomy.
Sorry about the pontification, back to the regularly scheduled
whitespace discussion.
--
error compiling committee.c: too many arguments to function
- Re: [Qemu-devel] [PATCH 1/4] rtc: fix 12-hour mode,
Avi Kivity <=