Re: [systemd-devel] [PATCH] timedatectl: work with old timedated

2013-12-12 Thread Lennart Poettering 
On Wed, 11.12.13 14:04, Shawn Landden (sh...@churchofgit.com) wrote:

 Which does have TimeUSec. Should we specifically check for this method
 instead of assuming time=0 means it doesn't exist?

Sounds OK to just check against time == 0.

  
 +if (i-time)

For numeric values please always check with explicit numerical
comparison. Something like this:

if (i-time  0) 

or so..

 +sec = (time_t) (i-time / USEC_PER_SEC);
 +else if (arg_transport == BUS_TRANSPORT_LOCAL)
 +sec = time(NULL);
 +else
 +return (void)fprintf(stderr, Could not get time from
 timedated and not operating locally.\n\n);

I am pretty sure we shouldn't exit here. We should instead show a line
of n/a or so for this value. 

Also please do not do stuff like return (void) printf()... We are not
in a contest to write non-obvious C code

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedatectl: work with old timedated

2013-12-12 Thread Lennart Poettering 
On Thu, 12.12.13 10:00, Shawn Landden (sh...@churchofgit.com) wrote:


Applied. Dropped the fflush(stderr) bits though as we that's not
necessary for stderr, and not even for stdout if an \n was printed
anyway...

