Messages in this thread | | | Date | Mon, 25 Apr 2022 21:45:59 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH 1/2] rtla: Remove procps-ng dependency |
| |
On Fri, 22 Apr 2022 12:01:31 +0200 Daniel Bristot de Oliveira <bristot@kernel.org> wrote:
> + > +/* > + * procfs_is_workload_pid - check if a procfs entry contains a workload_prefix* comm > + * > + * Check if the procfs entry is a directory of a process, and then check if the > + * process has a comm with the prefix set in char *workload_prefix. As the > + * current users of this function only check for kernel threads, there is no > + * need to check for the threads for the process. > + * > + * Return: True if the proc_entry contains a comm file with workload_prefix*. > + * Otherwise returns false. > + */ > +static int procfs_is_workload_pid(const char *workload_prefix, struct dirent *proc_entry) > +{ > + char comm_path[MAX_PATH], comm[MAX_PATH];
This is probably fine (but there is one issue), but I would have done this a little different.
int len = strlen(workload_prefix); char comm[len + 1];
> + int comm_fd, retval; > + char *t_name; > + > + if (proc_entry->d_type != DT_DIR) > + return 0; > + > + if (*proc_entry->d_name == '.') > + return 0; > + > + /* check if the string is a pid */ > + for (t_name = proc_entry->d_name; t_name; t_name++) { > + if (!isdigit(*t_name)) > + break; > + } > + > + if (*t_name != '\0') > + return 0; > + > + snprintf(comm_path, MAX_PATH, "/proc/%s/comm", proc_entry->d_name); > + comm_fd = open(comm_path, O_RDONLY); > + if (comm_fd < 0) > + return 0; > + > + memset(comm, 0, MAX_PATH);
No need for the memset.
> + retval = read(comm_fd, comm, MAX_PATH);
retval = read(comm_fd, comm, len + 1);
> + > + close(comm_fd); > + > + if (retval <= 0) > + return 0;
if (comm[len] != '\n') return 0;
> + > + retval = !strncmp(workload_prefix, comm, strlen(workload_prefix));
What happens if strlen(workload_prefix) is greater than MAX_PATH? ;-)
retval = !strncmp(workload_prefix, comm, len);
But that's me. If you want to keep this as is, let me know.
-- Steve
> + if (!retval) > + return 0; > + > + /* comm already have \n */ > + debug_msg("Found workload pid:%s comm:%s", proc_entry->d_name, comm); > + > + return 1; > +} > +
| |