Re: CVS commit: src/usr.bin/pkill
> cvs rdiff -u -r1.25 -r1.26 src/usr.bin/pkill/pkill.c Looks like -l option is necessary for prenice also. enami.
Re: CVS commit: src/usr.bin/pkill
On Tue, Nov 20, 2012 at 05:52:02PM -0500, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Tue Nov 20 22:52:02 UTC 2012 > > Modified Files: > src/usr.bin/pkill: pkill.c > > Log Message: > Don't use p_comm since it is only 16 characters long and you can find the > full argv[0]. It is just confusing to have a long command name, that ps > shows as the long command name, and then when you try to kill it using > the full command name as displayed you don't get a match. While there > fix a format nit, and remove the main() declaration. This might break some scripts. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/usr.bin/pkill
In article <20121121080326.gl...@snowdrop.l8s.co.uk>, David Laight wrote: >On Tue, Nov 20, 2012 at 05:52:02PM -0500, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Tue Nov 20 22:52:02 UTC 2012 >> >> Modified Files: >> src/usr.bin/pkill: pkill.c >> >> Log Message: >> Don't use p_comm since it is only 16 characters long and you can find the >> full argv[0]. It is just confusing to have a long command name, that ps >> shows as the long command name, and then when you try to kill it using >> the full command name as displayed you don't get a match. While there >> fix a format nit, and remove the main() declaration. > >This might break some scripts. Well, I could put backwards compatibility, but it is more dangerous to do so in the long term. Kids: don't use pkill(1) in scripts. christos
re: CVS commit: src/usr.bin/pkill
> In article <20121121080326.gl...@snowdrop.l8s.co.uk>, > David Laight wrote: > >On Tue, Nov 20, 2012 at 05:52:02PM -0500, Christos Zoulas wrote: > >> Module Name: src > >> Committed By: christos > >> Date: Tue Nov 20 22:52:02 UTC 2012 > >> > >> Modified Files: > >>src/usr.bin/pkill: pkill.c > >> > >> Log Message: > >> Don't use p_comm since it is only 16 characters long and you can find the > >> full argv[0]. It is just confusing to have a long command name, that ps > >> shows as the long command name, and then when you try to kill it using > >> the full command name as displayed you don't get a match. While there > >> fix a format nit, and remove the main() declaration. > > > >This might break some scripts. > > Well, I could put backwards compatibility, but it is more dangerous to > do so in the long term. Kids: don't use pkill(1) in scripts. this seems like a fairly big semantic change to me. could you change it to take a new option to look in argv[0] instead of p_comm? p_comm is not changeable by the user. .mrg.
re: CVS commit: src/usr.bin/pkill
On Nov 22, 4:39am, m...@eterna.com.au (matthew green) wrote: -- Subject: re: CVS commit: src/usr.bin/pkill | this seems like a fairly big semantic change to me. | | could you change it to take a new option to look in argv[0] instead | of p_comm? p_comm is not changeable by the user. You could already do this with -f. The point was to make the default case behave intuitively. Adding an option is against that. I could add backwards compatibility by checking both, but I think that in the long term this will be dangerous and confusing (since you might end up killing more than you thought you would...) christos
Re: CVS commit: src/usr.bin/pkill
On Wed, Nov 21, 2012 at 12:43:20PM -0500, Christos Zoulas wrote: > On Nov 22, 4:39am, m...@eterna.com.au (matthew green) wrote: > -- Subject: re: CVS commit: src/usr.bin/pkill > > | this seems like a fairly big semantic change to me. > | > | could you change it to take a new option to look in argv[0] instead > | of p_comm? p_comm is not changeable by the user. > > You could already do this with -f. The point was to make the default case > behave intuitively. Adding an option is against that. I could add backwards > compatibility by checking both, but I think that in the long term this will > be dangerous and confusing (since you might end up killing more than you > thought you would...) That (killing too much) is always true when you use something like pkill(). At least it is better that the 'ps | grep' I've seen many people do! Is pkill() guaranteed to do an atomic traversal of the process list? So it will kill something that keeps using fork() to change its pid. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/usr.bin/pkill
In article <20121121200309.gn...@snowdrop.l8s.co.uk>, David Laight wrote: >On Wed, Nov 21, 2012 at 12:43:20PM -0500, Christos Zoulas wrote: >> On Nov 22, 4:39am, m...@eterna.com.au (matthew green) wrote: >> -- Subject: re: CVS commit: src/usr.bin/pkill >> >> | this seems like a fairly big semantic change to me. >> | >> | could you change it to take a new option to look in argv[0] instead >> | of p_comm? p_comm is not changeable by the user. >> >> You could already do this with -f. The point was to make the default case >> behave intuitively. Adding an option is against that. I could add backwards >> compatibility by checking both, but I think that in the long term this will >> be dangerous and confusing (since you might end up killing more than you >> thought you would...) > >That (killing too much) is always true when you use something like pkill(). >At least it is better that the 'ps | grep' I've seen many people do! Yes, this is why it is best to not use things that kill by name from scripts. >Is pkill() guaranteed to do an atomic traversal of the process list? >So it will kill something that keeps using fork() to change its pid. I don't think our libkvm offers that. christos
Re: CVS commit: src/usr.bin/pkill
On Nov 21, 2012, at 1:25 PM, Christos Zoulas wrote: > In article <20121121200309.gn...@snowdrop.l8s.co.uk>, > David Laight wrote: >> On Wed, Nov 21, 2012 at 12:43:20PM -0500, Christos Zoulas wrote: >>> On Nov 22, 4:39am, m...@eterna.com.au (matthew green) wrote: >>> -- Subject: re: CVS commit: src/usr.bin/pkill >>> >>> | this seems like a fairly big semantic change to me. >>> | >>> | could you change it to take a new option to look in argv[0] instead >>> | of p_comm? p_comm is not changeable by the user. >>> >>> You could already do this with -f. The point was to make the default case >>> behave intuitively. Adding an option is against that. I could add backwards >>> compatibility by checking both, but I think that in the long term this will >>> be dangerous and confusing (since you might end up killing more than you >>> thought you would...) >> >> That (killing too much) is always true when you use something like pkill(). >> At least it is better that the 'ps | grep' I've seen many people do! > > Yes, this is why it is best to not use things that kill by name from scripts. > >> Is pkill() guaranteed to do an atomic traversal of the process list? >> So it will kill something that keeps using fork() to change its pid. > > I don't think our libkvm offers that. How could you guarantee this short of moving this into the kernel so you could do all the comparisons while keeping all forks from happening? Warner
Re: CVS commit: src/usr.bin/pkill
On Wed, Nov 21, 2012 at 01:36:01PM -0700, Warner Losh wrote: > >> Is pkill() guaranteed to do an atomic traversal of the process list? > >> So it will kill something that keeps using fork() to change its pid. > > > > I don't think our libkvm offers that. > > How could you guarantee this short of moving this into the kernel > so you could do all the comparisons while keeping all forks from happening? You probably can't ... Even in the kernel it might be 'interesting'. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/usr.bin/pkill
On Nov 21, 2012, at 1:55 PM, David Laight wrote: > On Wed, Nov 21, 2012 at 01:36:01PM -0700, Warner Losh wrote: Is pkill() guaranteed to do an atomic traversal of the process list? So it will kill something that keeps using fork() to change its pid. >>> >>> I don't think our libkvm offers that. >> >> How could you guarantee this short of moving this into the kernel >> so you could do all the comparisons while keeping all forks from happening? > > You probably can't ... > Even in the kernel it might be 'interesting'. Agreed. It was a leading question. Even with help from the kernel, it would be difficult Warner
Re: CVS commit: src/usr.bin/pkill
On Nov 21, 1:36pm, i...@bsdimp.com (Warner Losh) wrote: -- Subject: Re: CVS commit: src/usr.bin/pkill | How could you guarantee this short of moving this into the kernel so you = | could do all the comparisons while keeping all forks from happening? You could not; this was more tongue-in-cheek. christos
re: CVS commit: src/usr.bin/pkill
> | this seems like a fairly big semantic change to me. > | > | could you change it to take a new option to look in argv[0] instead > | of p_comm? p_comm is not changeable by the user. > > You could already do this with -f. The point was to make the default case > behave intuitively. Adding an option is against that. I could add backwards > compatibility by checking both, but I think that in the long term this will > be dangerous and confusing (since you might end up killing more than you > thought you would...) but again, it's a semantic change. the old system was a constant identifier that never changed over the life of the process (it may not be unique.) the argument list is under the control of the process itself, so what's being matched here is a different thing. i guess i've known about the 16-byte limitation and understood it, but since it was p_comm it was a reasonable limit. now there is no way to match against p_comm at all, right? .mrg.
Re: CVS commit: src/usr.bin/pkill
In article <20404.1353628...@splode.eterna.com.au>, matthew green wrote: > >> | this seems like a fairly big semantic change to me. >> | >> | could you change it to take a new option to look in argv[0] instead >> | of p_comm? p_comm is not changeable by the user. >> >> You could already do this with -f. The point was to make the default case >> behave intuitively. Adding an option is against that. I could add backwards >> compatibility by checking both, but I think that in the long term this will >> be dangerous and confusing (since you might end up killing more than you >> thought you would...) > >but again, it's a semantic change. > >the old system was a constant identifier that never changed over >the life of the process (it may not be unique.) the argument list >is under the control of the process itself, so what's being matched >here is a different thing. > >i guess i've known about the 16-byte limitation and understood it, >but since it was p_comm it was a reasonable limit. now there is >no way to match against p_comm at all, right? No there is not; and should we add it, we should consider truncating the provided string to 16 characters... christos
