[PATCH v2 03/10] BFD: Rename `*_set_reloc' to `*_finalize_section_relocs'

[Jan Beulich]

On 10.11.2025 16:26, Maciej W. Rozycki wrote:
> On Mon, 10 Nov 2025, Jan Beulich wrote:
> 
>>>>> --- binutils-gdb.orig/bfd/bfd-in2.h
>>>>> +++ binutils-gdb/bfd/bfd-in2.h
>>>>> @@ -2620,11 +2620,12 @@ long bfd_get_reloc_upper_bound (bfd *abf
>>>>>  long bfd_canonicalize_reloc
>>>>>     (bfd *abfd, asection *sec, arelent **loc, asymbol **syms);
>>>>>  
>>>>> -void bfd_set_reloc
>>>>> +void bfd_finalize_section_relocs
>>>>>     (bfd *abfd, asection *sec, arelent **rel, unsigned int count);
>>>>
>>>> What use is this declaration? Can't it be dropped rather than being
>>>> modified? All callers ...
>>>>
>>>>> -#define bfd_set_reloc(abfd, asect, location, count) \
>>>>> -       BFD_SEND (abfd, _bfd_set_reloc, (abfd, asect, location, count))
>>>>> +#define bfd_finalize_section_relocs(abfd, asect, location, count) \
>>>>> +       BFD_SEND (abfd, _bfd_finalize_section_relocs, \
>>>>> +		 (abfd, asect, location, count))
>>>>
>>>> ... end up using this macro anyway, afaict.
>>>
>>>  It's a mechanical change and not a code cleanup.  Please feel free to 
>>> investigate whether the prototype is needed or not and offer a separate 
>>> change if desired.  I don't think such an investigation ought to be a 
>>> prerequisite for this change to be accepted.
>>
>> Hmm, I can see your point, but then I also somewhat disagree. Yes, it's not
>> strictly a prereq. Yet at the same time, (reasonably) obvious cleanup is
>> also a good thing to happen while code is being touched anyway. Else it's
>> pretty likely to never happen.
> 
>  I've glanced over this and firstly the presence of this prototype is a 
> benign issue and secondly to remove it it's not an obvious cleanup at all.
> 
>  The prototype is generated as a side effect of parsing a comment block
> that serves as a manual entry to document this API.  While the API is 
> handled by a macro now, I think the intended data types for the macro's 
> arguments and the return type still need to be documented.  The use of a 
> macro rather than a function is an implementation detail (and frankly it 
> should possibly be converted to a static inline function indeed), and I
> find showing the interface in terms of a function prototype the right way 
> to do this.

Just one further remark here: See how converting to an inline function
(as long as you mean a static inline one) would then collide with the
prototype that's there?

>  Overall I think Jose did the right thing with the documentation part of 
> the API with his commit 2318686590bd ("bfd: new BFD target entry point 
> _bfd_set_reloc.") back in 2017.  Incidentally, the change was prompted by 
> an issue in the MIPS backend among others and I took part in that effort.  
> I completely forgot about it.
> 
>  So if you disagree with the current state, then please offer a change, 
> but I refuse to do anything about something I consider right and at worst 
> harmless, especially given the higher priority items on my plate, such as 
> a large outstanding MIPS patch review, which has been pending for much too 
> long now.  I feel like having spent too much time on this already.

I only now notice that this isn't an issue with just that function; the
same pattern occurs many more times. So while I think this wants tidying,
I now agree it needs doing elsewhere.

Jan