[systemd-devel] [PATCH] fix bogus boot error message

[Kelly Anderson]

On 07/19/11 06:29, Kay Sievers wrote:
> On Tue, Jul 19, 2011 at 14:20, Kelly Anderson
> <kelly at silka.with-linux.com>  wrote:
>> On 07/19/11 06:04, Kay Sievers wrote:
>>> On Tue, Jul 19, 2011 at 13:22, Kelly Anderson
>>> <kelly at silka.with-linux.com>    wrote:
>>>> The following commit introduces a bogus error message on boot since
>>>> localtime may be a negative number, i.e. GMT-7.
>>>> The following patch just uses errno directly rather than using it
>>>> indirectly
>>>> through hwclock_apply_localtime_delta():
>>> Ah, tm->tm_gmtoff can be negative.
>>>
>>>> +       errno = 0;
>>>>           min = hwclock_apply_localtime_delta();
>>> We usually prefer to return the errno instead of mangling it at the
>>> caller's side, and have the values that can be negative passed as
>>> arguments. Mind switching that? Or let me know.
>>>
>>> Thanks,
>>> Kay
>> Something along the lines of:
>>
>> int status = hwclock_apply_localtime_delta(&min)
>>
>> Is that what you mean?  If that's what you're looking for, I could do that.
> Yeah, we like to return errno-like values directly wherever possible.
> It's a bit like the kernel's function style. Only things that can only
> fail in one way only, like ENOMEM, can return the object or the NULL
> pointer.
>
> That means we need to pass the values as arguments. In timedated.c the
> value isn't even used, so allowing hwclock_apply_localtime_delta(NULL)
> might be nice.
>
> Kay
>
> Kay

OK,

Here's a patch that should have the desired results:

--- ./src/util.h.orig    2011-07-11 20:07:58.000000000 -0600
+++ ./src/util.h    2011-07-19 07:24:50.593040702 -0600
@@ -435,7 +435,7 @@ int conf_files_list(char ***strv, const

  int hwclock_is_localtime(void);

-int hwclock_apply_localtime_delta(void);
+int hwclock_apply_localtime_delta(int *min);
  int hwclock_reset_localtime_delta(void);
  int hwclock_get_time(struct tm *tm);
  int hwclock_set_time(const struct tm *tm);
--- ./src/main.c.orig    2011-07-11 20:07:58.000000000 -0600
+++ ./src/main.c    2011-07-19 07:28:27.362064963 -0600
@@ -1058,9 +1058,9 @@ int main(int argc, char *argv[]) {
                  if (hwclock_is_localtime() > 0) {
                          int min;

-                        min = hwclock_apply_localtime_delta();
-                        if (min < 0)
-                                log_error("Failed to apply local time 
delta: %s", strerror(-min));
+                        int rc = hwclock_apply_localtime_delta(&min);
+                        if (rc < 0)
+                                log_error("Failed to apply local time 
delta: %s", strerror(-rc));
                          else
                                  log_info("RTC configured in localtime, 
applying delta of %i minutes to system time.", min);
                  }
--- ./src/util.c.orig    2011-07-11 20:07:58.000000000 -0600
+++ ./src/util.c    2011-07-19 07:29:04.300728483 -0600
@@ -4905,7 +4905,7 @@ int hwclock_is_localtime(void) {
          return local;
  }

-int hwclock_apply_localtime_delta(void) {
+int hwclock_apply_localtime_delta(int *min) {
          const struct timeval *tv_null = NULL;
          struct timespec ts;
          struct tm *tm;
@@ -4916,6 +4916,9 @@ int hwclock_apply_localtime_delta(void)
          assert_se(tm = localtime(&ts.tv_sec));
          minuteswest = tm->tm_gmtoff / 60;

+        if ( min )
+                *min = minuteswest;
+
          tz.tz_minuteswest = -minuteswest;
          tz.tz_dsttime = 0; /* DST_NONE*/

@@ -4924,10 +4927,7 @@ int hwclock_apply_localtime_delta(void)
           * The very first time we set the kernel's timezone, it will warp
           * the clock so that it runs in UTC instead of local time.
           */
-        if (settimeofday(tv_null, &tz) < 0)
-                return -errno;
-
-        return minuteswest;
+        return (settimeofday(tv_null, &tz) < 0) ? -errno : 0;
  }

  int hwclock_reset_localtime_delta(void) {
--- ./src/timedated.c.orig    2011-06-29 17:59:24.000000000 -0600
+++ ./src/timedated.c    2011-07-19 07:27:24.959633340 -0600
@@ -332,7 +332,7 @@ static DBusHandlerResult timedate_messag
                                  struct tm *tm;

                                  /* 2. Teach kernel new timezone */
-                                hwclock_apply_localtime_delta();
+                                hwclock_apply_localtime_delta(NULL);

                                  /* 3. Sync RTC from system clock, with 
the new delta */
                                  
assert_se(clock_gettime(CLOCK_REALTIME, &ts) == 0);
@@ -382,7 +382,7 @@ static DBusHandlerResult timedate_messag

                          /* 2. Teach kernel new timezone */
                          if (local_rtc)
-                                hwclock_apply_localtime_delta();
+                                hwclock_apply_localtime_delta(NULL);
                          else
                                  hwclock_reset_localtime_delta();