| Message ID | cover.1539259394.git.artem.k.pisarenko@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix and improve core RTC function and documentation | expand |
I checked it with ./configure --enable-debug and didn't imagined that in
non-debug build (with -O2) compiler will complain:
"vl.c:847:12: error: ‘seconds’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]"
int qemu_timedate_diff(struct tm *tm)
{
time_t seconds;
switch (rtc_base_type) {
case RTC_BASE_DATETIME:
case RTC_BASE_UTC:
seconds = mktimegm(tm);
break;
case RTC_BASE_LOCALTIME:
{
struct tm tmp = *tm;
tmp.tm_isdst = -1; /* use timezone to figure it out */
seconds = mktime(&tmp);
break;
}
}
>>> return seconds - qemu_ref_timedate(QEMU_CLOCK_HOST);
}
Switch covers all defined enum values, so 'second' cannot be left
uninitialized. One way it could be is if someone assigns integer value to
'rtc_base_type' variable explicitly. Ok, let's look at it. This variable
initialized with valid value at definition and elsewhere in code it's being
assigned only valid enum values. Maybe it could be changed outside of
compile unit ? No, it's defined as static. Maybe GCC isn't just smart
enough to check it in whole compile unit scope context ? From my expirience
I've seen that it's able to make even more complex assumptions and smart
decisions. And with higher optimization levels GCC becomes just smarter.
But this time GCC behavior differs...
Modifications are motivated by bug https://bugs.launchpad.net/qemu/+bug/1797033 I've encountered recently. Trying to fix it and analyzing its effect on all use cases (not covered in bug report) revealed much deeper problems. This is my first patch to QEMU and I'm not sure whether the way I addressed them is proper. They definitely require either modifications to current implementation (breaking compability), or adding clarifications to user documentation. I've tried to find compromise. Changes I propose are driven mostly by my interpretation of user documentation and comments in code, rather than by existing implementation. I've splitted patches in a way that at least some of them may be accepted. Following subsets are make sense on their own: - 1 - 1,2,3 I limited refactoring only to vl.c, although it leads to reworking of all hw/* rtc models which have unreasonable duplications of actions performed by vl.c. I'm sure such kind of rework will reveal more hidden bugs/features, but so far I've had enough (it's a long story how muсh effort and pain cost me to find that tricky bug above). I suppose just to draw attention of relevant hw/* maintainers. Artem Pisarenko (4): vl: improve/fix documentation related to RTC function vl: refactor -rtc option references Fixes RTC bug with base datetime shifts in clock=vm vl,qapi: offset value calculation in RTC_CHANGE event reverted to match behavior before #1797033 bugfix and documented qapi/misc.json | 3 +- qemu-options.hx | 14 +++++--- vl.c | 102 ++++++++++++++++++++++++++++++++++++-------------------- 3 files changed, 76 insertions(+), 43 deletions(-)