[patch] last_fancy

[Cristian Ionescu-Idbohrn]

On Sun, 30 Nov 2008, Denys Vlasenko wrote:

> On Thursday 27 November 2008 23:58, Cristian Ionescu-Idbohrn wrote:
> > Just a simple cleanup patch.
> > Please consider applying.
>
> -       opt = getopt32(argv, "Wf:" /* "H" */, &filename);
> -#ifdef BUT_UTIL_LINUX_LAST_HAS_NO_SUCH_OPT
> +       opt = getopt32(argv, USE_FEATURE_LAST_FANCY_SHOW_HEADER("H") "Wf:",
> +                      &filename);
> +#ifdef ENABLE_FEATURE_LAST_FANCY_SHOW_HEADER
>
> Support for -H seems to be commented out for a reason:
> there is no such option in "standard" last.

Fine with me.  Let's completly remove that code then.
Should similar code be removed from last.c too?

> -//     if (days) {
> +       if (ENABLE_FEATURE_LAST_FANCY_TIME_DAYS || days) {
>                 sprintf(duration, "(%u+%02u:%02u)", days, hours, mins);
> -//     } else {
> -//             sprintf(duration, " (%02u:%02u)", hours, mins);
> -//     }
> +       } else {
> +               sprintf(duration, " (%02u:%02u)", hours, mins);
> +       }
>
> I am not totally against sacrificing a few bytes here
> and print (20:53) instead of (0+20:53).

We could even get around that by using #ifdef around the controversial
code.

> But you do need to try to convince me why. :) Do you think people
> will have problems with the concept of zero days?

My main motivation is to imitate sysvinit /usr/bin/last.
Secondly, I want to avoid patching the code for each import.
Having things like that (we sometimes disagree upon) configurable at
build time saves IMO a lot of grief.
Thirdly, the code is cleaner.

> Or are you saying "standard" last just prints 1234:53 for
> logins spanning many days? If yes, we might just do that,
> for compatibility. (I don't have "standard" last here to test).

Here is an example of what sysvinit 'last' shows:

user     pts/34       :0               Tue Nov 11 15:51 - 16:07  (00:16)
user     pts/33       :0               Tue Nov 11 15:35 - 16:03 (17+00:28)

> Omitting days and just printing (hours mod 24) would be
> bad enough to not do it even in the name of compat.
> We already have such problem with years.

Yes.  But wtmp files are rotated.

> Real output on my machine:
>
> root     tty2                          Thu Jan  3 00:23 - 21:16 (0+20:53)
> root     tty2                          Wed Jan  2 06:30 - 00:23 (0+17:52)
> root     tty2                          Tue Jan  1 03:04 - 06:30 (1+03:25)
> root     tty2                          Mon Dec 31 05:20 - 03:04 (0+21:44)
> root     tty2                          Sun Dec 30 03:51 - 05:20 (1+01:29)
> root     tty2                          Sat Dec 29 07:07 - 03:51 (0+20:43)
>
> Ok, so it was a transition between years. WHICH YEARS?
> Why this info is not shown?

I agree with you, but what do you do?  That's how sysvinit 'last'
behaves, and until that improves the best we can do is imitate.

> This is a bigger problem than "do we want to print zero days".

Sure.


Cheers,

-- 
Cristian