top(1): support command name and argument grepping #479
Conversation
|
This passes my initial traige, and also looks cool :) |
|
@allanjude mentioned this feature on BSD Now and I thought it sounded like a great challenge to tackle ;) |
usr.bin/top/top.c
Outdated
| new_message(MT_standout, | ||
| "Grep command name: "); | ||
| if (readline(tempbuf1, sizeof(tempbuf1), false) > 0) | ||
| { |
There was a problem hiding this comment.
This { needs to be at the end of the prior line.
Also, if I do /getty I see just the getty routines. However, if I do / next, it doesn't reset the display to all processes because readline is return -1 in this case... This seems wrong because the cmd_matches code seems to want to handle this case. I think we should set ps.command back to NULL after freeing it..
There was a problem hiding this comment.
And there's a resource leak, both of these can be fixed by free'ing ps.command and setting it to NULL before the readline...
There was a problem hiding this comment.
Resetting the display to all processes is handled by /+ That's the behavior in OpenBSD. I like it because it allows you to back out of a search.
The missing free is fixed. A const qualifier was added to the process_select struct in fc36f5a though I'm unsure why.
There was a problem hiding this comment.
The const can be removed, I think.
|
So this needs an update to proceed. |
|
Also, the cmd_match appears to be lifted 100% from OpenBSD w/o credit in the commit message. |
usr.bin/top/top.c
Outdated
| ps.command = NULL; | ||
| else if ((ps.command = strdup(tempbuf1)) == NULL) | ||
| quit(1); | ||
| clear_message(); |
There was a problem hiding this comment.
And this clear_message() needs to be outside the if, because we need to do it either way.
|
I'm still thinking about the const char *command -> char *command change... Otherwise I think this may be good. The code snagged from OpenBSD doesn't exactly match our current preferred style, but I think it's OK as it's in the style top was written in... |
usr.bin/top/top.c
Outdated
| "Grep command name: "); | ||
| if (readline(tempbuf1, sizeof(tempbuf1), false) > 0) { | ||
| if (tempbuf1[0] == '+' && tempbuf1[1] == '\0') { | ||
| free(ps.command); |
There was a problem hiding this comment.
This free needs to be moved up to before the if since we need to do this in both cases.
Otherwise we leak the previous ps.command
There was a problem hiding this comment.
Ah, yes, you're right. It needs to be freed regardless if display is reset or a new value is entered. Thanks for the quick review, Warner!
Obtained from: OpenBSD
|
This looks good at first blush. I'll take a look at it (in that I'll import it into a trial branch and compile it, test it, etc) |
|
Landed as a00d703 |
Obtained from: OpenBSD Reviewed by: imp@ Pull Request: #479
Obtained from: OpenBSD Reviewed by: imp@ Pull Request: freebsd/freebsd-src#479 (cherry picked from commit a00d703)
Obtained from: OpenBSD Reviewed by: imp@ Pull Request: freebsd/freebsd-src#479
This feature is similar to the grep functionality in OpenBSD's top command.