On Sun, Feb 01, 2009 at 08:02:48PM +0100, walter harms wrote:
>Hi list,
>this is latest version of a SUSv3 compliant busybox who
>i have replaced option_mask32 with a local opt. The save is impressiv.
>
>re,
> wh
>
>
>
>Final link with: <none>
>function                                             old     new   delta
>.rodata                                             1434    1418     -16
>who_main                                            1237    1168     -69
>------------------------------------------------------------------------------
>(add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-85)             Total: -85 bytes
>

Looks nice, size-wise :)
>
>/* vi: set sw=4 ts=4: */
>/*
> * SUSv3 who implementation for busybox

The current standard is SUSv4, fwiw.

> *
> * Copyright (C) 2008 by  <[email protected]>

missing real name

> * http://www.opengroup.org/onlinepubs/007904975/utilities/touch.html

wrong URL. The SUSv4 one for who would be
http://www.opengroup.org/onlinepubs/9699919799/utilities/who.html

> *
> * BB_AUDIT SUSv3 defects - missing long options

I do not see any long options mentioned in the standard?

But i do see 'who am i' in the standard but not handled below?

> * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
> * $Id: who.c,v 1.14 2009/01/30 20:31:25 walter Exp walter $

please remove that $Id

> */
>
>#include "libbb.h"
>#include <utmp.h>
>
>#define  ENABLE_FEATURE_GLOBAL_BUFFER  0
>#define  ENABLE_FEATURE_POSIX_TIMEFMT  0
>
>#if ENABLE_FEATURE_POSIX_TIMEFMT
>#define TIMEFMT       "%b %e %H:%M" 
>#else
>#define TIMEFMT       "%Y-%m-%d %T" 
>#endif
>
>/*
>  who -q file
>  who [-mu]-s[-bHlprt][<file>]
>  -a enables:  -b,-d, -l, -p, -r, -t, -T and -u 
>  no support for 'who am i' or 'mon loves'  

ah, ok. The helptext in the config should mention that too.
>*/
>
>// "%b %e %H:%M" 
>//
>// char bb_common_bufsiz1[COMMON_BUFSIZE]
>// move this to libbb ??
>
>
>#if ENABLE_FEATURE_GLOBAL_BUFFER
>
>static char *time2str(const char *fmt, time_t t)
>{
>       strftime(bb_common_bufsiz1, COMMON_BUFSIZE, fmt, localtime(&t));
>       return bb_common_bufsiz1;
>}
>#else
>static char *time2str(const char *fmt, time_t t)
>{
>       static char buf[20];
>       strftime(buf, sizeof(buf), fmt, localtime(&t));
>       return buf;
>}
>
>#endif
>
>
>#define OPT_a   1              /*  -b,-d, -l, -p, -r, -t, -T and -u  */
>#define OPT_b   2              /* last reboot */
>#define OPT_d   4              /* dead process */
>#define OPT_H   8              /* add Header */
>#define OPT_l   16             /* login process */
>#define OPT_m   32             /* current terminal only */
>#define OPT_p   64             /* processes spawn by init */
>#define OPT_q   128            /* quick */
>#define OPT_r   256            /* current runlevel */
>#define OPT_s   512            /* default */
>#define OPT_t   1024           /* last time change */
>#define OPT_T   2048           /* add time of login */
>#define OPT_u   4096           /* idle */
>#define OPT_D   8192           /* debug DUMPFILE */
>
>
>/* option -w:  like q with full info, GNU extension  */
>
>static int _stat(char *line,struct stat *st)
>{
>       int ret;
>       line = xasprintf("/dev/%s", line);
>       ret=stat(line, st);

missing spaces before and after '='

>       free(line);
>       return ret;
>}
>
>static char term_state(char *line)
>{
>       struct stat st;
>
>       if (_stat(line,&st) < 0) {
>               return '?';
>       } else {
>               if (st.st_mode & S_IWOTH)
>                       return '-';
>       }
>       return '+';
>}
>
>
>static void idle_time(char *line)
>{
>       struct stat st;
>       time_t t;
>
>       if (_stat(line, &st) < 0) 
>       {
>               putchar('?');
>               goto print;
>       }
>
>
>       /* one of the few occation we use atime */

"occasions"

>       t = time(NULL) - st.st_atime;

difftime()
>
>       if (t < 60) {
>               putchar('.');
>               goto print;
>       }
>       

whitespace damage above.

>       /* always  t >= 0 && */
>       if ( t < (24 * 60 * 60)) {
>               printf("%02d:%02d",
>                               (int) (t / (60 * 60)), (int) ((t % (60 * 60)) / 
> 60));
>
>       }
>       else
>               printf("old");
>       

ditto

>print:
>       putchar('\t');
>
>}
>
>enum {
>       NAME     = 1 << 0,
>       STATE    = 1 << 1,
>       LINE     = 1 << 2,
>       TIME     = 1 << 3,
>       ACTIVITY = 1 << 4,
>       PID      = 1 << 5,
>       COMMENT  = 1 << 6,
>       EXIT     = 1 << 7
>};
>
>
>static void print_header(int pattern)
>{
>
>#define        str             "NAME\0"\

ditto
>                       "STATE\0"\
>                       "LINE\0"\
>                       "TIME\0"\
>                       "ACTIVITY\0"\
>                       "PID\0"\
>                       "COMMENT\0"\
>                       "EXIT"
>
>       int i=0;

missing spaces

>       if (pattern < 0)
>               return;

why is this called even for pattern < 0 ?

>
>       while(pattern)  

missing space and surplus trailing space
>       {
>               if (pattern&1)
>                       printf("%s\t",nth_string(str,i));
>               

whitespace damage above.

>               pattern>>=1;

missing space.

>               i++;
>       }
>       putchar('\n');
>#undef str
>}
>
>
>/*
> print content of struct utmp, select what with mask 
>*/
>
>static void print_ut(struct utmp *ut, int mask)
>{
>#define str            "EMPTY\0"\
>                       "RUN_LVL\0"\
>                       "BOOT_TIME\0"\
>                       "NEW_TIME\0"\
>                       "OLD_TIME\0"\
>                      "INIT_PROCESS\0"\
>                       "LOGIN_PROCESS\0"\
>                      "USER_PROCESS\0"\
>                      "DEAD_PROCESS\0"\
>                      "ACCOUNTING"
>
>/*
>   10 = number of strings in str
>*/
>
>
>       if ( option_mask32 & OPT_D)
>               printf("%s\t", nth_string(str, ut->ut_type % 10 ) );
>#undef str
>
>       if (mask & NAME)
>               printf("%s\t", ut->ut_user);    /* NAME */
>
>       if (mask & STATE)
>               printf("%c\t", term_state(ut->ut_line));        /* STATE */
>
>       if (mask & LINE)
>               printf("%s\t", ut->ut_line);    /* LINE */
>
>       if (mask & TIME)
>               printf("%s\t", time2str(TIMEFMT, ut->ut_tv.tv_sec));    /* TIME 
> */
>
>       if (mask & ACTIVITY) 
>               idle_time(ut->ut_line);          /* ACTIVITY */
>       
>       if (mask & PID)
>               printf("%d\t", ut->ut_pid);     /* PID */
>
>       if (mask & COMMENT) 
>               printf("ID=%s\t%s\t", ut->ut_id, ut->ut_host);  /* COMMENT */
>       
>
>       if (mask & EXIT) 
>               printf("%d/%d", ut->ut_exit.e_termination, ut->ut_exit.e_exit); 
> /* EXIT */
>       

whitespace damage above, several.

>       putchar('\n');
>}
>
>int who_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
>
>int who_main(int argc UNUSED_PARAM, char **argv)
>{
>       struct utmp *ut;
>       char *name;
>       unsigned int ucnt = 0;
>       int pattern = -1;
>       unsigned int opt;
>       /*
>          FIXME: make better use of opt_complementary
>        */
>       //      opt_complementary = "q-abdHlmqprstuD";
>
>       opt=getopt32(argv, "abdHlmpqrstTuD");
>       argv += optind;
>/*
>  utmpname() will not fail if the file does not exists
>*/
>       if (*argv) 
>               utmpname(*argv);

perhaps utmpname returning <0 should set EXIT_FAILURE? The big one on my
box doesn't seem to do that but i would have expected 'who /nope' to fail.

>
>       if (opt == 0)
>               opt = OPT_s;
>       if (opt == OPT_u)
>               opt |= OPT_s;
>
>       if (opt & OPT_a)
>               opt |=(OPT_b | OPT_d | OPT_l | OPT_p | OPT_r | OPT_T | OPT_u);
>
>       /*
>          figure out pattern to figure out pattern for heading
>        */
>
>       if (opt & OPT_T)
>               pattern =
>                       (opt & OPT_u) ? NAME | STATE | LINE | TIME | PID |
>                       COMMENT | ACTIVITY : NAME | STATE | LINE | TIME | PID | 
> COMMENT;
>
>       else if (opt & OPT_s)
>               pattern = (opt & OPT_u) ? NAME | LINE | TIME | ACTIVITY
>                       : NAME | LINE | TIME;
>
>
>       else if (opt & OPT_l)
>               pattern =
>                       (opt & OPT_u) ? NAME | LINE | TIME | PID | COMMENT |
>                       ACTIVITY : NAME | LINE | TIME | PID | COMMENT;
>
>       else if (opt & OPT_p)
>               pattern =
>                       (opt & OPT_u) ? NAME | LINE | TIME | PID | COMMENT |
>                       EXIT | ACTIVITY : NAME | LINE | TIME | PID | COMMENT | 
> EXIT;
>       else if (opt & OPT_D)
>               pattern = NAME | LINE | TIME | PID | COMMENT |EXIT;

could be that it's cheaper to set the unconditional stuff (e.g. NAME) once, 
before that block.
>
>       if (opt & OPT_m) {
>               name = ttyname(STDIN_FILENO);
>               if (!name || strncmp(name, "/dev/", 5) )
>                       bb_perror_msg_and_die("ttyname()");
>               name += 5;
>               /*
>                 else 
>                 something strange is going on
>                 can this ever happen ?
>               */
>
>       }
>
>       setutent();
>
>       if (opt & OPT_H)
>               print_header(pattern);

I'd just test pattern here and don't call it otherwise?

>
>       while ((ut = getutent()) != NULL) {
>
>               /* -m : only current terminal */
>               if (opt & OPT_m && strcmp(name, ut->ut_line))
>                       continue;
>
>               /* -b */
>               if ((opt & OPT_b) && BOOT_TIME == ut->ut_type) {
>                       if (opt & OPT_H)
>                               printf("last reboot\t");
>
>                       print_ut(ut, TIME);
>
>                       /*
>                          if OPT_a is set we have to show this here only once
>                          and continue to handle the other options
>                        */
>
>                       if (opt & OPT_a)
>                               opt &= ~OPT_b;
>                       else
>                               break;
>               }
>
>               /* -d */
>               if ((opt & OPT_d) && DEAD_PROCESS == ut->ut_type) {
>                       print_ut(ut, pattern);
>               }
>
>               /* -l */
>               if ((opt & OPT_l) && LOGIN_PROCESS == ut->ut_type) {
>                       print_ut(ut, pattern);
>               }
>
>               /* -p */
>               if ((opt & OPT_p) && INIT_PROCESS == ut->ut_type) {
>                       print_ut(ut, pattern);
>               }

perhaps cheaper to merge these three above?
>
>               /* -q */
>               if ((opt & OPT_q) && USER_PROCESS == ut->ut_type) {
>                       printf("%s ", ut->ut_user);
>                       ucnt++;
>               }
>
>               /* -r */
>               if ((opt & OPT_r) && RUN_LVL == ut->ut_type) {
>                       if (opt & OPT_H) 
>                               printf("Current\tTime\t\tLast\n");
>                       

whitespace damage. You've seen our .indentpro script, didn't you. Just run 
indent
on that whole file.

>                       printf("%s %c\t%s\t%c\n",
>                              ut->ut_user, ut->ut_pid & 255,
>                              time2str(TIMEFMT, ut->ut_tv.tv_sec),
>                              (ut->ut_pid >> 8) ? 'N' : (ut->ut_pid >> 8) );
>
>                       if (opt & OPT_a)
>                               opt &= ~OPT_r;
>                       else
>                               break;
>               }
>
>               /* -s | default */
>               if ((opt & OPT_s) && USER_PROCESS == ut->ut_type) {
>                       print_ut(ut, pattern);
>               }
>
>               /* -t */
>               if ((opt & OPT_t) && NEW_TIME == ut->ut_type) {
>                       if (opt & OPT_H)
>                               printf("last time change\t");
>                       print_ut(ut, TIME);
>                       break;
>               }
>
>               /* -T */
>               if ((opt & OPT_T) && USER_PROCESS == ut->ut_type) {
>                       print_ut(ut, pattern);
>               }
>
>                /* -D debug option */
>               if (opt & OPT_D)
>                       print_ut(ut,pattern);
>
>       } /* while */
>
>/*
>  write user count
>*/
>
>       if (opt & OPT_q)
>               printf("\n#User %d\n", ucnt);
>
>       if (ENABLE_FEATURE_CLEAN_UP) 
>               endutent();
>
>
>       return EXIT_SUCCESS;
>}
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to