SUSv3 who

Bernhard Reutner-Fischer rep.dot.nop at gmail.com
Tue Feb 3 18:19:28 UTC 2009


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  <u173034 at informatik.uni-oldenburg.de>

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;
>}


More information about the busybox mailing list