Re: [PATCH 07/28] elf: Implement ld.so --help

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella via Libc-alpha:

>> @@ -98,6 +98,11 @@ call_init_paths (const struct dl_main_state *state)
>>  }
>>  
>>  /* Print ld.so usage information and exit.  */
>> -void _dl_usage (void) attribute_hidden __attribute__ ((__noreturn__));
>> +void _dl_usage (const char *argv0, const char *wrong_option)
>> +  attribute_hidden __attribute__ ((__noreturn__));
>> +
>> +/* Print ld.so --help output and exit.  */
>> +void _dl_help (const char *argv0, struct dl_main_state *state)
>> +  attribute_hidden __attribute__ ((__noreturn__));
>>  
>>  #endif /* _DL_MAIN */
>
> Maybe use _Noreturn here?

Fixed.

>> diff --git a/elf/dl-usage.c b/elf/dl-usage.c
>> index f3d89d22b7..72ff99e70f 100644
>> --- a/elf/dl-usage.c
>> +++ b/elf/dl-usage.c
>> @@ -19,12 +19,24 @@
>>  #include <dl-cache.h>
>>  #include <dl-main.h>
>>  #include <ldsodefs.h>
>> +#include <unistd.h>
>>  
>>  void
>> -_dl_usage (void)
>> +_dl_usage (const char *argv0, const char *wrong_option)
>>  {
>> -  _dl_fatal_printf ("\
>> -Usage: ld.so [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
>> +  if (wrong_option != NULL)
>> +    _dl_error_printf ("%s: unrecognized option '%s'\n", argv0, wrong_option);
>> +  else
>> +    _dl_error_printf ("%s: missing program name\n", argv0);
>> +  _dl_error_printf ("Try '%s --help' for more information.\n", argv0);
>> +  _exit (1);
>> +}
>> +
>
> Maybe EXIT_FAILURE here?

Fixed.

>> +void
>> +_dl_help (const char *argv0, struct dl_main_state *state)
>> +{
>> +  _dl_printf ("\
>> +Usage: %s [OPTION]... EXECUTABLE-FILE [ARGS-FOR-PROGRAM...]\n\
>>  You have invoked `ld.so', the helper program for shared library executables.\n\
>>  This program usually lives in the file `/lib/ld.so', and special directives\n\
>>  in executable files using ELF shared libraries tell the system's program\n\
>> @@ -47,5 +59,9 @@ of this helper program; chances are you did not intend to run this program.\n\
>>                          in LIST\n\
>>    --audit LIST          use objects named in LIST as auditors\n\
>>    --preload LIST        preload objects named in LIST\n\
>> -  --argv0 STRING        set argv[0] to STRING before running\n");
>> +  --argv0 STRING        set argv[0] to STRING before running\n\
>> +  --help                display this help and exit\n\
>> +",
>> +              argv0);
>> +  _exit (0);
>>  }
>
> Maybe EXIT_SUCCESS here?

Fixed.

>>        /* If we have no further argument the program was called incorrectly.
>>  	 Grant the user some education.  */
>>        if (_dl_argc < 2)
>> -	_dl_usage ();
>> +	{
>> +	  if (state.mode == rtld_help)
>> +	    /* --help without an executable is not an error.  */
>> +	    _dl_help (ld_so_name, &state);
>> +	  else
>> +	    _dl_usage (ld_so_name, NULL);
>> +	}
>>  
>
> Why open brackets here? There is no stack variable to possible escape.

They are guarded by an if statement:

      /* If we have no further argument the program was called incorrectly.
	 Grant the user some education.  */
      if (_dl_argc < 2)
	{
	  if (state.mode == rtld_mode_help)
	    /* --help without an executable is not an error.  */
	    _dl_help (ld_so_name, &state);
	  else
	    _dl_usage (ld_so_name, NULL);
	}


>> @@ -1647,6 +1681,11 @@ dl_main (const ElfW(Phdr) *phdr,
>>    audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_AUDIT);
>>    audit_list_add_dynamic_tag (&state.audit_list, main_map, DT_DEPAUDIT);
>>  
>> +  /* At this point, all data has been obtained that is included in the
>> +     --help output.  */
>> +  if (__builtin_expect (state.mode, normal) == rtld_help)
>> +    _dl_help (ld_so_name, &state);
>> +
>>    /* If we have auditing DSOs to load, do it now.  */
>>    bool need_security_init = true;
>>    if (state.audit_list.length > 0)
>> 
>
> Use __glibc_{un}likely or just remove the branch hint (I really don't think
> this matter here anyway).

Fixed.

I've pushed this with the indicated changes (and the adjustments for
renaming the enum constant to rtld_mode_help).

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill