Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use size_t for Process offset values #1588

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+154 −131
Open

Use size_t for Process offset values #1588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6777df4
PCPProcessTable_updateCmdline() minor logic adjustment
Explorer09 Feb 2, 2025
da6bf9b
Correct code commment in matchCmdlinePrefixWithExeSuffix()
Explorer09 Feb 2, 2025
fb60c0f
Minor code style adjustment in matchCmdlinePrefixWithExeSuffix()
Explorer09 Feb 2, 2025
f7d7017
Code simplification in Process_makeCommandStr()
Explorer09 Feb 2, 2025
a1f0182
Don't make highlights of zero-length cmdline basename
Explorer09 Feb 2, 2025
d7a428b
Improve process cmdline basename matching with procExe path
Explorer09 Feb 2, 2025
18962f6
Improve "comm" string highlighting in Process_makeCommandStr()
Explorer09 Feb 2, 2025
a9431da
Process: Use size_t for "cmdline" and "procExe" offsets
Explorer09 Feb 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 98 additions & 75 deletions Process.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,29 +64,26 @@ void Process_fillStarttimeBuffer(Process* this) {
*/
#define TASK_COMM_LEN 16

static bool findCommInCmdline(const char* comm, const char* cmdline, int cmdlineBasenameStart, int* pCommStart, int* pCommEnd) {
static bool findCommInCmdline(const char* comm, const char* cmdline, size_t cmdlineBasenameStart, size_t* pCommStart, size_t* pCommLen) {
/* Try to find procComm in tokenized cmdline - this might in rare cases
* mis-identify a string or fail, if comm or cmdline had been unsuitably
* modified by the process */
const char* tokenBase;
size_t tokenLen;
const size_t commLen = strlen(comm);

if (cmdlineBasenameStart < 0)
return false;
Comment on lines -75 to -76
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes a check when no basename was found in the command line …

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"No basename found" is no different from matching "comm" from the start of the "cmdline" string. You notice this function does not take cmdlineBasenameLen as input. "No basename found" may be indicated by cmdlineBasenameLen == 0, but it's irrelevant to this function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking the change in the code has been accounted for in a different way.


for (const char* token = cmdline + cmdlineBasenameStart; *token;) {
for (tokenBase = token; *token && *token != '\n'; ++token) {
if (*token == '/') {
tokenBase = token + 1;
}
}
tokenLen = token - tokenBase;
tokenLen = (size_t)(token - tokenBase);

if ((tokenLen == commLen || (tokenLen > commLen && commLen == (TASK_COMM_LEN - 1))) &&
strncmp(tokenBase, comm, commLen) == 0) {
*pCommStart = tokenBase - cmdline;
*pCommEnd = token - cmdline;
*pCommStart = (size_t)(tokenBase - cmdline);
*pCommLen = tokenLen;
return true;
}

Expand All @@ -99,15 +96,14 @@ static bool findCommInCmdline(const char* comm, const char* cmdline, int cmdline
return false;
}

static int matchCmdlinePrefixWithExeSuffix(const char* cmdline, int cmdlineBaseOffset, const char* exe, int exeBaseOffset, int exeBaseLen) {
int matchLen; /* matching length to be returned */
char delim; /* delimiter following basename */
static size_t matchCmdlinePrefixWithExeSuffix(const char* cmdline, size_t* cmdlineBasenameStart, const char* exe, size_t exeBaseOffset, size_t exeBaseLen) {
size_t matchLen; /* matching length to be returned */

/* cmdline prefix is an absolute path: it must match whole exe. */
if (cmdline[0] == '/') {
matchLen = exeBaseLen + exeBaseOffset;
if (strncmp(cmdline, exe, matchLen) == 0) {
delim = cmdline[matchLen];
char delim = cmdline[matchLen];
if (delim == 0 || delim == '\n' || delim == ' ') {
return matchLen;
}
Expand All @@ -121,35 +117,42 @@ static int matchCmdlinePrefixWithExeSuffix(const char* cmdline, int cmdlineBaseO
* that make htop's identification of the basename in cmdline unreliable.
* For e.g. /usr/libexec/gdm-session-worker modifies its cmdline to
* "gdm-session-worker [pam/gdm-autologin]" and htop ends up with
* proccmdlineBasenameEnd at "gdm-autologin]". This issue could arise with
* cmdlineBasenameStart at "gdm-autologin]". This issue could arise with
* chrome as well as it stores in cmdline its concatenated argument vector,
* without NUL delimiter between the arguments (which may contain a '/')
*
* So if needed, we adjust cmdlineBaseOffset to the previous (if any)
* component of the cmdline relative path, and retry the procedure. */
bool delimFound; /* if valid basename delimiter found */
size_t cmdlineBaseOffset = *cmdlineBasenameStart;
bool delimFound = true; /* if valid basename delimiter found */
do {
/* match basename */
matchLen = exeBaseLen + cmdlineBaseOffset;
if (cmdlineBaseOffset < exeBaseOffset &&
strncmp(cmdline + cmdlineBaseOffset, exe + exeBaseOffset, exeBaseLen) == 0) {
delim = cmdline[matchLen];
char delim = cmdline[matchLen];
if (delim == 0 || delim == '\n' || delim == ' ') {
int i, j;
size_t i, j;
/* reverse match the cmdline prefix and exe suffix */
for (i = cmdlineBaseOffset - 1, j = exeBaseOffset - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could underflow for both i and j …

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional. i and j may wrap over to (size_t)-1 and that's what the conditionals below are for.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't quite like doing it that way …

Especially as x >= 0 is a no-op (always true) for any unsigned x …

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any better way here? Note that the i >= 0 had been rewritten to i != (size_t)-1 by me when I changed the types to unsigned.

i >= 0 && j >= 0 && cmdline[i] == exe[j]; --i, --j)
i != (size_t)-1 && j != (size_t)-1 && cmdline[i] == exe[j]; --i, --j)
;

/* full match, with exe suffix being a valid relative path */
if (i < 0 && j >= 0 && exe[j] == '/')
if (i == (size_t)-1 && j != (size_t)-1 && exe[j] == '/') {
*cmdlineBasenameStart = cmdlineBaseOffset;
return matchLen;
}
}
}

/* Try to find the previous potential cmdlineBaseOffset - it would be
* preceded by '/' or nothing, and delimited by ' ' or '\n' */
for (delimFound = false, cmdlineBaseOffset -= 2; cmdlineBaseOffset > 0; --cmdlineBaseOffset) {
delimFound = false;
if (cmdlineBaseOffset <= 2) {
return 0;
}
for (cmdlineBaseOffset -= 2; cmdlineBaseOffset > 0; --cmdlineBaseOffset) {
if (delimFound) {
if (cmdline[cmdlineBaseOffset - 1] == '/') {
break;
Expand Down Expand Up @@ -309,17 +312,37 @@ void Process_makeCommandStr(Process* this, const Settings* settings) {
char* strStart = mc->str;
char* str = strStart;

int cmdlineBasenameStart = this->cmdlineBasenameStart;
int cmdlineBasenameEnd = this->cmdlineBasenameEnd;
size_t cmdlineBasenameStart = this->cmdlineBasenameStart;
size_t cmdlineBasenameLen = 0;
if (this->cmdlineBasenameEnd > this->cmdlineBasenameStart)
cmdlineBasenameLen = this->cmdlineBasenameEnd - this->cmdlineBasenameStart;

if (!cmdline) {
cmdlineBasenameStart = 0;
cmdlineBasenameEnd = 0;
cmdlineBasenameLen = 0;
cmdline = "(zombie)";
}

assert(cmdlineBasenameStart >= 0);
assert(cmdlineBasenameStart <= (int)strlen(cmdline));
assert(cmdlineBasenameStart <= strlen(cmdline));

size_t exeLen = 0;
size_t exeBasenameOffset = 0;
size_t exeBasenameLen = 0;
size_t matchLen = 0;
if (procExe) {
exeLen = strlen(procExe);
exeBasenameOffset = this->procExeBasenameOffset;
exeBasenameLen = exeLen - exeBasenameOffset;

assert(exeBasenameOffset <= strlen(procExe));

if (this->cmdline) {
matchLen = matchCmdlinePrefixWithExeSuffix(this->cmdline, &cmdlineBasenameStart, procExe, exeBasenameOffset, exeBasenameLen);
}
if (matchLen) {
cmdlineBasenameLen = exeBasenameLen;
}
}

if (!showMergedCommand || !procExe || !procComm) { /* fall back to cmdline */
if ((showMergedCommand || (Process_isUserlandThread(this) && showThreadNames)) && procComm && strlen(procComm)) { /* set column to or prefix it with comm */
Expand All @@ -337,37 +360,62 @@ void Process_makeCommandStr(Process* this, const Settings* settings) {
if (shadowDistPathPrefix && showProgramPath)
CHECK_AND_MARK_DIST_PATH_PREFIXES(cmdline);

if (cmdlineBasenameEnd > cmdlineBasenameStart)
WRITE_HIGHLIGHT(showProgramPath ? cmdlineBasenameStart : 0, cmdlineBasenameEnd - cmdlineBasenameStart, baseAttr, CMDLINE_HIGHLIGHT_FLAG_BASENAME);
if (cmdlineBasenameLen > 0) {
WRITE_HIGHLIGHT(showProgramPath ? cmdlineBasenameStart : 0, cmdlineBasenameLen, baseAttr, CMDLINE_HIGHLIGHT_FLAG_BASENAME);

if (this->procExeDeleted)
WRITE_HIGHLIGHT(showProgramPath ? cmdlineBasenameStart : 0, cmdlineBasenameEnd - cmdlineBasenameStart, delExeAttr, CMDLINE_HIGHLIGHT_FLAG_DELETED);
else if (this->usesDeletedLib)
WRITE_HIGHLIGHT(showProgramPath ? cmdlineBasenameStart : 0, cmdlineBasenameEnd - cmdlineBasenameStart, delLibAttr, CMDLINE_HIGHLIGHT_FLAG_DELETED);
if (this->procExeDeleted)
WRITE_HIGHLIGHT(showProgramPath ? cmdlineBasenameStart : 0, cmdlineBasenameLen, delExeAttr, CMDLINE_HIGHLIGHT_FLAG_DELETED);
else if (this->usesDeletedLib)
WRITE_HIGHLIGHT(showProgramPath ? cmdlineBasenameStart : 0, cmdlineBasenameLen, delLibAttr, CMDLINE_HIGHLIGHT_FLAG_DELETED);
}

(void)stpcpyWithNewlineConversion(str, cmdline + (showProgramPath ? 0 : cmdlineBasenameStart));

return;
}

int exeLen = strlen(this->procExe);
int exeBasenameOffset = this->procExeBasenameOffset;
int exeBasenameLen = exeLen - exeBasenameOffset;

assert(exeBasenameOffset >= 0);
assert(exeBasenameOffset <= (int)strlen(procExe));
size_t commLen = 0;

bool haveCommInExe = false;
if (procExe && procComm && (!Process_isUserlandThread(this) || showThreadNames)) {
haveCommInExe = strncmp(procExe + exeBasenameOffset, procComm, TASK_COMM_LEN - 1) == 0;
}
if (haveCommInExe) {
commLen = exeBasenameLen;
}

bool haveCommInCmdline = false;
size_t commStart = 0;

if (!haveCommInExe && this->cmdline && procComm && searchCommInCmdline && (!Process_isUserlandThread(this) || showThreadNames)) {
haveCommInCmdline = findCommInCmdline(procComm, cmdline, cmdlineBasenameStart, &commStart, &commLen);
}

if (!stripExeFromCmdline) {
matchLen = 0;
}
if (matchLen) {
/* strip the matched exe prefix */
cmdline += matchLen;

if (haveCommInCmdline) {
if (commStart == cmdlineBasenameStart) {
haveCommInExe = true;
haveCommInCmdline = false;
commStart = 0;
} else {
assert(commStart >= matchLen);
commStart -= matchLen;
}
}
}

/* Start with copying exe */
if (showProgramPath) {
if (shadowDistPathPrefix)
CHECK_AND_MARK_DIST_PATH_PREFIXES(procExe);
if (haveCommInExe)
WRITE_HIGHLIGHT(exeBasenameOffset, exeBasenameLen, commAttr, CMDLINE_HIGHLIGHT_FLAG_COMM);
WRITE_HIGHLIGHT(exeBasenameOffset, commLen, commAttr, CMDLINE_HIGHLIGHT_FLAG_COMM);
WRITE_HIGHLIGHT(exeBasenameOffset, exeBasenameLen, baseAttr, CMDLINE_HIGHLIGHT_FLAG_BASENAME);
if (this->procExeDeleted)
WRITE_HIGHLIGHT(exeBasenameOffset, exeBasenameLen, delExeAttr, CMDLINE_HIGHLIGHT_FLAG_DELETED);
Expand All @@ -376,7 +424,7 @@ void Process_makeCommandStr(Process* this, const Settings* settings) {
str = stpcpy(str, procExe);
} else {
if (haveCommInExe)
WRITE_HIGHLIGHT(0, exeBasenameLen, commAttr, CMDLINE_HIGHLIGHT_FLAG_COMM);
WRITE_HIGHLIGHT(0, commLen, commAttr, CMDLINE_HIGHLIGHT_FLAG_COMM);
WRITE_HIGHLIGHT(0, exeBasenameLen, baseAttr, CMDLINE_HIGHLIGHT_FLAG_BASENAME);
if (this->procExeDeleted)
WRITE_HIGHLIGHT(0, exeBasenameLen, delExeAttr, CMDLINE_HIGHLIGHT_FLAG_DELETED);
Expand All @@ -385,18 +433,6 @@ void Process_makeCommandStr(Process* this, const Settings* settings) {
str = stpcpy(str, procExe + exeBasenameOffset);
}

bool haveCommInCmdline = false;
int commStart = 0;
int commEnd = 0;

/* Try to match procComm with procExe's basename: This is reliable (predictable) */
if (searchCommInCmdline) {
/* commStart/commEnd will be adjusted later along with cmdline */
haveCommInCmdline = (!Process_isUserlandThread(this) || showThreadNames) && findCommInCmdline(procComm, cmdline, cmdlineBasenameStart, &commStart, &commEnd);
}

int matchLen = matchCmdlinePrefixWithExeSuffix(cmdline, cmdlineBasenameStart, procExe, exeBasenameOffset, exeBasenameLen);

bool haveCommField = false;

if (!haveCommInExe && !haveCommInCmdline && procComm && (!Process_isUserlandThread(this) || showThreadNames)) {
Expand All @@ -406,18 +442,6 @@ void Process_makeCommandStr(Process* this, const Settings* settings) {
haveCommField = true;
}

if (matchLen) {
if (stripExeFromCmdline) {
/* strip the matched exe prefix */
cmdline += matchLen;

commStart -= matchLen;
commEnd -= matchLen;
} else {
matchLen = 0;
}
}

if (!matchLen || (haveCommField && *cmdline)) {
/* cmdline will be a separate field */
WRITE_SEPARATOR;
Expand All @@ -427,7 +451,7 @@ void Process_makeCommandStr(Process* this, const Settings* settings) {
CHECK_AND_MARK_DIST_PATH_PREFIXES(cmdline);

if (!haveCommInExe && haveCommInCmdline && !haveCommField && (!Process_isUserlandThread(this) || showThreadNames))
WRITE_HIGHLIGHT(commStart, commEnd - commStart, commAttr, CMDLINE_HIGHLIGHT_FLAG_COMM);
WRITE_HIGHLIGHT(commStart, commLen, commAttr, CMDLINE_HIGHLIGHT_FLAG_COMM);

/* Display cmdline if it hasn't been consumed by procExe */
if (*cmdline)
Expand All @@ -445,20 +469,20 @@ void Process_writeCommand(const Process* this, int attr, int baseAttr, RichStrin
const ProcessMergedCommand* mc = &this->mergedCommand;
const char* mergedCommand = mc->str;

int strStart = RichString_size(str);
size_t strStart = RichString_size(str);

const Settings* settings = this->super.host->settings;
const bool highlightBaseName = settings->highlightBaseName;
const bool highlightSeparator = true;
const bool highlightDeleted = settings->highlightDeletedExe;

if (!mergedCommand) {
int len = 0;
size_t len = 0;
const char* cmdline = this->cmdline;

if (highlightBaseName || !settings->showProgramPath) {
int basename = 0;
for (int i = 0; i < this->cmdlineBasenameEnd; i++) {
size_t basename = 0;
for (size_t i = 0; i < this->cmdlineBasenameEnd; i++) {
if (cmdline[i] == '/') {
basename = i + 1;
} else if (cmdline[i] == ':') {
Expand Down Expand Up @@ -850,7 +874,7 @@ bool Process_rowMatchesFilter(const Row* super, const Table* table) {
void Process_init(Process* this, const Machine* host) {
Row_init(&this->super, host);

this->cmdlineBasenameEnd = -1;
this->cmdlineBasenameEnd = 0;
this->st_uid = (uid_t)-1;
}

Expand Down Expand Up @@ -1002,12 +1026,12 @@ void Process_updateComm(Process* this, const char* comm) {
this->mergedCommand.lastUpdate = 0;
}

static int skipPotentialPath(const char* cmdline, int end) {
static size_t skipPotentialPath(const char* cmdline, size_t end) {
if (cmdline[0] != '/')
return 0;

int slash = 0;
for (int i = 1; i < end; i++) {
size_t slash = 0;
for (size_t i = 1; i < end; i++) {
if (cmdline[i] == '/' && cmdline[i + 1] != '\0') {
slash = i + 1;
continue;
Expand All @@ -1023,11 +1047,10 @@ static int skipPotentialPath(const char* cmdline, int end) {
return slash;
}

void Process_updateCmdline(Process* this, const char* cmdline, int basenameStart, int basenameEnd) {
assert(basenameStart >= 0);
assert((cmdline && basenameStart < (int)strlen(cmdline)) || (!cmdline && basenameStart == 0));
void Process_updateCmdline(Process* this, const char* cmdline, size_t basenameStart, size_t basenameEnd) {
assert((cmdline && basenameStart < strlen(cmdline)) || (!cmdline && basenameStart == 0));
assert((basenameEnd > basenameStart) || (basenameEnd == 0 && basenameStart == 0));
assert((cmdline && basenameEnd <= (int)strlen(cmdline)) || (!cmdline && basenameEnd == 0));
assert((cmdline && basenameEnd <= strlen(cmdline)) || (!cmdline && basenameEnd == 0));

if (!this->cmdline && !cmdline)
return;
Expand Down Expand Up @@ -1060,7 +1083,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) ? (lastSlash - exe + 1) : 0;
this->procExeBasenameOffset = (lastSlash && *(lastSlash + 1) != '\0' && lastSlash != exe) ? (size_t)(lastSlash - exe + 1) : 0;
} else {
this->procExe = NULL;
this->procExeBasenameOffset = 0;
Expand Down
8 changes: 4 additions & 4 deletions Process.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ typedef struct Process_ {
char* cmdline;

/* End Offset in cmdline of the process basename */
int cmdlineBasenameEnd;
size_t cmdlineBasenameEnd;

/* Start Offset in cmdline of the process basename */
int cmdlineBasenameStart;
size_t cmdlineBasenameStart;

/* The process' "command" name */
char* procComm;
Expand All @@ -144,7 +144,7 @@ typedef struct Process_ {
char* procCwd;

/* Offset in procExe of the process basename */
int procExeBasenameOffset;
size_t procExeBasenameOffset;

/* Tells if the executable has been replaced in the filesystem since start */
bool procExeDeleted;
Expand Down Expand Up @@ -326,7 +326,7 @@ int Process_compareByKey_Base(const Process* p1, const Process* p2, ProcessField
const char* Process_getCommand(const Process* this);

void Process_updateComm(Process* this, const char* comm);
void Process_updateCmdline(Process* this, const char* cmdline, int basenameStart, int basenameEnd);
void Process_updateCmdline(Process* this, const char* cmdline, size_t basenameStart, size_t basenameEnd);
void Process_updateExe(Process* this, const char* exe);

/* This function constructs the string that is displayed by
Expand Down
Loading
Loading