[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: SUSv3 touch+who
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2009-03-23 7:38:32
Message-ID: 200903230838.32077.vda.linux () googlemail ! com
[Download RAW message or body]

On Sunday 22 March 2009 19:08, walter harms wrote:
> Hi Denis,
> my SUSv3 conform versions of touch+who had some rounds on the bb mailing list.
> I think you can now move them into the svn.

The latest who.c I found in my mail is in this mail:

Message-ID: <49952AA5.6050001@bfs.de>
Date: Fri, 13 Feb 2009 09:09:09 +0100
From: walter harms <wharms@bfs.de>

Ok, I'm working on it...

...an hour later...

First thing I did is I removed lots of whitespace damage.
See attached 2.patch (13 kilobytes).

Then I build it, fixed one warning (you have to use CONFIG_WERROR
before submitting code), and I see this:

function                                             old     new   delta
who_main                                             292     771    +479
print_ut                                               -     441    +441
time2str                                               -      44     +44
_stat                                                  -      40     +40
ttyname                                                -      33     +33
------------------------------------------------------------------------------
(add/remove: 4/0 grow/shrink: 1/0 up/down: 1037/0)           Total: 1037 bytes
   text    data     bss     dec     hex filename
 821187     476    7616  829279   ca75f busybox_old
 822432     476    7648  830556   cac5c busybox_unstripped

Not very encouraging.

Okay, let's see how it performs.

Coreutils 6.9:

# /usr/bin/who -a
LOGIN      /dev/tty6    2009-03-23 06:11              1048 id=v/t
root     + tty2         2009-03-23 06:12 02:21         902

Busybox before patch:

# ./busybox who -a
USER       TTY      IDLE      TIME            HOST
LOGIN      /dev/tty6 ?         Mar 23 06:11:32
root       tty2     02:21     Mar 23 06:12:25

After patch:

# ./busybox who -a
(nothing)

This is not good.

Strace:

open("/var/run/utmp", O_RDWR)           = 3
fcntl(3, F_GETFD)                       = 0
fcntl(3, F_SETFD, FD_CLOEXEC)           = 0
lseek(3, 0, SEEK_SET)                   = 0
read(3, "\6\0\0\0\30\4\0\0/dev/tty6\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 384) = 384
read(3, "\7\0\0\0\206\3\0\0tty2\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 384) = 384
read(3, "", 384)                        = 0
_exit(0)   

Please find my /var/run/utmp attached.

Are you willing to debug this problem?
--
vda

["2.patch" (text/x-diff)]

diff -d -urpN busybox.1/coreutils/who.c busybox.2/coreutils/who.c
--- busybox.1/coreutils/who.c	2009-03-23 08:30:39.000000000 +0100
+++ busybox.2/coreutils/who.c	2009-03-23 08:29:57.000000000 +0100
@@ -1,77 +1,56 @@
-
 /* vi: set sw=4 ts=4: */
 /*
  * SUSv3 who implementation for busybox
  *
- * Copyright (C) 2008 by Walter Harms 
+ * Copyright (C) 2008 by Walter Harms
  * http://www.opengroup.org/onlinepubs/9699919799/utilities/who.html
  *
- * BB_AUDIT SUSv3 defects - 
+ * BB_AUDIT SUSv3 defects -
  * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
  * $Id: who.c,v 1.16 2009/02/03 22:16:39 walter Exp walter $
  */
 
-#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 'mom loves'  
+  -a enables:  -b, -d, -l, -p, -r, -t, -T and -u
+  no support for 'who am i' or 'mom loves'
 */
+/* option -w:  like q with full info, GNU extension  */
 
-// "%b %e %H:%M" 
-//
-// char bb_common_bufsiz1[COMMON_BUFSIZE]
-// move this to libbb ??
-
-
-#if ENABLE_FEATURE_GLOBAL_BUFFER
+#include "libbb.h"
+#include <utmp.h>
 
-static char *time2str(const char *fmt, time_t t)
-{
-	strftime(bb_common_bufsiz1, COMMON_BUFSIZE, fmt, localtime(&t));
-	return bb_common_bufsiz1;
-}
+#if 0
+# define TIMEFMT       "%b %e %H:%M"
 #else
-static char *time2str(const char *fmt, time_t t)
-{
-	static char buf[20];
-
-	strftime(buf, sizeof(buf), fmt, localtime(&t));
-	return buf;
-}
-
+# define TIMEFMT       "%Y-%m-%d %T"
 #endif
 
+enum {
+	OPT_a = (1 << 0),  /* -b, -d, -l, -p, -r, -t, -T and -u */
+	OPT_b = (1 << 1),  /* last reboot */
+	OPT_d = (1 << 2),  /* dead process */
+	OPT_H = (1 << 3),  /* add Header */
+	OPT_l = (1 << 4),  /* login process */
+	OPT_m = (1 << 5),  /* current terminal only */
+	OPT_p = (1 << 6),  /* processes spawn by init */
+	OPT_q = (1 << 7),  /* quick */
+	OPT_r = (1 << 8),  /* current runlevel */
+	OPT_s = (1 << 9),  /* default */
+	OPT_t = (1 << 10), /* last time change */
+	OPT_T = (1 << 11), /* add time of login */
+	OPT_u = (1 << 12), /* idle */
+	OPT_D = (1 << 13), /* debug DUMPFILE */
+};
 
-#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 */
-
+#define time_str_buf bb_common_bufsiz1
 
-/* option -w:  like q with full info, GNU extension  */
+static char *time2str(const char *fmt, time_t t)
+{
+	strftime(time_str_buf, sizeof(time_str_buf), fmt, localtime(&t));
+	return time_str_buf;
+}
 
 static int _stat(char *line, struct stat *st)
 {
@@ -87,73 +66,66 @@ static char term_state(char *line)
 {
 	struct stat st;
 
-	if (_stat(line, &st) < 0) {
+	if (_stat(line, &st) < 0)
 		return '?';
-	} else {
-		if (st.st_mode & S_IWOTH)
+	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('?');
+		bb_putchar('?');
 		goto print;
 	}
 
-
 	/* one of the few occation we use atime */
 	t = time(NULL) - st.st_atime;
 
 	if (t < 60) {
-		putchar('.');
+		bb_putchar('.');
 		goto print;
 	}
 
 	/* always  t >= 0 && */
-	if (t < (24 * 60 * 60)) {
+	if (t < (24 * 60 * 60))
 		printf("%02d:%02d",
-			   (int) (t / (60 * 60)), (int) ((t % (60 * 60)) / 60));
-
-	} else
+			(int) (t / (60 * 60)),
+			(int) ((t % (60 * 60)) / 60));
+	else
 		printf("old");
 
-  print:
-	putchar('\t');
-
+ print:
+	bb_putchar('\t');
 }
 
 enum {
-	NAME = 1 << 0,
-	STATE = 1 << 1,
-	LINE = 1 << 2,
-	TIME = 1 << 3,
+	NAME     = 1 << 0,
+	STATE    = 1 << 1,
+	LINE     = 1 << 2,
+	TIME     = 1 << 3,
 	ACTIVITY = 1 << 4,
-	PID = 1 << 5,
-	COMMENT = 1 << 6,
-	EXIT = 1 << 7
+	PID      = 1 << 5,
+	COMMENT  = 1 << 6,
+	EXIT     = 1 << 7
 };
 
-
 static void print_header(int pattern)
 {
-
-#define	str             "NAME\0"\
-		        "STATE\0"\
-		        "LINE\0"\
-		        "TIME\0"\
-		        "ACTIVITY\0"\
-		        "PID\0"\
-		        "COMMENT\0"\
-		        "EXIT"
-
 	int i = 0;
+#define	str \
+	"NAME\0"\
+	"STATE\0"\
+	"LINE\0"\
+	"TIME\0"\
+	"ACTIVITY\0"\
+	"PID\0"\
+	"COMMENT\0"\
+	"EXIT"
 
 	if (pattern < 0)
 		return;
@@ -161,94 +133,70 @@ static void print_header(int pattern)
 	while (pattern) {
 		if (pattern & 1)
 			printf("%s\t", nth_string(str, i));
-
 		pattern >>= 1;
 		i++;
 	}
-	putchar('\n');
+	bb_putchar('\n');
 #undef str
 }
 
 
 /*
- print content of struct utmp, select what with mask 
+ 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
-*/
-
+#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 */
-
-	putchar('\n');
+	bb_putchar('\n');
 }
 
 int who_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
-
-	int who_main(int argc UNUSED_PARAM, char **argv)
+int who_main(int argc UNUSED_PARAM, char **argv)
 {
 	struct utmp *ut;
-	char *name;
+	char *name = name; /* for compiler */
 	unsigned int ucnt = 0;
-	//	int pattern = -1;
-	int pattern= NAME| LINE | TIME ;
+	int pattern = NAME | LINE | TIME;
 	unsigned int opt;
 
 	/*
 	   FIXME: make better use of opt_complementary
 	 */
 	//  opt_complementary = "q-abdHlmqprstuD";
-
 	opt = getopt32(argv, "abdHlmpqrstTuD");
 	argv += optind;
-
-	argc-=optind;
-/*
-  utmpname() will not fail if the file does not exists
-*/
-
-
 	if (opt == 0)
 		opt = OPT_s;
 	if (opt == OPT_u)
@@ -257,190 +205,80 @@ int who_main(int argc, char **argv) MAIN
 	if (opt & OPT_a)
 		opt |= (OPT_b | OPT_d | OPT_l | OPT_p | OPT_r | OPT_T | OPT_u);
 
-
-
-	switch (argc) {
-
-	case 1:
-	     utmpname(*argv);
-	     break;
-
-	case 2:
-	      if ( (strcmp(*argv,"mom") == 0 && strcmp(*(argv+1),"loves") == 0) ||
-	           (strcmp(*argv,"am")  == 0 && strcmp(*(argv+1),"i") == 0) )
-		opt|=OPT_m;	   
-		/* fall tru */
-	case 0:
-	  break;	  
-
-	default:
-	  fprintf(stderr,"to many arguments\n");
-	  exit(1);
+	/* utmpname() will not fail if the file does not exists */
+	if (argv[1]) {
+		if (argv[2])
+			/* "who am i", "who mom loves" == "who -m: */
+			opt |= OPT_m;
+		else
+			utmpname(argv[1]);
 	}
+
 	/*
 	   figure out pattern to figure out pattern for heading
 	 */
-
 	if (opt & OPT_T)
-		pattern |= (opt & OPT_u) ?  STATE | COMMENT | ACTIVITY :  STATE | COMMENT;
-
+		pattern |= (opt & OPT_u) ? STATE | COMMENT | ACTIVITY : STATE | COMMENT;
 	else if (opt & OPT_s) {
-	     if (opt & OPT_u)  
-	       pattern |= ACTIVITY ;
+		if (opt & OPT_u)
+			pattern |= ACTIVITY;
 	}
-
 	else if (opt & OPT_l)
-		pattern |= (opt & OPT_u) ?  PID | COMMENT | ACTIVITY :   PID | COMMENT;
-
+		pattern |= (opt & OPT_u) ? PID | COMMENT | ACTIVITY : PID | COMMENT;
 	else if (opt & OPT_p)
-		pattern |= (opt & OPT_u) ?  PID | COMMENT | EXIT | ACTIVITY :  PID | COMMENT | EXIT;
+		pattern |= (opt & OPT_u) ? PID | COMMENT | EXIT | ACTIVITY : PID | COMMENT | EXIT;
 	else if (opt & OPT_D)
-		pattern |=   PID | COMMENT | EXIT;
+		pattern |= PID | COMMENT | EXIT;
 
 	/*
 	 * if anyone has a console outside /dev we will ignore it
 	 */
 	if (opt & OPT_m) {
 		name = ttyname(STDIN_FILENO);
-		if (!name || strncmp(name, "/dev/", 5))
+		if (!name || strncmp(name, "/dev/", 5) != 0)
 			bb_perror_msg_and_die("ttyname()");
 		name += 5;
 		/*
-		   else 
+		   else
 		   something strange is going on
 		   can this ever happen ?
 		 */
-
 	}
 
 	setutent();
 
 	if (opt & OPT_H)
 		print_header(pattern);
-#if 0
 
 	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);
-		}
-
-		/* -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");
-
-			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 */
-#else
-
-	while ((ut = getutent()) != NULL) {
-
-		/* -m : only current terminal */
-		if (opt & OPT_m && strcmp(name, ut->ut_line))
+		if (opt & OPT_m && strcmp(name, ut->ut_line) != 0)
 			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;
 		}
 
-
-		if (
-		    ((opt & OPT_d) && DEAD_PROCESS == ut->ut_type)  ||		/* -d */
-		    ((opt & OPT_l) && LOGIN_PROCESS == ut->ut_type) ||           /* -l */
-	            ((opt & OPT_p) && INIT_PROCESS == ut->ut_type)  ||           /* -p */
-		    ((opt & OPT_s) && USER_PROCESS == ut->ut_type)  ||          /* -q */
-                    ((opt & OPT_T) && USER_PROCESS == ut->ut_type)  ||           /* -T */
-		    (opt & OPT_D) ) {
+		if ((opt & OPT_D)
+		 || ((opt & OPT_d) && DEAD_PROCESS == ut->ut_type)  /* -d */
+		 || ((opt & OPT_l) && LOGIN_PROCESS == ut->ut_type) /* -l */
+		 || ((opt & OPT_p) && INIT_PROCESS == ut->ut_type)  /* -p */
+		 || ((opt & OPT_s) && USER_PROCESS == ut->ut_type)  /* -q */
+		 || ((opt & OPT_T) && USER_PROCESS == ut->ut_type)  /* -T */
+		) {
 			print_ut(ut, pattern);
 		}
 
-		
 		/* -q */
 		if ((opt & OPT_q) && USER_PROCESS == ut->ut_type) {
 			printf("%s ", ut->ut_user);
@@ -453,9 +291,10 @@ int who_main(int argc, char **argv) MAIN
 				printf("Current\tTime\t\tLast\n");
 
 			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));
+				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;
@@ -463,7 +302,6 @@ int who_main(int argc, char **argv) MAIN
 				break;
 		}
 
-
 		/* -t */
 		if ((opt & OPT_t) && NEW_TIME == ut->ut_type) {
 			if (opt & OPT_H)
@@ -471,27 +309,14 @@ int who_main(int argc, char **argv) MAIN
 			print_ut(ut, TIME);
 			break;
 		}
+	} /* while */
 
-
-	}					/* while */
-
-
-
-#endif
-
-
-
-
-/*
-  write user count
-*/
-
+	/* write user count */
 	if (opt & OPT_q)
 		printf("\n#User %d\n", ucnt);
 
 	if (ENABLE_FEATURE_CLEAN_UP)
 		endutent();
 
-
 	return EXIT_SUCCESS;
 }

["utmp" (application/octet-stream)]

_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic