| Summary: | The 'ps' command doesn't show real thread names | ||
|---|---|---|---|
| Product: | Busybox | Reporter: | Daniel Ng <daniel.ng1234> |
| Component: | Other | Assignee: | unassigned |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | busybox-cvs |
| Priority: | P5 | ||
| Version: | 1.18.x | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Host: | Target: | ||
| Build: | |||
| Attachments: |
Test program makes a thread and loops forever
proposed patch |
||
Created attachment 3379 [details]
proposed patch
patch attached- Change to procps.c: -The real thread name comes from ps->comm ->This needs to be populated by procps_scan(), and read by ps_main(). This was populated only when the PSSCAN_STAT flag was set. It is now populated when either PSSCAN_STAT or PSSCAN_COMM is set. Changes to ps.c: -The default behaviour of 'ps' is to print out the cmdline via func_args() -When the '-T' option is specified by the user, set the PSSCAN_COMM flag. Otherwise, do not use this flag. -Setting the PSSCAN_COMM flag causes ps_main() to read ps->comm for the real thread name rather than use cmdline I was told that Resolved-Fixed means this fix has already been committed. Changing to 'reopened'- waiting for someone to review this bug and if it's OK to go ahead and commit. - { MAX_WIDTH , "args" ,"COMMAND",func_args ,PSSCAN_COMM },
+ { MAX_WIDTH , "args" ,"COMMAND",func_args ,0 },
Why?
- if (flags & PSSCAN_STAT) {
+ if ( (flags & PSSCAN_STAT) || (flags & PSSCAN_COMM) ) {
Seems wrong. PSSCAN_STAT already contains COMM bit. See libbb.h
-#define DEFAULT_O_STR (SELINUX_O_PREFIX "pid,user" IF_FEATURE_PS_TIME(",time") ",args")
+#endif
+#if ENABLE_FEATURE_SHOW_THREADS
+#define THREADS_O_SUFFIX ",comm"
+#endif
+
+#define DEFAULT_O_STR_BASE ("pid,user" IF_FEATURE_PS_TIME(",time"))
+#define DEFAULT_O_STR_SUFFIX ",args"
+#if ENABLE_SELINUX
+#define MAX_DEFAULT_O_STR_SIZE (sizeof(DEFAULT_O_STR_BASE) + sizeof(DEFAULT_O_STR_SUFFIX) \
+ + sizeof(SELINUX_O_PREFIX)).
#else
-#define DEFAULT_O_STR ("pid,user" IF_FEATURE_PS_TIME(",time") ",args")
+#define MAX_DEFAULT_O_STR_SIZE (sizeof(DEFAULT_O_STR_BASE) + sizeof(DEFAULT_O_STR_SUFFIX))
I don't understand what you are trying to achieve here. Aha, conditionally add ",comm"? This will show comm for _every_ process, even the vast majority which has comm = argv0.
I have a different proposal: modify read_cmdline() so that it prepends {comm} only if it's different from argv0. This will also fix top.
Going to commit the fix to git.
Fixed in git by commit 5331e382f72a606c026424e95fcc7dc50a25608c http://git.busybox.net/busybox/commit/?id=5331e382f72a606c026424e95fcc7dc50a25608c Thanks for the commit of your fix, Denys. This doesn't result in the same behaviour as the traditional 'ps' and 'top', but I think it is a good compromise given that the alternative would require architectural code changes as discussed here: http://article.gmane.org/gmane.linux.busybox/34355 The main point is that the real thread names are now displayed, which is at least what I want. |
Created attachment 3373 [details] Test program makes a thread and loops forever 1) Run the attached test program which generates a thread that loops forever 2) Run 'busybox ps -T'. Note how the thread generated in the test program is just listed as './main' rather than its real thread name 'DansThread'. This would make it difficult to analyse an application with multiple threads because the real name of each individual thread would not be shown. ie. 'busybox ps -T' currently shows: ./main ./main ./main ./main The fixed version of 'busybox ps -T' would show: DansThread FredsThread FooThread BarThread