Re: kern_sig.c
Date: Sun, 22 Nov 2015 18:46:42 + From: Taylor R Campbell If someone wanted to explain it all to me in the form of a long and rambly email, which should be easily written by someone familiar with the code, I would volunteer to turn that into an organized man page. Evidently we already have a signal(9) man page, so never mind. (But maybe it could use some work.)
Re: kern_sig.c
On November 21, 2015 6:13:58 AM EST, Masao Uebayashi wrote: >I think that renaming things a bit would help people to understand the >code. ... >- postsig() -> sendsig() > >This is so badly named and incredibly confusing, as these is a >function called sigpost() which is completely different. > >sigpost() posts a signal to a signal queue. sigpost() can be called >from anywhere including interrupt context, because all it does is to > >sendsig(), the function which is called as postsig() now, and referred >to by the signal(9) manual page and comments all over the tree, is the >sequence of code that is called when a signal is delivered (some >actual action is taking place). Er... to me that seems to make things *more* confusing. My initial instinct would be that "sendsig()" is the thing you use to initiate a signal. If postsig() happens to actually get renamed, I would suggest "deliver_signal_to_process()" or a suitable abbreviation, but given the history of the current names perhaps just better docs is sufficient. Eric
Re: kern_sig.c
Date: Sun, 22 Nov 2015 20:36:26 +1100 from: matthew green > Renaming things is fine, but please try not to break it :-) i'm not sure i agree with any of the proposals here for renaming. these are well entrenched names and concepts here and i don't think we should rename them without significant consideration. i've never had problems with this code because of the terms, and if they all change away from what history and others use, i don't see what the benefit is. Maybe Someone^TM could write a man page explaining the signal delivery mechanism, including a flow diagram of all these functions and descriptions of their respective contracts (as they are today, not as they were in 4.4BSD). If someone wanted to explain it all to me in the form of a long and rambly email, which should be easily written by someone familiar with the code, I would volunteer to turn that into an organized man page.
Re: kern_sig.c
On Sun, Nov 22, 2015 at 20:36:26 +1100, matthew green wrote: > > Renaming things is fine, but please try not to break it :-) > > i'm not sure i agree with any of the proposals here for renaming. > > these are well entrenched names and concepts here and i don't think > we should rename them without significant consideration. i've never > had problems with this code because of the terms, and if they all > change away from what history and others use, i don't see what the > benefit is. I agree. I know the itch to clarify things with proper names, but in my experience more often than not it's not a good idea. It's not uncommon to realize at some point in the rototill that the new set of names has its own problems; except they are sort of obvious for you since you've just been through all this code, so you consider them minor warts in what is otherwise an improvement. But for someone else that original set of warts and your new set of warts are not that different in their wartiness, except that all existing history that uses old names has been obfuscated by the renaming. Spare a thought for all those well meaning English spelling reforms... -uwe
re: kern_sig.c
> Renaming things is fine, but please try not to break it :-) i'm not sure i agree with any of the proposals here for renaming. these are well entrenched names and concepts here and i don't think we should rename them without significant consideration. i've never had problems with this code because of the terms, and if they all change away from what history and others use, i don't see what the benefit is. .mrg.
Re: kern_sig.c
In article , Masao Uebayashi wrote: >On Sat, Nov 21, 2015 at 8:21 PM, Masao Uebayashi wrote: >> On Sat, Nov 21, 2015 at 8:13 PM, Masao Uebayashi wrote: >>> I'm too young to understand how signal works in kernel. But I guess >>> I'm not alone. >>> >>> I think that renaming things a bit would help people to understand the code. >>> >>> * >>> - sendsig() -> netbsd_sendsig() >>> - trapsignal() -> netbsd_trapsignal() >>> >>> These are native emul functions of e_sendsig and e_trapsignal respectively. >>> >>> * >>> >>> - postsig() -> sendsig() >>> >>> This is so badly named and incredibly confusing, as these is a >>> function called sigpost() which is completely different. >>> >>> sigpost() posts a signal to a signal queue. sigpost() can be called >>> from anywhere including interrupt context, because all it does is to > >... put a pending signal onto the target's queue. > >- kpsignal2() -> kpsignal() > >The code in kpsignal() filling ksi_fd should belong to >kern_filedesc.c; callers are responsible to fill ksi before calling >kpsignal(). Then kpsignal2() can happily declare it as "kpsignal()". >signal(9) has to follow too. > >(hi xtos) Hey, I was trying to minimize code changes... :-) Renaming things is fine, but please try not to break it :-) christos
Re: kern_sig.c
On Sat, Nov 21, 2015 at 8:21 PM, Masao Uebayashi wrote: > On Sat, Nov 21, 2015 at 8:13 PM, Masao Uebayashi wrote: >> I'm too young to understand how signal works in kernel. But I guess >> I'm not alone. >> >> I think that renaming things a bit would help people to understand the code. >> >> * >> - sendsig() -> netbsd_sendsig() >> - trapsignal() -> netbsd_trapsignal() >> >> These are native emul functions of e_sendsig and e_trapsignal respectively. >> >> * >> >> - postsig() -> sendsig() >> >> This is so badly named and incredibly confusing, as these is a >> function called sigpost() which is completely different. >> >> sigpost() posts a signal to a signal queue. sigpost() can be called >> from anywhere including interrupt context, because all it does is to ... put a pending signal onto the target's queue. - kpsignal2() -> kpsignal() The code in kpsignal() filling ksi_fd should belong to kern_filedesc.c; callers are responsible to fill ksi before calling kpsignal(). Then kpsignal2() can happily declare it as "kpsignal()". signal(9) has to follow too. (hi xtos)
Re: kern_sig.c
On Sat, Nov 21, 2015 at 8:13 PM, Masao Uebayashi wrote:
> I'm too young to understand how signal works in kernel. But I guess
> I'm not alone.
>
> I think that renaming things a bit would help people to understand the code.
>
> *
> - sendsig() -> netbsd_sendsig()
> - trapsignal() -> netbsd_trapsignal()
>
> These are native emul functions of e_sendsig and e_trapsignal respectively.
>
> *
>
> - postsig() -> sendsig()
>
> This is so badly named and incredibly confusing, as these is a
> function called sigpost() which is completely different.
>
> sigpost() posts a signal to a signal queue. sigpost() can be called
> from anywhere including interrupt context, because all it does is to
>
> sendsig(), the function which is called as postsig() now, and referred
> to by the signal(9) manual page and comments all over the tree, is the
> sequence of code that is called when a signal is delivered (some
> actual action is taking place).
>
> sendsig() ("postsig()") is only called from userret() -> mi_userret()
> -> lwp_userret(). If a pending signal is found and it is decided to
> be delivered, trap frame is overwritten by signal handler trampoline.
This should have been read as: trap frame is replaced with signal
handler trampoline by sendsig_siginfo().
