Discussion:
[Toybox] Fix, killall is not allowed to kill itself
Elie De Brauwer
2012-12-16 15:54:24 UTC
Permalink
Hi all,

The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."

The current implementation of killall does not adhere to this rule, with
the patch in attach killall will adhere to this rule.

Before:
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed

After:
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process


Note: this patch should be applied after my previous submission, if for
some reason the previous path is unwanted let me know and I'll resubmit
this one.

my 2 cents
E.
--
Elie De Brauwer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: killall_may_not_kill_itself.patch
Type: text/x-patch
Size: 917 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121216/8be56dee/attachment.bin>
Elie De Brauwer
2012-12-16 17:12:59 UTC
Permalink
All,

In my previous patch, the pif shouldn't be stored inside the GLOBAL(),
it should be just a regular global variable. In attach a patch which
corrects this.

gr
E.
Post by Elie De Brauwer
Hi all,
The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this rule, with
the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
Note: this patch should be applied after my previous submission, if for
some reason the previous path is unwanted let me know and I'll resubmit
this one.
my 2 cents
E.
--
Elie De Brauwer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: killall_may_not_kill_itself_take2.patch
Type: text/x-patch
Size: 901 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121216/29541f5c/attachment.bin>
Rob Landley
2012-12-23 20:42:35 UTC
Permalink
Post by Elie De Brauwer
All,
In my previous patch, the pif shouldn't be stored inside the
GLOBAL(), it
should be just a regular global variable. In attach a patch which
corrects this.
Ok, I finally took a proper look at this, and I don't understand why
can't we have pid_t in GLOBALS()? It's a standard type, it's not being
used as an argument target (in which case it would have to be a long),
the #includes in toys.h give us that type...

Could you explain your reasoning? (In the meantime, I've applied your
first patch.)

Thanks,

Rob
Elie De Brauwer
2012-12-23 20:45:57 UTC
Permalink
Post by Rob Landley
All,
In my previous patch, the pif shouldn't be stored inside the GLOBAL(), it
should be just a regular global variable. In attach a patch which
corrects this.
Ok, I finally took a proper look at this, and I don't understand why
can't we have pid_t in GLOBALS()? It's a standard type, it's not being
used as an argument target (in which case it would have to be a long),
the #includes in toys.h give us that type...
Could you explain your reasoning? (In the meantime, I've applied your
first patch.)
Mea culpa, I thought GLOBALS() was only meant for command line parsing,
but apparently it isn't.

gr
E.
--
Elie De Brauwer
Elie De Brauwer
2012-12-23 20:45:57 UTC
Permalink
Post by Rob Landley
All,
In my previous patch, the pif shouldn't be stored inside the GLOBAL(), it
should be just a regular global variable. In attach a patch which
corrects this.
Ok, I finally took a proper look at this, and I don't understand why
can't we have pid_t in GLOBALS()? It's a standard type, it's not being
used as an argument target (in which case it would have to be a long),
the #includes in toys.h give us that type...
Could you explain your reasoning? (In the meantime, I've applied your
first patch.)
Mea culpa, I thought GLOBALS() was only meant for command line parsing,
but apparently it isn't.

gr
E.
--
Elie De Brauwer
Rob Landley
2012-12-23 20:42:35 UTC
Permalink
Post by Elie De Brauwer
All,
In my previous patch, the pif shouldn't be stored inside the
GLOBAL(), it
should be just a regular global variable. In attach a patch which
corrects this.
Ok, I finally took a proper look at this, and I don't understand why
can't we have pid_t in GLOBALS()? It's a standard type, it's not being
used as an argument target (in which case it would have to be a long),
the #includes in toys.h give us that type...

Could you explain your reasoning? (In the meantime, I've applied your
first patch.)

Thanks,

Rob
David Seikel
2012-12-16 17:31:49 UTC
Permalink
On Sun, 16 Dec 2012 16:54:24 +0100 Elie De Brauwer
Post by Elie De Brauwer
The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this rule,
with the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
It's 3AM and way past my bed time, but after it's run out of things to
kill, it just kills itself. Seems pointless to not allow suicide on
something that will just suicide anyway, but I need sleep.

Like rm'ing something that does not exist, the end result is the same,
except for the useless error. The thing does not exist, killall is no
longer running. The error message is superfluous, as the result you
wanted is exactly what you get.
--
A big old stinking pile of genius that no one wants
coz there are too many silver coated monkeys in the world.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121217/1fd9bfc6/attachment-0001.pgp>
Rob Landley
2012-12-17 08:57:39 UTC
Permalink
Post by David Seikel
On Sun, 16 Dec 2012 16:54:24 +0100 Elie De Brauwer
Post by Elie De Brauwer
The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this rule,
with the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
It's 3AM and way past my bed time, but after it's run out of things to
kill, it just kills itself. Seems pointless to not allow suicide on
something that will just suicide anyway, but I need sleep.
Like rm'ing something that does not exist, the end result is the same,
except for the useless error. The thing does not exist, killall is no
longer running. The error message is superfluous, as the result you
wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID namespace
wraps), so if you kill things in increasing PID order this killall
instance isn't guaranteed to be last so it won't necessarily kill
everything it should.

Rob
David Seikel
2012-12-24 02:45:07 UTC
Permalink
Post by Rob Landley
Post by David Seikel
On Sun, 16 Dec 2012 16:54:24 +0100 Elie De Brauwer
Post by Elie De Brauwer
The LSB specification of killall says "A killall process never
kills itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this
rule, with the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
It's 3AM and way past my bed time, but after it's run out of things
to kill, it just kills itself. Seems pointless to not allow
suicide on something that will just suicide anyway, but I need
sleep.
Like rm'ing something that does not exist, the end result is the
same, except for the useless error. The thing does not exist,
killall is no longer running. The error message is superfluous, as
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order this
killall instance isn't guaranteed to be last so it won't necessarily
kill everything it should.
A valid objection for not killing yourself until you are done killing
everyone else. No excuse for an error message though, once killall is
done, the requested task is accomplished, that's not an error, that's
success.
--
A big old stinking pile of genius that no one wants
coz there are too many silver coated monkeys in the world.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121224/139c1ff2/attachment.pgp>
Rob Landley
2012-12-24 05:25:46 UTC
Permalink
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Like rm'ing something that does not exist, the end result is the
same, except for the useless error.
Using the host version on Ubuntu:

$ rm doesnotexist
rm: cannot remove `doesnotexist': No such file or directory

You're thinking rm -f maybe?
Post by David Seikel
Post by Rob Landley
Post by David Seikel
The thing does not exist,
killall is no longer running. The error message is superfluous,
as
Post by Rob Landley
Post by David Seikel
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order this
killall instance isn't guaranteed to be last so it won't necessarily
kill everything it should.
A valid objection for not killing yourself until you are done killing
everyone else. No excuse for an error message though, once killall is
done, the requested task is accomplished, that's not an error, that's
success.
Using the host version in ubuntu:

$ killall doesnotexist
doesnotexist: no process found

You're thinking killall -q maybe? Which toybox implements?

./toybox killall -q killall

Rob
David Seikel
2012-12-24 05:44:27 UTC
Permalink
Post by Rob Landley
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Like rm'ing something that does not exist, the end result is the
same, except for the useless error.
$ rm doesnotexist
rm: cannot remove `doesnotexist': No such file or directory
You're thinking rm -f maybe?
Post by David Seikel
Post by Rob Landley
Post by David Seikel
The thing does not exist,
killall is no longer running. The error message is
superfluous,
as
Post by Rob Landley
Post by David Seikel
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order
this killall instance isn't guaranteed to be last so it won't
necessarily kill everything it should.
A valid objection for not killing yourself until you are done
killing everyone else. No excuse for an error message though, once
killall is done, the requested task is accomplished, that's not an
error, that's success.
$ killall doesnotexist
doesnotexist: no process found
You're thinking killall -q maybe? Which toybox implements?
./toybox killall -q killall
Yes, the "killall killall" case that this thread is all about.

I'm just ranting against the fact that getting the end result you asked
for (the thing you are trying to delete does not exist, the thing you
are trying to kill is dead) is considered an error. The state of
things is precisely how you wanted them to be, what's the point of
declaring that as an error condition?

It's just a rant, nothing more. I know toybox will continue to stick to
the the standards. I'm allowed to point out that I think the standards
are stupid some times. I think it's particularly hilarious that the
only way to "successfully" remove something that does not exist is to
forcefully remove it. Kinda like beating a dead horse, by nuking it
from orbit, the horse isn't any more dead. lol
--
A big old stinking pile of genius that no one wants
coz there are too many silver coated monkeys in the world.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121224/6dfeca6a/attachment.pgp>
Rob Landley
2012-12-24 06:01:21 UTC
Permalink
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Like rm'ing something that does not exist, the end result is
the
Post by Rob Landley
Post by David Seikel
Post by Rob Landley
Post by David Seikel
same, except for the useless error.
$ rm doesnotexist
rm: cannot remove `doesnotexist': No such file or directory
You're thinking rm -f maybe?
Post by David Seikel
Post by Rob Landley
Post by David Seikel
The thing does not exist,
killall is no longer running. The error message is
superfluous,
as
Post by Rob Landley
Post by David Seikel
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order
this killall instance isn't guaranteed to be last so it won't
necessarily kill everything it should.
A valid objection for not killing yourself until you are done
killing everyone else. No excuse for an error message though,
once
Post by Rob Landley
Post by David Seikel
killall is done, the requested task is accomplished, that's not an
error, that's success.
$ killall doesnotexist
doesnotexist: no process found
You're thinking killall -q maybe? Which toybox implements?
./toybox killall -q killall
Yes, the "killall killall" case that this thread is all about.
"killall -q doesnotexist" works the exact same way.
Post by David Seikel
I'm just ranting against the fact that getting the end result you asked
for (the thing you are trying to delete does not exist, the thing you
are trying to kill is dead) is considered an error. The state of
things is precisely how you wanted them to be, what's the point of
declaring that as an error condition?
You requested an action that could not be performed. This is an error.
There is a flag to indicate that you do not consider it an error. This
is the existing design of _both_ tools.

I'm not seeing a problem here.
Post by David Seikel
It's just a rant, nothing more. I know toybox will continue to stick to
the the standards. I'm allowed to point out that I think the
standards
are stupid some times. I think it's particularly hilarious that the
only way to "successfully" remove something that does not exist is to
forcefully remove it.
I note that neither the "unlink" nor "rmdir" commands care about the
target's permission flags, so pausing to warn you about deleting them
is an artificial check in the rm command. All the -f flag does is not
tell you about anything that isn't an actual failure. So you could just
as easily say "-f" is "only report failures".
Post by David Seikel
Kinda like beating a dead horse, by nuking it
from orbit, the horse isn't any more dead. lol
In this case, the horse is alive and well.

Rob
Rob Landley
2012-12-24 06:01:21 UTC
Permalink
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Like rm'ing something that does not exist, the end result is
the
Post by Rob Landley
Post by David Seikel
Post by Rob Landley
Post by David Seikel
same, except for the useless error.
$ rm doesnotexist
rm: cannot remove `doesnotexist': No such file or directory
You're thinking rm -f maybe?
Post by David Seikel
Post by Rob Landley
Post by David Seikel
The thing does not exist,
killall is no longer running. The error message is
superfluous,
as
Post by Rob Landley
Post by David Seikel
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order
this killall instance isn't guaranteed to be last so it won't
necessarily kill everything it should.
A valid objection for not killing yourself until you are done
killing everyone else. No excuse for an error message though,
once
Post by Rob Landley
Post by David Seikel
killall is done, the requested task is accomplished, that's not an
error, that's success.
$ killall doesnotexist
doesnotexist: no process found
You're thinking killall -q maybe? Which toybox implements?
./toybox killall -q killall
Yes, the "killall killall" case that this thread is all about.
"killall -q doesnotexist" works the exact same way.
Post by David Seikel
I'm just ranting against the fact that getting the end result you asked
for (the thing you are trying to delete does not exist, the thing you
are trying to kill is dead) is considered an error. The state of
things is precisely how you wanted them to be, what's the point of
declaring that as an error condition?
You requested an action that could not be performed. This is an error.
There is a flag to indicate that you do not consider it an error. This
is the existing design of _both_ tools.

I'm not seeing a problem here.
Post by David Seikel
It's just a rant, nothing more. I know toybox will continue to stick to
the the standards. I'm allowed to point out that I think the
standards
are stupid some times. I think it's particularly hilarious that the
only way to "successfully" remove something that does not exist is to
forcefully remove it.
I note that neither the "unlink" nor "rmdir" commands care about the
target's permission flags, so pausing to warn you about deleting them
is an artificial check in the rm command. All the -f flag does is not
tell you about anything that isn't an actual failure. So you could just
as easily say "-f" is "only report failures".
Post by David Seikel
Kinda like beating a dead horse, by nuking it
from orbit, the horse isn't any more dead. lol
In this case, the horse is alive and well.

Rob

