Changed the return value of getCommandLine() method of linuxOSProcess and macOSProcess from null-delimited string to space-delimited string #1729
Conversation
The return value of getCommandLine() method of macOSProcess and linuxOSProcess have been changed from a null-delimited string to a space-delimited string.
# Conflicts: # UPGRADING.md
| @@ -133,7 +135,7 @@ public String getPath() { | |||
|
|
|||
| @Override | |||
| public String getCommandLine() { | |||
| return this.commandLine.get(); | |||
| return Arrays.stream(commandLine.get().split("\0")).collect(Collectors.joining(" ")); | |||
There was a problem hiding this comment.
While this approach was appropriate for Linux, you can look at the queryCommandLine() method and see it simply joins getArguments() using the null. So It's better to just join with a space in that method, and then you could return it directly here without further processing.
There was a problem hiding this comment.
All right, I've fixed it and understood how it improves performance. Could you maybe explain in brief why linux and mac do this differently? Like what section makes it platform-dependent and if it improves performance in both the cases, just for learning's sake.
There was a problem hiding this comment.
Linux provides the command line arguments via a null-delimited string in the proc filesystem, specifically /proc/[pid]/cmdline. (See man proc.)
macOS provides the command line arguments via sysctl call (KERN_PROCARGS2) in a null-delimited buffer. We can iterate that buffer to fetch the strings.
Before we (recently) added methods to fetch the process arguments separately, and because of the native null-delimited strings, these two OS's provided the ability to parse the arguments without having to do special treatment of spaces in names, etc. Rather than "lose information" by replacing the nulls with spaces, we provided users the full null-delimited string to parse to get the argument list themselves.
For the rest of the Unix-based implementations we just fetched the space-delimited argument string from the ps output (which is truncated to 80 characters on Solaris), and we got a space-delimited string from Windows via WMI (with additional parsing complexities using quotation marks). Parsing space-delimited command lines is not trivial, and rather than try to parse all these to a list we just returned them as-is to the user and let them do the parsing.
With the addition of a separate list of command line arguments, user-level parsing of getCommandLine() is no longer necessary, so the goal of this fix was just to simplify the getCommandLine output to the same common "user readable" space-delimited output on all OS's. Users who wanted to simply display a user-readable string had to do special cases for macOS and Linux to process the nulls. They will no longer have to do so.
There is still some bit of platform dependence in the output. While all the other Unix implementations now combine the argument list, that approach fails if you're not running with elevated permissions and querying a process you don't own. For those cases, the ps-based parsing does still work (since ps is owned by root) and is provided as a "backup" option (so for example on Solaris not running elevated, you'll get a truncated 80-character output for some processes and full output on others that you own). Bottom line: the method should now be treated only as a cosmetic "best effort output" and anyone looking to actually parse the output should use getArguments().
There was a problem hiding this comment.
Ohhh, I understand now. Thanks for the explanation and all the help. Learnt a lot from this.
oshi-core/src/main/java/oshi/software/os/linux/LinuxOSProcess.java
Outdated
Show resolved
Hide resolved
This is a fix to the third issue mentioned in #1723. The OSProcess javadoc and UPGRADING.md have been fixed as required.