[PATCH v5 05/20] gas: implement parsing of object attributes v2

[Matthieu Longo]

On 2025-07-10 10:57, Richard Earnshaw (lists) wrote:
> On 09/07/2025 16:05, Matthieu Longo wrote:
>> On 2025-07-09 14:55, Richard Earnshaw (lists) wrote:
>>> On 07/07/2025 17:49, Matthieu Longo wrote:
>>>> diff --git a/bfd/elf-attrs.c b/bfd/elf-attrs.c
>>>> index 1ccbcc40d4d..acf93a3cee5 100644
>>>> --- a/bfd/elf-attrs.c
>>>> +++ b/bfd/elf-attrs.c
>>>> +
>>>> +/* Return True if the given subsection name is part of the reserved "gnu-testing"
>>>> +   namespace.  */
>>>> +static bool
>>>> +gnu_testing_namespace (const char *subsec_name)
>>>> +{
>>>> +  return strncmp ("gnu-testing", subsec_name, 11) == 0;
>>>
>>> What's the reason for using strncmp here rather than strcmp?
>>>
>>
>> The subsection name can be longer than "gnu-testing" (see example in the tests), so we need to compare at most len("gnu-testing"), i.e. 11 bytes.
>> In my understanding, you cannot do this with strcmp.
> 
> No, you can't but this would match a section called "gnu-testing-for-christmas", which doesn't really seem correct: it's not the gnu-testing subsection.  If subsec_name isn't null terminated, then it would be reasonable to use strncmp, but then you'd want to pass the length limit as an argument to this function.
> 
> 
> But maybe I've misunderstood how the naming scheme works here.  Perhaps this *is* supposed to be a prefix to longer strings.  But in that case:
> 1) I think the comment could be a bit clearer.
> 2) I think you should check that subsec_name[11] is not '\0', since then the string is not a prefix.
> 
> 
> R.

Yes, this is supposed to be a prefix to longer strings, but at the same 
time, "gnu-testing\0" is acceptable too if for the test case, one 
gnu-testing subsection is enough, and people don't care about a merge 
policy (i.e. no linker test).

There is already a comment about this at the top of the file, and in the 
commit message:

In order to test this generic logic, AArch64's use cases are not 
offering enough coverage, so a "GNU testing namespace" which corresponds 
to the name of the subsection was introduced. It follows the following 
pattern:
      gnu-testing-<XXXXXX>-MERGE-<POLICY>

- <XXXXXX>: an arbitrary name for your testing subsection.
- <POLICY>: the name of the merging policy to apply on the values in the 
subsection. The currently supported merge policy are:
          * -MERGE-AND: bitwise AND applied on numerical values.
          * -MERGE-OR: bitwise OR applied on numerical values.
          * -MERGE-ADD: concatenates strings together with a '+' in-between.
          Note: "-MERGE-ADD" does not make really sense, and will very 
likely never be used for a real merge. Its only purpose is to test the 
correct handling of merges with strings.

Matthieu