David Seikel
2012-12-24 05:44:27 UTC
Permalink
Post by Rob Landley
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Like rm'ing something that does not exist, the end result is the
same, except for the useless error.
$ rm doesnotexist
rm: cannot remove `doesnotexist': No such file or directory
You're thinking rm -f maybe?
Post by David Seikel
Post by Rob Landley
Post by David Seikel
The thing does not exist,
killall is no longer running. The error message is
superfluous,
as
Post by Rob Landley
Post by David Seikel
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order
this killall instance isn't guaranteed to be last so it won't
necessarily kill everything it should.
A valid objection for not killing yourself until you are done
killing everyone else. No excuse for an error message though, once
killall is done, the requested task is accomplished, that's not an
error, that's success.
$ killall doesnotexist
doesnotexist: no process found
You're thinking killall -q maybe? Which toybox implements?
./toybox killall -q killall
Yes, the "killall killall" case that this thread is all about.

I'm just ranting against the fact that getting the end result you asked
for (the thing you are trying to delete does not exist, the thing you
are trying to kill is dead) is considered an error. The state of
things is precisely how you wanted them to be, what's the point of
declaring that as an error condition?

It's just a rant, nothing more. I know toybox will continue to stick to
the the standards. I'm allowed to point out that I think the standards
are stupid some times. I think it's particularly hilarious that the
only way to "successfully" remove something that does not exist is to
forcefully remove it. Kinda like beating a dead horse, by nuking it
from orbit, the horse isn't any more dead. lol
--
A big old stinking pile of genius that no one wants
coz there are too many silver coated monkeys in the world.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121224/6dfeca6a/attachment-0002.pgp>
Rob Landley
2012-12-24 05:25:46 UTC
Permalink
Post by David Seikel
Post by Rob Landley
Post by David Seikel
Like rm'ing something that does not exist, the end result is the
same, except for the useless error.
Using the host version on Ubuntu:

$ rm doesnotexist
rm: cannot remove `doesnotexist': No such file or directory

You're thinking rm -f maybe?
Post by David Seikel
Post by Rob Landley
Post by David Seikel
The thing does not exist,
killall is no longer running. The error message is superfluous,
as
Post by Rob Landley
Post by David Seikel
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order this
killall instance isn't guaranteed to be last so it won't necessarily
kill everything it should.
A valid objection for not killing yourself until you are done killing
everyone else. No excuse for an error message though, once killall is
done, the requested task is accomplished, that's not an error, that's
success.
Using the host version in ubuntu:

$ killall doesnotexist
doesnotexist: no process found

You're thinking killall -q maybe? Which toybox implements?

./toybox killall -q killall

Rob
David Seikel
2012-12-24 02:45:07 UTC
Permalink
Post by Rob Landley
Post by David Seikel
On Sun, 16 Dec 2012 16:54:24 +0100 Elie De Brauwer
Post by Elie De Brauwer
The LSB specification of killall says "A killall process never
kills itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this
rule, with the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
It's 3AM and way past my bed time, but after it's run out of things
to kill, it just kills itself. Seems pointless to not allow
suicide on something that will just suicide anyway, but I need
sleep.
Like rm'ing something that does not exist, the end result is the
same, except for the useless error. The thing does not exist,
killall is no longer running. The error message is superfluous, as
the result you wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID
namespace wraps), so if you kill things in increasing PID order this
killall instance isn't guaranteed to be last so it won't necessarily
kill everything it should.
A valid objection for not killing yourself until you are done killing
everyone else. No excuse for an error message though, once killall is
done, the requested task is accomplished, that's not an error, that's
success.
--
A big old stinking pile of genius that no one wants
coz there are too many silver coated monkeys in the world.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121224/139c1ff2/attachment-0002.pgp>
Rob Landley
2012-12-17 08:57:39 UTC
Permalink
Post by David Seikel
On Sun, 16 Dec 2012 16:54:24 +0100 Elie De Brauwer
Post by Elie De Brauwer
The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this rule,
with the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
It's 3AM and way past my bed time, but after it's run out of things to
kill, it just kills itself. Seems pointless to not allow suicide on
something that will just suicide anyway, but I need sleep.
Like rm'ing something that does not exist, the end result is the same,
except for the useless error. The thing does not exist, killall is no
longer running. The error message is superfluous, as the result you
wanted is exactly what you get.
I think the objection is that PIDs aren't monotonic (the PID namespace
wraps), so if you kill things in increasing PID order this killall
instance isn't guaranteed to be last so it won't necessarily kill
everything it should.

