[PATCH v2] bfd: fill in PE load config directory entry.

[Jeremy Drake]

On Tue, 11 Mar 2025, Jan Beulich wrote:

> I understand the same is used when looking up _tls_used, yet it feels wrong
> to assume that bfd_get_symbol_leading_char() can only ever return nil or '_'.
> Nevertheless, because of the pre-existing similar code, I'm not going to
> insist that this be corrected right here.

OK.  It sounds like LIU Hao had some ideas in that department, anyway,
presumably for future cleanup

> > +	{
> > +	  _bfd_error_handler
> > +	    (_("%pB: unable to fill in DataDictionary[10] because __load_config_used is missing"),
> > +	     abfd);
>
> Message text here suggests this is rather (or also?) meant to be in the outer
> if()'s "else", i.e.
>
> > +	  result = false;
> > +	}
> > +    }
>
> .... here. Perhaps things want adjusting such that the message is issued in
> both cases, e.g.
>
>   h1 = coff_link_hash_lookup (coff_hash_table (info),
> 			      (bfd_get_symbol_leading_char (abfd) != 0
> 			       ? "__load_config_used" : "_load_config_used"),
> 			      false, false, true);
>   if (h1 != NULL
>       && ((h1->root.type != bfd_link_hash_defined
> 	   && h1->root.type != bfd_link_hash_defweak)
> 	  || h1->root.u.def.section == NULL
> 	  || h1->root.u.def.section->output_section == NULL))
>     h1 = NULL;
>   if (h1 != NULL)
>     {
>       ...
>
> ?


I'm sorry, I was having a hard time understanding what you were
suggesting.  I *think* a light bulb just went on, that you are suggesting
issuing an error if h1 is NULL (the outer if).  This is not what I
intended: it is not an error if there is no symbol _load_config_used, it
should just not fill in the load config directory.  However, if the symbol
does exist, it should be of the correct type and have sufficient data
available at it to read the size.  Does this make sense, or am I
misunderstanding either the code or your feedback?