Re: [PATCH 3/5][v2][BZ #15022] Correct global-scope dlopen issues in static executables

[Roland McGrath <roland at hack dot frob dot com>]

> > It would be even better if as much of this as possible could be done with
> > static initialization rather than code that runs at startup.  Can you try
> > to do it that way (in elf/dl-support.c) and see if it's not desperately
> > difficult?
> 
>  How about all static then?

That's ideal.

> +  /* Try to map a module into the global scope.  */
> +  handle = dlopen ("modstatic3.so", RTLD_LAZY | RTLD_GLOBAL);

I guess this is testing just that having RTLD_GLOBAL there doesn't make it
crash?  There is nothing in the test that actually makes using RTLD_GLOBAL
worthwhile, so the comment should say clearly why it's using the flag.

> +/* Global-scope DSO tests with a static executable.

This is the same comment as the first test.  Is there something concise you
can say here that distinguishes why there are two tests?  If it's just sort
of a basic test and an exhaustive test, I guess just adding ", more cases"
or something would be fine.

> +  /* Try to map itself into the global scope.  */
> +  initial_handle = dlopen (NULL, RTLD_LAZY | RTLD_GLOBAL);

RTLD_GLOBAL isn't really meaningful here, is it?
The main program pseudo-object is the basis of the global scope.

> +      printf ("*foop [initial]: got 0x%x, expected 0x%x\n", foo, MAGIC0);

Always use %#x in preference to 0x%x.

> +    .l_libname = &(struct libname_list) { .name = "", .dont_free = 1 },

I didn't realize &compound_literal was kosher.  Nice!

> +#if NO_TLS_OFFSET != 0
> +    .l_tls_offset = NO_TLS_OFFSET,
> +#endif

Drop the #if.  An explicit initializer to zero has no cost.

Aside from that one nit, the implementation itself looks great.  I'll trust
you to do appropriate commenting and clean-up on the tests without another
round of review.


Thanks,
Roland