Rob
Rob Landley
2012-12-23 21:29:40 UTC
Permalink
Post by Elie De Brauwer
Post by Rob Landley
All,
In my previous patch, the pif shouldn't be stored inside the
GLOBAL(), it
should be just a regular global variable. In attach a patch which
corrects this.
Ok, I finally took a proper look at this, and I don't understand why
can't we have pid_t in GLOBALS()? It's a standard type, it's not
being
used as an argument target (in which case it would have to be a
long),
the #includes in toys.h give us that type...
Could you explain your reasoning? (In the meantime, I've applied your
first patch.)
Mea culpa, I thought GLOBALS() was only meant for command line
parsing,
but apparently it isn't.
The first few entries in GLOBALS have to match up with the option
string's use of it (if any), but everything after that is just normal
global variables (initialized to 0).

I usually mark this by putting a blank line between the last option
slot and the first non-option global, when it's got both in there.

The advantage of GLOBALS() is the same memory gets used for all
commands (union of structures), so we're not allocating a huge bss. It
also means that if a command has to call another command it can memset
the globals block and be sure that everything starts in the same
initial state even if it's been run more than once since exec().
(Useful for toysh().)

Rob
Elie De Brauwer
2012-12-16 15:54:24 UTC
Permalink
Hi all,

The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."

The current implementation of killall does not adhere to this rule, with
the patch in attach killall will adhere to this rule.

Before:
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed

After:
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process


Note: this patch should be applied after my previous submission, if for
some reason the previous path is unwanted let me know and I'll resubmit
this one.

my 2 cents
E.
--
Elie De Brauwer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: killall_may_not_kill_itself.patch
Type: text/x-patch
Size: 917 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121216/8be56dee/attachment-0002.bin>
Elie De Brauwer
2012-12-16 17:12:59 UTC
Permalink
All,

In my previous patch, the pif shouldn't be stored inside the GLOBAL(),
it should be just a regular global variable. In attach a patch which
corrects this.

gr
E.
Post by Elie De Brauwer
Hi all,
The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this rule, with
the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
Note: this patch should be applied after my previous submission, if for
some reason the previous path is unwanted let me know and I'll resubmit
this one.
my 2 cents
E.
--
Elie De Brauwer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: killall_may_not_kill_itself_take2.patch
Type: text/x-patch
Size: 901 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121216/29541f5c/attachment-0002.bin>
David Seikel
2012-12-16 17:31:49 UTC
Permalink
On Sun, 16 Dec 2012 16:54:24 +0100 Elie De Brauwer
Post by Elie De Brauwer
The LSB specification of killall says "A killall process never kills
itself (but may kill other killall processes)."
The current implementation of killall does not adhere to this rule,
with the patch in attach killall will adhere to this rule.
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
Killed
edb at lapedb:~/edb-stuff/toybox/toybox$ ./toybox killall -9 toybox
killall: No such process
It's 3AM and way past my bed time, but after it's run out of things to
kill, it just kills itself. Seems pointless to not allow suicide on
something that will just suicide anyway, but I need sleep.

Like rm'ing something that does not exist, the end result is the same,
except for the useless error. The thing does not exist, killall is no
longer running. The error message is superfluous, as the result you
wanted is exactly what you get.
--
A big old stinking pile of genius that no one wants
coz there are too many silver coated monkeys in the world.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20121217/1fd9bfc6/attachment-0002.pgp>
Rob Landley
2012-12-23 21:29:40 UTC
Permalink
Post by Elie De Brauwer
Post by Rob Landley
All,
In my previous patch, the pif shouldn't be stored inside the
GLOBAL(), it
should be just a regular global variable. In attach a patch which
corrects this.
Ok, I finally took a proper look at this, and I don't understand why
can't we have pid_t in GLOBALS()? It's a standard type, it's not
being
used as an argument target (in which case it would have to be a
long),
the #includes in toys.h give us that type...
Could you explain your reasoning? (In the meantime, I've applied your
first patch.)
Mea culpa, I thought GLOBALS() was only meant for command line
parsing,
but apparently it isn't.
The first few entries in GLOBALS have to match up with the option
string's use of it (if any), but everything after that is just normal
global variables (initialized to 0).

I usually mark this by putting a blank line between the last option
slot and the first non-option global, when it's got both in there.

The advantage of GLOBALS() is the same memory gets used for all
commands (union of structures), so we're not allocating a huge bss. It
also means that if a command has to call another command it can memset
the globals block and be sure that everything starts in the same
initial state even if it's been run more than once since exec().
(Useful for toysh().)

Rob
Loading...