[PATCH 4/8] BFD: Add BFD pointer member to `struct carsym'

[Jan Beulich]

On 24.10.2025 15:37, Maciej W. Rozycki wrote:
> On Mon, 20 Oct 2025, Jan Beulich wrote:
> 
>>>>> +.{* Holds a file position or bfd* depending on context.  *}
>>>>> +.typedef union file_ptr_or_bfd
>>>>> +.{
>>>>> +.  file_ptr file_offset;
>>>>> +.  bfd *abfd;
>>>>> +.}
>>>>> +.file_ptr_or_bfd;
>>>>
>>>> The comment is too little for my taste, I'm afraid. How would one know which
>>>> of the two it is, merely seeing an instance of this type?
>>>
>>>  It's up to the user of this data structure to decide, just as the comment 
>>> says.  Can you please elaborate what kind of description you'd like to see 
>>> here?
>>
>> Hard to make a suggestion without knowing what, in at least an example
>> case, the approach to distinguish is. To take an abstract example: There
>> may be callback functions which want to take either kind of context data.
>> There it would be clear that the "scope" is strictly that between the
>> site setting up the data and the callback function. Since here you embed
>> the new type in another struct, it's less (if at all) clear.
> 
>  Please let's not invent hypothetical use cases.  This is a generic data 
> type and it doesn't set the usage policy.  It's up to the code writer to 
> use this data type in a reasonable manner and you can't figure out which 
> of the two members to use from the data type alone.  It's not uncommon for 
> code to use such data types.  The comment is as accurate as it can be in 
> my opinion.
> 
>  I guess I could rewrite the comment to say:
> 
> "Holds an archive member reference via a file position or bfd* depending 
> on context."
> 
> but it wouldn't fulfil your criteria and would dictate usage unecessarily, 
> so I'm skeptical as to such an "improvement".
> 
>  FWIW one use case of this data type is coming with 7/8, where the member 
> to use is determined by bfd_ardata (abfd)->symdef_use_bfd.  Another use 
> case will come with the second part of this effort.

Before replying to the above, I then will want to have actually looked at
patch 7.

Jan

>  Actually I have realised this data entity gratuitously uses a signed data 
> type for the file offset while in similar places we have an unsigned one, 
> so I'll respin with this change preceded by one taking care of signedness. 
> It might make sense to do a larger cleanup in this area, for example I've 
> identified places where ufile_ptr data is assigned to by strtol/strtoll 
> rather than strtoul/strtoull, but I won't do this on this occasion.
> 
>  Thank you for your input anyway.
> 
>   Maciej
>