[PATCH] ld: Find files relative to the current linker script for INPUT() and GROUP()

Nick Clifton nickc@redhat.com
Mon Apr 20 10:45:49 GMT 2020


Hi Fangrui,

> 	PR ld/25806
> 	* ldlang.h (struct lang_input_statement_struct): Add extra_search_path.
> 	* ldlang.c (current_input_file): New.
> 	(ldirname): New.
> 	(new_afile): Add from_filename parameter. Set extra_search_path.
> 	(lang_add_input_file): Pass current_input_file to new_afile.
> 	(load_symbols): Set current_input_file.

Note - adding a new feature like this definitely needs some documentation
added to ld/ld.texi, an entry in ld/NEWS and ideally one or more new tests
added to ld/testsuite/ld-scripts.


> diff --git a/ld/ChangeLog b/ld/ChangeLog
> index dffd363494..d03acbbc28 100644

FYI - there is no need to include the ChangeLog as part of the context
diff.  Providing it as plain text, as you have done above, is sufficient.
Indeed having a diff to a changelog entry often makes the patch fail to
apply cleanly as the changelogs do get updated quite often.


> +      /* If extra_search_path is set, entry->filename is a relative path.
> +         Search the directory of the current linkerscript script before
> +         searching other paths. */

s/linkerscript/linker script/


> +      if (entry->extra_search_path)
> +        {
> +          char *path = concat (entry->extra_search_path, slash, entry->filename,
> +                               (const char *)0);

I prefer NULL to (const char *)0 myself.


> +          if (ldfile_try_open_bfd (path, entry)) {

Formatting: Please put the { on a separate line.


> +            entry->filename = path;
> +            entry->flags.search_dirs = FALSE;
> +            return;
> +          }There is a memory leak here - path is not freed.
> +        }



>  	 within the sysroot subdirectory.)  */
>        unsigned int outer_sysrooted = input_flags.sysrooted;
>        input_flags.sysrooted = 0;

This code fragment made me wonder.  Have you tested your patch
in a sys-rooted environment ?  Does it still work ?

Plus that reminds me - which target(s) have you used to check this patch ?

Cheers
  Nick



More information about the Binutils mailing list