This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3] Extra dlopen/getpagesize static executable tests
- From: Roland McGrath <roland at hack dot frob dot com>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: <libc-alpha at sourceware dot org>
- Date: Thu, 27 Jun 2013 14:43:07 -0700 (PDT)
- Subject: Re: [PATCH v3] Extra dlopen/getpagesize static executable tests
- References: <alpine dot DEB dot 1 dot 10 dot 1301152056590 dot 4834 at tp dot orcam dot me dot uk> <20130116215545 dot 7A37A2C0B0 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1301240655220 dot 4834 at tp dot orcam dot me dot uk> <20130531200059 dot C94C02C077 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1306140202520 dot 16287 at tp dot orcam dot me dot uk> <20130620000549 dot 416D12C111 at topped-with-meat dot com> <alpine dot DEB dot 1 dot 10 dot 1306271748120 dot 16287 at tp dot orcam dot me dot uk>
> 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.
> 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.
> Given that tststatic4 does not trip on BZ #15022 anymore, this change no
> longer has a reason to refer to it.
That makes sense.
> OK to apply?
Looks OK to me.
Thanks,
Roland