Thanks!

 
 diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
 index 9b81513..6b09559 100644
 --- a/src/timedate/timedatectl.c
 +++ b/src/timedate/timedatectl.c
 @@ -98,6 +98,7 @@ static void print_status_info(const StatusInfo *i) {
  char s[32];
  struct tm tm;
  time_t sec;
 +bool have_time = false;
  char *zc, *zn;
  time_t t, tc, tn;
  int dn;
 @@ -109,20 +110,35 @@ static void print_status_info(const StatusInfo *i) {
  /* Enforce the values of /etc/localtime */
  if (getenv(TZ)) {
  fprintf(stderr, Warning: ignoring the TZ variable, reading 
 the system's timezone setting only.\n\n);
 +fflush(stderr);
  unsetenv(TZ);
  }
  
 -sec = (time_t) (i-time / USEC_PER_SEC);
 +if (i-time != 0) {
 +sec = (time_t) (i-time / USEC_PER_SEC);
 +have_time = true;
 +} else if (arg_transport == BUS_TRANSPORT_LOCAL) {
 +sec = time(NULL);
 +have_time = true;
 +} else {
 +fprintf(stderr, Warning: could not get time from timedated 
 and not operating locally.\n\n);
 +fflush(stderr);
 +}
  
 -zero(tm);
 -assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S %Z, 
 localtime_r(sec, tm))  0);
 -char_array_0(a);
 -printf(  Local time: %s\n, a);
 +if (have_time) {
 +zero(tm);
 +assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S %Z, 
 localtime_r(sec, tm))  0);
 +char_array_0(a);
 +printf(  Local time: %s\n, a);
  
 -zero(tm);
 -assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S UTC, 
 gmtime_r(sec, tm))  0);
 -char_array_0(a);
 -printf(  Universal time: %s\n, a);
 +zero(tm);
 +assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S UTC, 
 gmtime_r(sec, tm))  0);
 +char_array_0(a);
 +printf(  Universal time: %s\n, a);
 +} else {
 +printf(  Local time: %s\n, n/a);
 +printf(  Universal time: %s\n, n/a);
 +}
  
  if (i-rtc_time  0) {
  time_t rtc_sec;
 @@ -133,7 +149,7 @@ static void print_status_info(const StatusInfo *i) {
  char_array_0(a);
  printf(RTC time: %s\n, a);
  } else
 -printf(RTC time: n/a\n);
 +printf(RTC time: %s\n, n/a);
  
  zero(tm);
  assert_se(strftime(a, sizeof(a), %Z, %z, localtime_r(sec, tm))  
 0);
 @@ -151,8 +167,8 @@ static void print_status_info(const StatusInfo *i) {
   tc, zc, is_dstc,
   tn, dn, zn, is_dstn);
  if (r  0)
 -printf(  DST active: n/a\n);
 -else {
 +printf(  DST active: %s\n, n/a);
 +else if (have_time) {
  printf(  DST active: %s\n, yes_no(is_dstc));
  
  t = tc - 1;
 @@ -183,7 +199,8 @@ static void print_status_info(const StatusInfo *i) {
  
  free(zc);
  free(zn);
 -}
 +} else
 +printf(  DST active: %s\n, yes_no(is_dstc));
  
  if (i-rtc_local)
  fputs(\n ANSI_HIGHLIGHT_ON


Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedatectl: work with old timedated

2013-12-12 Thread Shawn Landden 
On Thu, Dec 12, 2013 at 11:02 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Thu, 12.12.13 10:00, Shawn Landden (sh...@churchofgit.com) wrote:


 Applied. Dropped the fflush(stderr) bits though as we that's not
 necessary for stderr, and not even for stdout if an \n was printed
 anyway...
ok, I was worried about the lack of synchronization between stderr and
stdout causing them to interlace.

 Thanks!


 diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
 index 9b81513..6b09559 100644
 --- a/src/timedate/timedatectl.c
 +++ b/src/timedate/timedatectl.c
 @@ -98,6 +98,7 @@ static void print_status_info(const StatusInfo *i) {
  char s[32];
  struct tm tm;
  time_t sec;
 +bool have_time = false;
  char *zc, *zn;
  time_t t, tc, tn;
  int dn;
 @@ -109,20 +110,35 @@ static void print_status_info(const StatusInfo *i) {
  /* Enforce the values of /etc/localtime */
  if (getenv(TZ)) {
  fprintf(stderr, Warning: ignoring the TZ variable, reading 
 the system's timezone setting only.\n\n);
 +fflush(stderr);
  unsetenv(TZ);
  }

 -sec = (time_t) (i-time / USEC_PER_SEC);
 +if (i-time != 0) {
 +sec = (time_t) (i-time / USEC_PER_SEC);
 +have_time = true;
 +} else if (arg_transport == BUS_TRANSPORT_LOCAL) {
 +sec = time(NULL);
 +have_time = true;
 +} else {
 +fprintf(stderr, Warning: could not get time from timedated 
 and not operating locally.\n\n);
 +fflush(stderr);
 +}

 -zero(tm);
 -assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S %Z, 
 localtime_r(sec, tm))  0);
 -char_array_0(a);
 -printf(  Local time: %s\n, a);
 +if (have_time) {
 +zero(tm);
 +assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S %Z, 
 localtime_r(sec, tm))  0);
 +char_array_0(a);
 +printf(  Local time: %s\n, a);

 -zero(tm);
 -assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S UTC, 
 gmtime_r(sec, tm))  0);
 -char_array_0(a);
 -printf(  Universal time: %s\n, a);
 +zero(tm);
 +assert_se(strftime(a, sizeof(a), %a %Y-%m-%d %H:%M:%S 
 UTC, gmtime_r(sec, tm))  0);
 +char_array_0(a);
 +printf(  Universal time: %s\n, a);
 +} else {
 +printf(  Local time: %s\n, n/a);
 +printf(  Universal time: %s\n, n/a);
 +}

  if (i-rtc_time  0) {
  time_t rtc_sec;
 @@ -133,7 +149,7 @@ static void print_status_info(const StatusInfo *i) {
  char_array_0(a);
  printf(RTC time: %s\n, a);
  } else
 -printf(RTC time: n/a\n);
 +printf(RTC time: %s\n, n/a);

  zero(tm);
  assert_se(strftime(a, sizeof(a), %Z, %z, localtime_r(sec, tm)) 
  0);
 @@ -151,8 +167,8 @@ static void print_status_info(const StatusInfo *i) {
   tc, zc, is_dstc,
   tn, dn, zn, is_dstn);
  if (r  0)
 -printf(  DST active: n/a\n);
 -else {
 +printf(  DST active: %s\n, n/a);
 +else if (have_time) {
  printf(  DST active: %s\n, yes_no(is_dstc));

  t = tc - 1;
 @@ -183,7 +199,8 @@ static void print_status_info(const StatusInfo *i) {

  free(zc);
  free(zn);
 -}
 +} else
 +printf(  DST active: %s\n, yes_no(is_dstc));

  if (i-rtc_local)
  fputs(\n ANSI_HIGHLIGHT_ON


 Lennart

 --
 Lennart Poettering, Red Hat
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel