<div dir="ltr">Its good to see you back on the list!!<div><br></div><div>there was a typo!!, I did mean __non-existing__ process instead of __non-exiting__<br><div class="gmail_extra"><br></div><div class="gmail_extra">-Ashwini<br>
<br><div class="gmail_quote">On Tue, Dec 17, 2013 at 7:50 PM, Rob Landley <span dir="ltr"><<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im">On 11/22/2013 12:26:14 AM, Ashwini Sharma wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Hi All,<br>
<br>
Due to some issue with my email account, changing my email to<br>
<a href="mailto:ak.ashwini1981@gmail.com" target="_blank">ak.ashwini1981@gmail.com</a><br>
</blockquote>
<br></div>
Finally catching up on my email myself. :)<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
I came across a bug in pidof command. The issue is<br>
with _-o_ option. callback function returns _1_ for omit<br>
list, to names_to_pid().<br>
</blockquote>
<br></div>
Sigh. names_to_pid() is in pending, a command not in pending is calling it...<br>
<br>
(My bad.)<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
names_to_pid(), breaks the internal _for_ loop, at the same<br>
time it also breaks out of the _while(readdir())_ loop.<br>
Which is causing the issue, as other pids are not searched for.<br>
</blockquote>
<br></div>
Indeed, it should return 0 not 1.<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
for (curname = names; *curname; curname++)<br>
if (**curname == '/' ? !strcmp(cmd, *curname)<br>
: !strcmp(basename(cmd), basename(*curname)))<br>
if (callback(u, *curname)) break;<br>
- if (*curname) break;<br>
}<br>
<br>
Why break from _while_ loop, as there can be other processes in the list<br>
waiting for actioin.<br>
removing this fixes the issue.<br>
</blockquote>
<br></div>
Returning nonzero says to abort the loop. We shouldn't return nonzero if we want additional callbacks callbacks.<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
_names_to_pid()_ is also used by _killall_. killall also has issues,<br>
<br>
1. the exit status is always > 0, thats because of toys.exitval++<br>
in _main() function. Exit status need to be corrected.<br>
</blockquote>
<br></div>
Actually it's supposed to exit 1 if no processes were found, and 0 if it could kill at least one process. I'll have it zero out the exit code if a signal could successfully be delivered.<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
2. No error/warning message is displayed for a non-exiting process.<br>
as is one by GNU implementation. instead due to the _toys.exitval++_<br>
its always printing __ No such process __ without any process name.<br>
</blockquote>
<br></div>
Actually it prints ("bad %u", pid)?<br>
<br>
ret = kill(pid, TT.signum);<br>
if (ret == -1 && !(toys.optflags & FLAG_q))<br>
error_msg("bad %u", (unsigned)pid);<br>
<br>
That looks like it should be perror_msg() so it reports the errno reason.<br>
<br>
The "no such process" message was always printed because the exit value was never cleared.<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
No, the problem is if you kill things that don't exist in the ubuntu version:<br>
<br>
$ killall whackamole froglegs<br>
whackamole: no process found<br>
froglegs: no process found<br>
<br>
You get an error message for each one. And this infrastructure isn't tracking that. Although to be honest, the upstream infrastructure is lying:<br>
<br>
$ killall init whackamole<br>
init(1): Operation not permitted<br>
init: no process found<br>
whackamole: no process found<br>
<br>
And the errno behavior is slightly non-obvious:<br>
<br>
$ sleep 9999 &<br>
[1] 8016<br>
$ killall init sleep whackamole<br>
init(1): Operation not permitted<br>
init: no process found<br>
whackamole: no process found<br>
[1]+ Terminated sleep 9999<br>
$ echo $?<br>
1<br>
<br>
Rummage, rummage... Ok, if there was a command it couldn't kill (including "not found"), errno is 1. Order doesn't matter.<br>
<br>
Ideally I'd save the errno of each entry so the message was right, but I don't want to mark up optargs[] (and if I did it'd screw up killing a second command with the same name), and can't conveniently use toybuf as a bitfield for two reasons (already used for a message right now, not necessarily long enough).<br>
<br>
Speaking of not necessarily long enough:<br>
<br>
if (toys.optflags & FLAG_i) {<br>
sprintf(toybuf, "Signal %s(%d) ?", name, pid);<br>
<br>
Bad because toybuf is 4k and name is environment data that could be 128k, so somebody can trivially manipulate a heap overflow out of that.<br>
<br>
Right, if I switch that to xsmprintf() I can use toybuf. I can save the start of the name array into the globals and then do "name-start" to get the offset into the array. If I use it as a bitfield I can reproduce the original (broken) behavior, and can track 32768 commands, which is probably more than anyone will ever use. (With 128k environment space they'd all have to be 3 character command names to _fill_ that, you'd need 2 chars to surpass it...)<br>
<br>
If I use it as an array to store the errno... does it need to be short or char?<br>
<br>
$ cat arch/*/include/uapi/asm/errno* | awk '{print $3}' | sort -n<br>
...<br>
255<br>
256<br>
257<br>
516<br>
1133<br>
<br>
Sigh. Those last few aren't relevant (I grepped, they won't be produced by kill(2)), but from a complexity level I don't want to have to explain it in a giant comment, nor am I quite comfortable with 4096 entries as a limit where behavior magically changes.<br>
<br>
Screw it: malloc an array.<br>
<br>
The next question is, how do I get the position in the array? I was thinking I could do pointer subtraction but that gives me how many bytes of string data were used. Hack up names_to_pid() to pass it? (Only currently two users...) Do a for loop checking?<br>
<br>
Eh, I'll just do a for loop. Pathological case is less than 60k command line, and I don't have to compare strings (just pointers).<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
In order not to modify the lib function _names_to_pid_ too much, these<br>
killall issues could be fixed from callback function _kill_process()_.<br>
</blockquote>
<br></div>
Modifying the callback is the correct approach, yes. The names_to_pid() function just iterates over the array we give it to find pids, there shouldn't be any logic about what use we put the results to.<div class="im">
<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
For printing the warning message for non existent processes, can have a map<br>
for the processes which are signalled thru _kill_process_ and finally once<br>
_names_to_pid_ returns, print the message for non-signalled processes.<br>
<br>
Like to have your opinion.<br>
</blockquote>
<br></div>
I implemented the array version mentioned above before I actually read this far in the message. :)<br>
<br>
I need to debug a bit before checking in and it's bus-to-work time, but I'll see if I can get this checked in tonight.<span class=""><font color="#888888"><br>
<br>
Rob</font></span></blockquote></div><br></div></div></div>