[PATCH v7 01/19] bfd: rename parsing methods of object attribute v1 API

[Jan Beulich]

On 15.07.2025 12:22, Matthieu Longo wrote:
> On 2025-07-15 09:44, Jan Beulich wrote:
>> On 14.07.2025 18:58, Matthieu Longo wrote:
>>> --- a/gas/config/obj-elf.c
>>> +++ b/gas/config/obj-elf.c
>>> @@ -2068,23 +2068,51 @@ skip_past_char (char ** str, char c)
>>>   /* A list of attributes that have been explicitly set by the assembly code.
>>>      VENDOR is the vendor id, BASE is the tag shifted right by the number
>>>      of bits in MASK, and bit N of MASK is set if tag BASE+N has been set.  */
>>> -struct recorded_attribute_info {
>>> -  struct recorded_attribute_info *next;
>>> -  int vendor;
>>> +typedef struct recorded_attribute_info_t {
>>
>> The bogus _t is still there.
>>
> 
> I forgot to change it in this patch. Path 2 has those issues fixed.
> Fixed in the next revision.
> 
>>> +  struct recorded_attribute_info_t *next;
>>> +  obj_attr_vendor_t vendor;
>>>     unsigned int base;
>>>     unsigned long mask;
>>> -};
>>> -static struct recorded_attribute_info *recorded_attributes;
>>> +} recorded_attribute_info_t;
>>> +static recorded_attribute_info_t *recorded_attributes;
>>> +
>>> +static void
>>> +oav1_attr_info_free (recorded_attribute_info_t *node)
>>> +{
>>> +  recorded_attribute_info_t *next;
>>> +  while (node != NULL)
>>> +    {
>>> +      next = node->next;
>>> +      free (node);
>>> +      node = next;
>>> +    }
>>> +}
>>
>> Didn't you agree to limit the scope of "next"?
>>
>> Yes, both comments were given in reply to patch 2, but they really apply
>> already here.
>>
> 
> I forgot to change it in this patch. Path 2 has those issues fixed.
> Fixed in the next revision.
> 
>>> --- a/gas/config/tc-arm.c
>>> +++ b/gas/config/tc-arm.c
>>> @@ -4971,9 +4971,9 @@ s_arm_unwind_raw (int ignored ATTRIBUTE_UNUSED)
>>>   static void
>>>   s_arm_eabi_attribute (int ignored ATTRIBUTE_UNUSED)
>>>   {
>>> -  int tag = obj_elf_vendor_attribute (OBJ_ATTR_PROC);
>>> +  obj_attr_tag_t tag = obj_attr_v1_process_attribute (OBJ_ATTR_PROC);
>>>   
>>> -  if (tag >= 0 && tag < NUM_KNOWN_OBJ_ATTRIBUTES)
>>> +  if (tag < NUM_KNOWN_OBJ_ATTRIBUTES)
>>
>> Is this a safe change to make? I don't think we force the compiler to
>> represent enumerator values as unsigned? (I can't even find a gcc option
>> doing so, and even if there was one there would still be the question of
>> other compilers, most notably Clang.)
> 
> obj_attr_tag_t is not an enum.
> 
> This is the definition in bfd/elf-attrs.h:
> typedef uint32_t obj_attr_tag_t;
> 
> Hence my change to remove "tag >= 0" from the test.

Oh, sorry for not looking closely enough. With the other two adjustments done
the patch is okay then.

Jan