andrewdavidmackenzie
commented
8 months ago Thanks very much for the investigation and proposed fix. I have mentioned the original code contributor to see if he recalls where that came from.
Looking at this:
match ret {
value if value <= 0 => Err(io::Error::last_os_error()), <<<<<<<<<<<<<<<<<<<<<<<<<<<
_ => {
let items_count = ret as usize / mem::size_of::<u32>() - 1;
unsafe {
pids.set_len(items_count);
}
Ok(pids)
}
}and the C source:
int
proc_listpids(uint32_t type, uint32_t typeinfo, void *buffer, int buffersize)
{
int retval;
if ((type == PROC_ALL_PIDS) || (type == PROC_PGRP_ONLY) || (type == PROC_TTY_ONLY) || (type == PROC_UID_ONLY) || (type == PROC_RUID_ONLY)) {
if ((retval = __proc_info(1, type, typeinfo,(uint64_t)0, buffer, buffersize)) == -1)
return(0); <<<<<<<<<<<<<<<<<<<<<<<<<<<<
} else {
errno = EINVAL;
retval = 0; <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
}
return(retval);
}- if there is no error, but there are zero pids, it returns zero
- if there is an error, it returns zero and an errno
So, I think this line: (fyi @mjpieters )
value if value <= 0 => Err(io::Error::last_os_error()),can't distinguish the two cases and we need to look at errno to know if the 0 indicates
- An error
- there was no error but there are zero pids to return
Something like:
match ret {
value if value < 0 || errno => Err(io::Error::last_os_error()), // psuedo-code
_ => {
let items_count = ret as usize / mem::size_of::<u32>(); // 0 / x = 0
unsafe {
pids.set_len(items_count);
}
Ok(pids)
}
}WDYT?
Testing
Any ideas for tests we could add for the:
- zero pids case
- normal case
I see test_listpids_parent_pid() exists, but need to study it.
I suspect this:
let pids = listpids(ProcFilter::ByParentProcess { ppid: *ppid }).unwrap_or_default();
maybe avoiding/handling any error during running the test?
And I think this:
if bsdinfo_pids.len() <= 1 {
continue;
}will skip the call to listpids(parent)
…at for a ppid with a single child, I would get an empty list using 'let pids = pids_by_type(libproc::processes::ProcFilter::ByParentProcess { ppid: MY_PROC_PID }).unwrap();'. Where the same direct call in C would yield a list with 1 PID. Digging into libproc-rs, I noticed that the size was being adjusted -1, and when corrected I now see the correct single child pid.
One worrisome thing is what is happening now when there are actually zero children for a PID? I think it's producing an error.