Re: address@hidden: uptime on GNU]

[Paul Eggert]

"Alfred M. Szmidt" <address@hidden> writes:

> +AC_DEFUN([gl_LIB_PS_CHECK],
> +[
> +  ac_saved_libs="$LIBS"
> +  AC_SEARCH_LIBS(ps_context_create, [ps],
> +                 [test "$ac_cv_search_ps_context_create" = "none required" ||
> +                  LIB_PS=$ac_cv_search_ps_context_create])
> +  AC_SUBST(LIB_PS)
> +  AC_CHECK_FUNCS(ps_context_create)

Doesn't this set both LIB_PS and LIBS?  Will that cause problems?

> +#if HAVE_PS_CONTEXT_CREATE
> +/* Find the absolute timestamp of when the system was booted.  The GNU
> +   system defines "system boot time" as the task creation time of PID
> +   1 (init).  */
> +static error_t
> +fetch_boot_time (struct timeval *when)
> +{
> +  error_t err = 0;
> +  struct ps_context *context;
> +  struct proc_stat *ps;
> +  
> +  err = ps_context_create (getproc (), &context);
> +  if (err)
> +    return err;
> +  
> +  err = ps_context_find_proc_stat (context, 1, &ps);
> +  if (err)
> +    return err;

Doesn't this have a leak?  That is, don't you need
to free CONTEXT before returning ERR here?


> +      time_value_t *const tv = &proc_stat_task_basic_info 
> (ps)->creation_time;

Surely this should be "time_value_t const *", not "time_value_t
*const".  There is usually little point to declaring a local var to be
const, as you (or the compiler) can easily see that it's const.


> +#if HAVE_PS_CONTEXT_CREATE
> +  {
> +    struct timeval result;
> +    if (fetch_boot_time (&result))
> +      boot_time = 0;
> +    else
> +      boot_time = result.tv_sec;
> +  }
> +#endif

I don't see why the "boot_time = 0" is needed here.  How about
something like this instead?

   #if HAVE_PS_CONTEXT_CREATE
     {
       struct timeval result;
       if (fetch_boot_time (&result) == 0)
         boot_time = result.tv_sec;
     }
   #endif

The proposed patch sometimes generates a result that is off by 1.  If
the boot time is 1.9 seconds after the epoch, and the current time is
2.1 seconds after the epoch, it will report an uptime of 1 second (2.0
- 1.0) instead of the correct 0 seconds (2.1 - 1.9, then truncated).