[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [BusyBox] [patch] ps w, take two
From: Bernhard Fischer <rep.nop () aon ! at>
Date: 2005-05-31 18:25:00
Message-ID: 20050531182500.GA24584 () aon ! at
[Download RAW message or body]
On Tue, May 31, 2005 at 02:57:07AM -0400, Rob Landley wrote:
>On Tuesday 08 February 2005 09:49 am, Bernhard Fischer wrote:
>> On Fri, Feb 04, 2005 at 01:17:41PM +0300, Vladimir N. Oleynik wrote:
>> >Rainer,
>> >
>> >>>The attached patch adds an option CONFIG_PS_FEATURE_WIDE which adds
>
>> What do you think?
>
>I prefer to minimize #ifdefs. The new #ifdef in libbb.h seems gratuitous (how
>much space does not returning 0 and ignoring it actually save)? (The changes
>to usage.h are also kind of ugly, but that's usage.h for you.)
I guess that it saves not much space. I removed those defines in libbb.h
so it's void as before. An alternative approach to deal with output not
going to a tty is outlined below.
>
>You shouldn't need to #define w_opt as 0 if data flow analysis says it can
>never be set to nonzero anywhere. It's a local variable, it's all within one
>function, any reasonably modern compiler should be able to figure this out
>when optimizing.
>
>
>Snippet from the patch:
>- i = 0;
>- if(strlen(namecmd) > i)
>+ i = 0;
>+ if((w_opt < 2) && (strlen(namecmd) > i))
> namecmd[i] = 0;
>
>Why not just set terminal_width=INT_MAX in the 2 case and drop the games with
I dropped the whole w_opt thing.
>w_opt? (Is it that you explicitly want -w -w to act like the #ifdef
>AUTOWIDTH was enabled and the call failed? In that case, would a single -w
That was my intention, yes. Use case was to pipe the output of ps to
another app. In this case, GNU (procps) ps does set wide output
unconditionally. Please see the attached optional snippet to mimic this.
>also directly setting it to INT_MAX be a bad thing? Is a width of 132
>special?)
It is. As vodz pointed out earlier in this thread, one w is supposed to
set 132, more than one w sets it to unlimited.
>
>What is the size impact of enabling both of these features?
With the attached patch (which does _not_ handle automatic wide output
when output goes to not a tty) sizes with FEATURE_AUTOWIDTH set in all
cases are:
text data bss dec hex filename
311 0 0 311 137 procps/ps.o.oorig
291 0 0 291 123 procps/ps.o.without-w
343 0 0 343 157 procps/ps.o.with-w
Please also note that in the attached, i cleaned up the option handling,
so both "w" and selinux' "c" are handled if one of those happen to be
enabled by the user. Also note that the whole wide support is disabled
per default.
Thank you,
Bernhard
["ps-wide.3.diff" (text/plain)]
diff -rdup upstream/busybox/include/usage.h busybox.tst/include/usage.h
--- upstream/busybox/include/usage.h 2005-05-31 10:18:49.000000000 +0200
+++ busybox.tst/include/usage.h 2005-05-31 14:22:01.817907440 +0200
@@ -2036,18 +2036,30 @@
"$ printf \"Val=%d\\n\" 5\n" \
"Val=5\n"
+#if !defined(CONFIG_SELINUX) && !defined(CONFIG_PS_FEATURE_WIDE)
+#define USAGE_PS "\n\tThis version of ps accepts no options."
+#else
+#define USAGE_PS "\nOptions:"
+#endif
#ifdef CONFIG_SELINUX
#define USAGE_NONSELINUX(a)
#else
#define USAGE_NONSELINUX(a) a
#endif
+#ifdef CONFIG_PS_FEATURE_WIDE
+#define USAGE_PS_WIDE(a) a
+#else
+#define USAGE_PS_WIDE(a)
+#endif
#define ps_trivial_usage \
""
#define ps_full_usage \
"Report process status\n" \
- USAGE_NONSELINUX("\n\tThis version of ps accepts no options.") \
- USAGE_SELINUX("\nOptions:\n\t-c\tshow SE Linux context")
+ USAGE_PS \
+ USAGE_SELINUX("\n\t-c\tshow SE Linux context") \
+ USAGE_PS_WIDE("\n\tw\twide output")
+
#define ps_example_usage \
"$ ps\n" \
diff -rdup upstream/busybox/procps/Config.in busybox.tst/procps/Config.in
--- upstream/busybox/procps/Config.in 2005-02-09 19:08:13.000000000 +0100
+++ busybox.tst/procps/Config.in 2005-05-31 11:51:53.000000000 +0200
@@ -43,6 +43,15 @@ config CONFIG_PS
help
ps gives a snapshot of the current processes.
+config CONFIG_FEATURE_PS_WIDE
+ bool " Enable argument for wide output (-w)"
+ default n
+ depends on CONFIG_PS
+ help
+ Support argument 'w' for wide output.
+ If given once, 132 chars are printed and given more than
+ one, the length is unlimited.
+
config CONFIG_RENICE
bool "renice"
default n
diff -rdup upstream/busybox/procps/ps.c busybox.tst/procps/ps.c
--- upstream/busybox/procps/ps.c 2005-05-03 10:14:02.000000000 +0200
+++ busybox.tst/procps/ps.c 2005-05-31 19:40:07.115501192 +0200
@@ -34,24 +34,42 @@
#include <selinux/selinux.h> /* for is_selinux_enabled() */
#endif
-static const int TERMINAL_WIDTH = 79; /* not 80 in case terminal has linefold bug */
-
-
+#define TERMINAL_WIDTH 80
extern int ps_main(int argc, char **argv)
{
procps_status_t * p;
- int i, len;
- int terminal_width = TERMINAL_WIDTH;
+ int i, len, terminal_width;
#ifdef CONFIG_SELINUX
int use_selinux = 0;
security_context_t sid=NULL;
- if(is_selinux_enabled() && argv[1] && !strcmp(argv[1], "-c") )
- use_selinux = 1;
#endif
-
get_terminal_width_height(0, &terminal_width, NULL);
+
+#if defined(CONFIG_FEATURE_PS_WIDE) || defined(CONFIG_SELINUX)
+ /* handle arguments */
+ /* bb_getopt_ulflags(argc, argv,) would force a leading dash */
+ for (len = 1; len < argc; len++) {
+ char *c = argv[len];
+ while (*c) {
+#ifdef CONFIG_FEATURE_PS_WIDE
+ if (*c == 'w')
+ /* if w is given once, GNU ps sets the width to 132,
+ * if w is given more than once, it is "unlmiited"
+ */
+ terminal_width =
+ (terminal_width==TERMINAL_WIDTH) ? 132 : INT_MAX;
+#endif
+#ifdef CONFIG_SELINUX
+ if (*c == 'c' && is_selinux_enabled())
+ use_selinux = 1;
+#endif
+ c++;
+ }
+ }
+#endif
+
/* Go one less... */
terminal_width--;
@@ -85,18 +103,19 @@ extern int ps_main(int argc, char **argv
safe_strncpy(sbuf, "unknown",7);
}
len = printf("%5d %-32s %s ", p->pid, sbuf, p->state);
- }
+ }
else
#endif
if(p->rss == 0)
len = printf("%5d %-8s %s ", p->pid, p->user, p->state);
else
len = printf("%5d %-8s %6ld %s ", p->pid, p->user, p->rss, p->state);
+
i = terminal_width-len;
- if(namecmd != 0 && namecmd[0] != 0) {
+ if(namecmd && namecmd[0]) {
if(i < 0)
- i = 0;
+ i = 0;
if(strlen(namecmd) > i)
namecmd[i] = 0;
printf("%s\n", namecmd);
@@ -108,7 +127,10 @@ extern int ps_main(int argc, char **argv
namecmd[i-2] = 0;
printf("[%s]\n", namecmd);
}
- free(p->cmd);
+#ifdef CONFIG_FEATURE_CLEAN_UP
+/* if (p->cmd) *//* no check needed, but to make valgrind happy.. */
+ free(p->cmd);
+#endif
}
return EXIT_SUCCESS;
}
["ps-wide.3.pipe-wide.diff" (text/plain)]
\\ stat is costy (that's why i initially proposed an extra config-option
\\ for CONFIG_PS_FEATURE_WIDE_PIPE).
\\ The snipped below would add about 34 bytes..
--- procps/ps.c.3 2005-05-31 19:40:07.115501000 +0200
+++ procps/ps.c 2005-05-31 20:04:35.192319592 +0200
@@ -45,7 +45,13 @@ extern int ps_main(int argc, char **argv
int use_selinux = 0;
security_context_t sid=NULL;
#endif
- get_terminal_width_height(0, &terminal_width, NULL);
+ struct stat stat_buf;
+ /* if stdout is a pipe then use wide output, else query stdin's tty props */
+ fstat(1, &stat_buf);
+ if (S_ISFIFO(stat_buf.st_mode))
+ terminal_width = INT_MAX;
+ else
+ get_terminal_width_height(0, &terminal_width, NULL);
#if defined(CONFIG_FEATURE_PS_WIDE) || defined(CONFIG_SELINUX)
/* handle arguments */
_______________________________________________
busybox mailing list
busybox@mail.busybox.net
http://codepoet.org/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic