This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v3] Extra dlopen/getpagesize static executable tests


On Thu, 27 Jun 2013, Roland McGrath wrote:

> >  Mapping with RTLD_GLOBAL relies on the fix to be in place.
> 
> If this intended to be a test for RTLD_GLOBAL crashing, then it should at
> least have a comment saying that's the reason to use the flag.  But IMHO
> using RTLD_GLOBAL in a test should be confined to tests that actually
> verify the RTLD_GLOBAL functionality in some fashion.
> 
> > > I don't understand why this is using RTLD_GLOBAL.
> > > That does not seem to be material to this test.
> > 
> >  This is an artefact from the original use of this test.  I've switched it 
> > to RTLD_LOCAL now, though frankly I don't think any choice made between 
> > the two really matters for the scope of the test.  The use of RTLD_LOCAL 
> > avoids BZ #15022 however.
> 
> Use of RTLD_GLOBAL is rare and strange.  Nobody should ever use it without
> a clear reason.  Using it always expands the scope of the test to include
> the RTLD_GLOBAL implementation.

 Understood.  Thanks for your insights.

> >  I have adjusted the comments and mentally noted the special case (hoping 
> > that it sticks).  And after a bit of thinking I have concluded tststatic5 
> > doesn't really bring anything new here, so I have discarded it altogether.
> 
> You say this, but this patch adds tststatic5.  So I don't know what you mean.

 Off-by-one error, sorry about this.  s/tststatic5/tststatic6/ here...

> > Given that tststatic4 does not trip on BZ #15022 anymore, this change no 
> > longer has a reason to refer to it.
> 
> That makes sense.

 ... and s/tststatic4/tststatic5/ here.

> >  OK to apply?
> 
> Looks OK to me.

 I have committed it now, thanks for your review.

  Maciej


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]