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

[Matthieu Longo]

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.)
> 
> Jan

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.

Matthieu