Re: [PATCH] Save final 'error' in __nptl_setxid_error()

[Peter Zelezny <peter dot zelezny at dektech dot com dot au>]

On 20/09/17 10:46, Peter Zelezny wrote:

> On 19/09/17 19:43, Florian Weimer wrote:
>
> > On 09/19/2017 09:55 AM, Peter Zelezny wrote:
> > > -      if (olderror != -1)
> > > +      if (olderror != -1) {
> > > +    /* save error to memory so it's not lost in coredumps. */
> > > +    cmdp->error = error;
> > >       /* Mismatch between current and previous results.  */
> > >       abort ();
> > > +    }
> >
> > Would you please change the formatting so that the { } are on lines 
> > by themselves (see the surrounding code for examples)?
> >
> > I think it's better to store the error code in a new local volatile 
> > int variable, so that both old and new error code are accessible.  
> > The old error code seems quite valuable, too.
> >
> > Thanks,
> > Florian
>
> I hope I got the indentation right now, copying nearby code (one tab 
> and two spaces).
>
> Yes, perhaps just saving this to a local stack int would be better, 
> like this?
>
> Cheers,
> -Peter.

Any other opinions or concerns on this patch?

Cheers,
-Peter.

--- glibc.orig/nptl/allocatestack.c	2017-09-19 12:58:11.456588117 +1000
+++ glibc/nptl/allocatestack.c	2017-09-19 12:59:43.906355264 +1000
@@ -1084,8 +1084,12 @@ __nptl_setxid_error (struct xid_command
       if (olderror == error)
 	break;
       if (olderror != -1)
-	/* Mismatch between current and previous results.  */
-	abort ();
+	{
+	  /* save 'error' to memory so it's not lost in coredumps.  */
+	  volatile int xid_err __attribute__((unused)) = error;
+	  /* Mismatch between current and previous results.  */
+	  abort ();
+	}
     }
   while (atomic_compare_and_exchange_bool_acq (&cmdp->error, error, -1));
 }