Re: Allow top(1) to search arguments (again)

2016-05-28 Thread Edd Barrett
On Wed, May 11, 2016 at 02:28:51PM +0200, Michal Mazurek wrote:
> As discussed off list, "if (!term)" is redundant, as the caller does the
> check.
> 
> Also fix whitespace in some unrelated places.

Can you split out the "term" change from style changes?

You can do a KNF whack in a separate commit (later).

-- 
Best Regards
Edd Barrett

http://www.theunixzoo.co.uk



Re: Allow top(1) to search arguments (again)

2016-05-11 Thread Ted Unangst
this improves the realloc loop. there is no need to constantly call realloc to
resize the memory. if we have enough, we have enough. also no need to penny
pinch the initial allocation.

calling sysctl all the time is still wasteful, but harder to fix.

Index: machine.c
===
RCS file: /cvs/src/usr.bin/top/machine.c,v
retrieving revision 1.86
diff -u -p -r1.86 machine.c
--- machine.c   11 May 2016 08:11:27 -  1.86
+++ machine.c   11 May 2016 17:42:42 -
@@ -366,20 +366,25 @@ static char **
 get_proc_args(struct kinfo_proc *kp)
 {
static char **s;
-   size_t  siz = 100;
+   static size_t   siz = 1023;
int mib[4];
 
-   for (;; siz *= 2) {
-   if ((s = realloc(s, siz)) == NULL)
-   err(1, NULL);
-   mib[0] = CTL_KERN;
-   mib[1] = KERN_PROC_ARGS;
-   mib[2] = kp->p_pid;
-   mib[3] = KERN_PROC_ARGV;
-   if (sysctl(mib, 4, s, &siz, NULL, 0) == 0)
+   if (!s && !(s = malloc(siz)))
+   err(1, NULL);
+
+   mib[0] = CTL_KERN;
+   mib[1] = KERN_PROC_ARGS;
+   mib[2] = kp->p_pid;
+   mib[3] = KERN_PROC_ARGV;
+   for (;;) {
+   size_t space = siz;
+   if (sysctl(mib, 4, s, &space, NULL, 0) == 0)
break;
if (errno != ENOMEM)
return NULL;
+   siz *= 2;
+   if ((s = realloc(s, siz)) == NULL)
+   err(1, NULL);
}
return s;
 }



Re: Allow top(1) to search arguments (again)

2016-05-11 Thread Michal Mazurek
As discussed off list, "if (!term)" is redundant, as the caller does the
check.

Also fix whitespace in some unrelated places.

Index: usr.bin/top/display.c
===
RCS file: /cvs/src/usr.bin/top/display.c,v
retrieving revision 1.50
diff -u -p -r1.50 display.c
--- usr.bin/top/display.c   26 Oct 2015 12:44:22 -  1.50
+++ usr.bin/top/display.c   11 May 2016 12:23:23 -
@@ -516,7 +516,7 @@ void
 i_header(char *text)
 {
if (header_status == Yes && (screen_length > y_header
-  || !smart_terminal)) {
+   || !smart_terminal)) {
if (!smart_terminal) {
putn();
if (fputs(text, stdout) == EOF)
Index: usr.bin/top/machine.c
===
RCS file: /cvs/src/usr.bin/top/machine.c,v
retrieving revision 1.86
diff -u -p -r1.86 machine.c
--- usr.bin/top/machine.c   11 May 2016 08:11:27 -  1.86
+++ usr.bin/top/machine.c   11 May 2016 12:23:23 -
@@ -390,29 +390,25 @@ cmd_matches(struct kinfo_proc *proc, cha
extern int  show_args;
char**args = NULL;
 
-   if (!term) {
-   /* No command filter set */
+   /* Filter set, process name needs to contain term */
+   if (strstr(proc->p_comm, term))
return 1;
-   } else {
-   /* Filter set, process name needs to contain term */
-   if (strstr(proc->p_comm, term))
-   return 1;
-   /* If showing arguments, search those as well */
-   if (show_args) {
-   args = get_proc_args(proc);
+   /* If showing arguments, search those as well */
+   if (show_args) {
+   args = get_proc_args(proc);
 
-   if (args == NULL) {
-   /* Failed to get args, so can't search them */
-   return 0;
-   }
+   if (args == NULL) {
+   /* Failed to get args, so can't search them */
+   return 0;
+   }
 
-   while (*args != NULL) {
-   if (strstr(*args, term))
-   return 1;
-   args++;
-   }
+   while (*args != NULL) {
+   if (strstr(*args, term))
+   return 1;
+   args++;
}
}
+
return 0;
 }
 
Index: usr.bin/top/screen.c
===
RCS file: /cvs/src/usr.bin/top/screen.c,v
retrieving revision 1.20
diff -u -p -r1.20 screen.c
--- usr.bin/top/screen.c5 Feb 2010 10:21:10 -   1.20
+++ usr.bin/top/screen.c11 May 2016 12:23:23 -
@@ -116,11 +116,11 @@ init_termcap(int interactive)
else
screen_width -= 1;
 
-/* get necessary capabilities */
-if (tgetstr("cl", NULL) == NULL || tgetstr("cm", NULL) == NULL) {
-smart_terminal = No;
-return;
-}
+   /* get necessary capabilities */
+   if (tgetstr("cl", NULL) == NULL || tgetstr("cm", NULL) == NULL) {
+   smart_terminal = No;
+   return;
+   }
 
/* get the actual screen size with an ioctl, if needed */
/*
Index: usr.bin/top/top.h
===
RCS file: /cvs/src/usr.bin/top/top.h,v
retrieving revision 1.15
diff -u -p -r1.15 top.h
--- usr.bin/top/top.h   21 Sep 2013 14:15:19 -  1.15
+++ usr.bin/top/top.h   11 May 2016 12:23:23 -
@@ -61,8 +61,8 @@
 
 
 struct errs {  /* structure for a system-call error */
-int err;   /* value of errno (that is, the actual error) */
-char *arg; /* argument that caused the error */
+   int err;/* value of errno (that is, the actual error) */
+   char *arg;  /* argument that caused the error */
 };
 
 extern struct errs errs[];

-- 
Michal Mazurek



Re: Allow top(1) to search arguments (again)

2016-05-10 Thread Ted Unangst
Edd Barrett wrote:
> On Thu, Apr 28, 2016 at 03:26:48PM +0100, Edd Barrett wrote:
> > Resubmitting this diff, as I've been unable to get an OK.
> 
> Style tweaks fixed, as pointed out by Michal Mazurek. Thanks for this.
> 
> OK?

ok



Re: Allow top(1) to search arguments (again)

2016-05-10 Thread Edd Barrett
On Thu, Apr 28, 2016 at 03:26:48PM +0100, Edd Barrett wrote:
> Resubmitting this diff, as I've been unable to get an OK.

Style tweaks fixed, as pointed out by Michal Mazurek. Thanks for this.

OK?

Index: machine.c
===
RCS file: /home/edd/cvsync/src/usr.bin/top/machine.c,v
retrieving revision 1.85
diff -u -p -r1.85 machine.c
--- machine.c   20 Aug 2015 22:32:42 -  1.85
+++ machine.c   4 May 2016 22:09:32 -
@@ -57,6 +57,8 @@
 static int swapmode(int *, int *);
 static char*state_abbr(struct kinfo_proc *);
 static char*format_comm(struct kinfo_proc *);
+static int cmd_matches(struct kinfo_proc *, char *);
+static char**get_proc_args(struct kinfo_proc *);
 
 /* get_process_info passes back a handle.  This is what it looks like: */
 
@@ -360,6 +362,60 @@ getprocs(int op, int arg, int *cnt)
return (procbase);
 }
 
+static char **
+get_proc_args(struct kinfo_proc *kp)
+{
+   static char **s;
+   size_t  siz = 100;
+   int mib[4];
+
+   for (;; siz *= 2) {
+   if ((s = realloc(s, siz)) == NULL)
+   err(1, NULL);
+   mib[0] = CTL_KERN;
+   mib[1] = KERN_PROC_ARGS;
+   mib[2] = kp->p_pid;
+   mib[3] = KERN_PROC_ARGV;
+   if (sysctl(mib, 4, s, &siz, NULL, 0) == 0)
+   break;
+   if (errno != ENOMEM)
+   return NULL;
+   }
+   return s;
+}
+
+static int
+cmd_matches(struct kinfo_proc *proc, char *term)
+{
+   extern int  show_args;
+   char**args = NULL;
+
+   if (!term) {
+   /* No command filter set */
+   return 1;
+   } else {
+   /* Filter set, process name needs to contain term */
+   if (strstr(proc->p_comm, term))
+   return 1;
+   /* If showing arguments, search those as well */
+   if (show_args) {
+   args = get_proc_args(proc);
+
+   if (args == NULL) {
+   /* Failed to get args, so can't search them */
+   return 0;
+   }
+
+   while (*args != NULL) {
+   if (strstr(*args, term))
+   return 1;
+   args++;
+   }
+   }
+   }
+   return 0;
+}
+
 caddr_t
 get_process_info(struct system_info *si, struct process_select *sel,
 int (*compare) (const void *, const void *))
@@ -421,8 +477,7 @@ get_process_info(struct system_info *si,
(!hide_uid || pp->p_ruid != sel->huid) &&
(!show_uid || pp->p_ruid == sel->uid) &&
(!show_pid || pp->p_pid == sel->pid) &&
-   (!show_cmd || strstr(pp->p_comm,
-   sel->command))) {
+   (!show_cmd || cmd_matches(pp, sel->command))) {
*prefp++ = pp;
active_procs++;
}
@@ -462,27 +517,17 @@ state_abbr(struct kinfo_proc *pp)
 static char *
 format_comm(struct kinfo_proc *kp)
 {
-   static char **s, buf[MAX_COLS];
-   size_t siz = 100;
-   char **p;
-   int mib[4];
-   extern int show_args;
+   static char buf[MAX_COLS];
+   char**p, **s;
+   extern int  show_args;
 
if (!show_args)
return (kp->p_comm);
 
-   for (;; siz *= 2) {
-   if ((s = realloc(s, siz)) == NULL)
-   err(1, NULL);
-   mib[0] = CTL_KERN;
-   mib[1] = KERN_PROC_ARGS;
-   mib[2] = kp->p_pid;
-   mib[3] = KERN_PROC_ARGV;
-   if (sysctl(mib, 4, s, &siz, NULL, 0) == 0)
-   break;
-   if (errno != ENOMEM)
-   return (kp->p_comm);
-   }
+   s = get_proc_args(kp);
+   if (s == NULL)
+   return kp->p_comm;
+
buf[0] = '\0';
for (p = s; *p != NULL; p++) {
if (p != s)
Index: top.1
===
RCS file: /home/edd/cvsync/src/usr.bin/top/top.1,v
retrieving revision 1.66
diff -u -p -r1.66 top.1
--- top.1   6 May 2015 07:53:29 -   1.66
+++ top.1   4 May 2016 22:15:10 -
@@ -108,6 +108,7 @@ The default is 1 for dumb terminals.
 Display only processes that contain
 .Ar string
 in their command name.
+If displaying of arguments is enabled, the arguments are searched too.
 .It Fl H
 Show process threads in the display.
 Normally, only the main process is shown.
@@ -306,6 +307,7 @@ command.
 Display only processes that contain
 .Ar string
 in their command name.
+If disp