Use size_t for Process offset values #1588
Conversation
Signed-off-by: Kang-Che Sung <[email protected]>
Explorer09
force-pushed
the
process-offsets-size-type
branch
from
454cf1e to
87ab00e
Compare
| @@ -1057,7 +1057,7 @@ void Process_updateExe(Process* this, const char* exe) { | |||
| if (exe) { | |||
| this->procExe = xStrdup(exe); | |||
| const char* lastSlash = strrchr(exe, '/'); | |||
| this->procExeBasenameOffset = (lastSlash && *(lastSlash + 1) != '\0' && lastSlash != exe) ? (size_t)(lastSlash - exe + 1) : 0; | |||
| this->procExeBasenameOffset = (lastSlash > exe && *(lastSlash + 1) != '\0') ? (size_t)(lastSlash - exe + 1) : 0; | |||
There was a problem hiding this comment.
Side note: I have no idea why this conditional needs a lastSlash != exe. I thought it should be legitimate when lastSlash == exe, that is, when the exe string begins with a slash such as "/vmlinuz".
BenBE
added
enhancement
|
Well, it looks like the changes to The I think the logic there needs to be revised. While I could workaround by using I would try to make a commit that properly fixes that. |
Improve readability of local variables. Signed-off-by: Kang-Che Sung <[email protected]>
The matchCmdlinePrefixWithExeSuffix() internal function (in Process.c) can match basenames in multiple tries in case the "cmdlineBasenameStart" input value is unreliable. Take advantage of this and update "cmdlineBasenameStart" with the new offset detected by the function. Also make the matching behavior consistent regardless of "showMergedCommand" setting. Signed-off-by: Kang-Che Sung <[email protected]>
Explorer09
force-pushed
the
process-offsets-size-type
branch
from
87ab00e to
c0c9a0b
Compare
Add a special case when the process "comm" string is found in "cmdline", but at the starting basename position that would be stripped by "stripExeFromCmdline" setting. This special case will now highlight the basename of "exe" string as well as the first token of "cmdline" (as the two strings are concatenated together when displaying). Signed-off-by: Kang-Che Sung <[email protected]>
Convert the members "cmdlineBasenameStart", "cmdlineBasenameEnd" and "procExeBasenameOffset" in "Process" to size_t type. Also upgrade many routines that search for "basenames", COMM, and PROC_EXE string to use size_t for iterators. The "cmdlineBasenameEnd" value is no longer allowed to negative. It is now set to 0 during Process_init(). Signed-off-by: Kang-Che Sung <[email protected]>
Signed-off-by: Kang-Che Sung <[email protected]>
Signed-off-by: Kang-Che Sung <[email protected]>
Explorer09
force-pushed
the
process-offsets-size-type
branch
from
c0c9a0b to
707cde1
Compare
|
FYI; the last time I needed to work on this logic, it took me the better of 3 months to track down some issue with the command line handling in this code. So be careful with your changes; the details are important with this part of the code … |
|
@BenBE But I'm not sure I should file a new PR for it or just merge with this one. The new changes depend on this PR and I've seen you added tags and categories to this PR already. |
|
@Explorer09 Make a second PR as this makes discussing the changes easier. From a quick view at your commits on that branch I already noticed some things I'd like to change with that patchset … |
Convert
cmdlineBasenameStart,cmdlineBasenameEndandprocExeBasenameOffsetvalues inProcesstosize_ttype. Improve functions that search for "basenames", "comm" and "procExe" to usesize_tfor offsets as well.I made these set of code changes before the length properties of
RichStringobject can be upgraded tosize_t.Note: This PR contains many platform specific code changes. I didn't test all of them. It is possible that I broke something.