Lukasz Skalski <[email protected]> said:
Description of problem:
Pidof can't get the pid when using full path.
Sigh. lib/pending.c is the place for library code I'm still not
comfortable with. Let's see...
The guts of this function are reimplementing readfile(), which implies
readfile isn't flexible enough and should be able to take a buffer the
way readline() can. (And then malloc its own buffer if that buffer is
NULL.) I don't hugely care about it returning the length (it's a null
terminated string, don't use it for stuff that can have another null in
it).
The name of the function is way too long, let's just call it
name_to_pid() and rename the two existing users...
I'm a bit uncomfortable declaring a 4k buffer on the stack. I'm aware
linux userspace generally has plenty of space for this, but a lot of
environments don't (kernel, u-boot, thread stacks, nommu systems) and
I'm trying to be agnostic about where this code gets reused from. (This
is exactly the sort of thing toybuf is for, except that's left free for
command code to use, so library functions can't use it. Possibly I
should look to see if a libbuf would make sense?)
Manpage says:
"[...] When pidof is invoked with a full pathname to the program it
should
find the pid of, it is reasonably safe. Otherwise it is possible that
it
returns pids of running programs that happen to have the same name as
the
program you're after but are actually other programs [...]"
This patch fix above problem in for_each_pid_with_name_in() function
(lib/pending.c).
Doesn't calling basename on both entries defeat the whole "reasonably
safe" bit you just quoted?
Ok, I can compare the first char to '/' to find absolute paths, but
what do I do with a relative path? It's still specifying a specific
file, but we have to normalize it against cwd of the process to get the
absolute path. Also, what about "/usr/../bin/ls"? Should we require the
caller to supply normalized absolute paths, or convert them on the fly
here? If we do convert them, is storing the pointer in the original
array acceptable, or should I malloc a new one? Hmmm... I should _not_
do realpath if the kernel doesn't, because busybox cares what the
symlink was named.
Ah, the old tension between "do it properly" and "be simple". I guess
for the moment starting with / means compare the whole thing, not
starting with / means compare.
As long as I'm renaming the function: system calls return 0 for success
and -1 for failure, let's make 0 the default return so we have the
option in future of specifying different non-default behaviors (or at
least making the return code mean something specific).
While adjusting the callers: killall.c resets toys.exitval to 0 each
time it tries to kill something. This means its return code can only be
failure if the _last_ thing it tried to kill didn't work, that's
probably wrong. It defaults to 0, so let's just yank that line...
Also, use toybox's error_msg() instead of libc's perror(), and print
the error before the success message in the -v case (calling printf
changes errno)...
Right, let's check this in before going _too_ far down that tangent.
Rob
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net
