Bernhard Reutner-Fischer schrieb:
> 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
>
i work after that specs, did they change something ?
>> *
>> * 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
>
please after this is moved to bb svn. it is my cvs and i need them to keep track
about the changes.
>> */
>>
>> #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.
i hope to add this for 1.0 but first the functions should work
i will fix also "mom loves".
>> */
>>
>> // "%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.
>
void utmpname(const char *file);
>> 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.
i do not understand. do you suggest something like ..
pattern=NAME
if (foo)
pattern|=LINE;
>> 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?
pattern is used by print_header so what what header infos are needed , is that
the question ?
>
>> 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?
i would like to leave it for 1.0 when the functions here are checked.
>> /* -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.
>
actualy i did some time ago, it seems i added some spaces
i will rerun indent again and resend
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
