On Tue, 1 Jul 2014 17:41:12 +0200
Mateusz Guzik <mjgu...@gmail.com> wrote:
> On Tue, Jul 01, 2014 at 05:05:24PM +0200, Gary Jennejohn wrote:
> > On Tue, 1 Jul 2014 16:19:20 +0200
> > Mateusz Guzik <mjgu...@gmail.com> wrote:
> >
> > > On Tue, Jul 01, 2014 at 09:40:57AM -0400, John Baldwin wrote:
> > > > On Tuesday, July 01, 2014 2:29:15 am Mateusz Guzik wrote:
> > > > > Modified: head/sys/kern/kern_sig.c
> > > > >
> > > > ==============================================================================
> > > > > --- head/sys/kern/kern_sig.c Tue Jul 1 06:23:48 2014
> > > > > (r268073)
> > > > > +++ head/sys/kern/kern_sig.c Tue Jul 1 06:29:15 2014
> > > > > (r268074)
> > > > > @@ -3453,10 +3453,6 @@ sigacts_copy(struct sigacts *dest, struc
> > > > > int
> > > > > sigacts_shared(struct sigacts *ps)
> > > > > {
> > > > > - int shared;
> > > > >
> > > > > - mtx_lock(&ps->ps_mtx);
> > > > > - shared = ps->ps_refcnt > 1;
> > > > > - mtx_unlock(&ps->ps_mtx);
> > > > > - return (shared);
> > > > > + return (ps->ps_refcnt > 1);
> > > > > }
> > > >
> > > > You should KASSERT() in sigacts_shared that P_HADTHREADS is not set so
> > > > that
> > > > new code does not call this function unsafely in the future.
> > > >
> > >
> > > Looks more like !P_HADTHREADS || p->p_singlethread != NULL on the first
> > > sight, but yeah, I'll add an assertion.
> > >
> >
> > Also, please fix the build with gcc, which dies with an error that
> > newsigacts may be use unitialzed.
> >
> > What I did was set both oldsigacts and newsigacts to NULL before this
> > line at around 622:
> >
> > if (sigacts_share(p->p_sigacts)) {
> >
> > which allowed me to remove the else branch for setting oldsigacts to
> > NULL.
> >
>
> Sorry for breakage.
>
> Does the following fix the problem for you?
>
> diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c
> index 91d161d..dc4eb9b 100644
> --- a/sys/kern/kern_exec.c
> +++ b/sys/kern/kern_exec.c
> @@ -625,8 +625,10 @@ interpret:
> oldsigacts = p->p_sigacts;
> newsigacts = sigacts_alloc();
> sigacts_copy(newsigacts, oldsigacts);
> - } else
> + } else {
> oldsigacts = NULL;
> + newsigacts = NULL; /* satisfy gcc */
> + }
>
> PROC_LOCK(p);
> if (oldsigacts)
>
Yes, this also works.
--
Gary Jennejohn
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
