[PATCH v5 13/20] Merge of Object Attributes v2 during linkage (generic logic)

[Matthieu Longo]

On 2025-07-10 09:14, Jan Beulich wrote:
> On 09.07.2025 18:47, Matthieu Longo wrote:
>> On 2025-07-09 15:29, Richard Earnshaw (lists) wrote:
>>> On 07/07/2025 17:49, Matthieu Longo wrote:
>>>> +/* Merge two subsections together (object attributes v2 only).
>>>> +   The result is stored into subsec1.  subsec2 is destroyed.
>>>> +   Return true if the merge was successful, false otherwise.
>>>> +   Note: subsec1 and subsec2 are expected to be sorted before the call to this
>>>> +   function.  */
>>>> +static bool
>>>> +oav2_subsection_destructive_merge (bfd *abfd,
>>>> +				   obj_attr_subsection_v2 *subsec1,
>>>> +				   obj_attr_subsection_v2 *subsec2)
>>>> +{
>>>> +  BFD_ASSERT (subsec1->encoding == subsec2->encoding
>>>> +	   && subsec1->optional == subsec2->optional);
>>>> +
>>>> +  bool success = true;
>>>> +
>>>> +  success &= oav2_compact_tags (abfd, subsec1);
>>>> +  success &= oav2_compact_tags (abfd, subsec2);
>>>> +
>>>> +  obj_attr_v2 *a1 = subsec1->first_;
>>>> +  obj_attr_v2 *a2 = subsec2->first_;
>>>> +  while (a1 != NULL && a2 != NULL)
>>>> +    {
>>>> +      if (a1->tag < a2->tag)
>>>> +	{} /* Nothing to do, a1 is already in subsec1.  */
>>>> +      else if (a1->tag > a2->tag)
>>>> +	{
>>>> +	  /* a2 is missing in subsec1, add it.  */
>>>> +	  obj_attr_v2 *previous = LINKED_LIST_REMOVE(obj_attr_v2) (subsec2, a2);
>>>> +	  LINKED_LIST_INSERT_BEFORE(obj_attr_v2) (subsec1, a2, a1);
>>>> +	  a2 = previous;
>>>> +	}
>>>> +      else
>>>> +	{
>>>> +	  if (subsec1->encoding == OA_ENC_ULEB128
>>>> +	   && a1->vals.uint_val != a2->vals.uint_val)
>>>> +	    {
>>>> +	      success = false;
>>>> +	      _bfd_error_handler (_("%pB: error: found 2 subsections with the "
>>>
>>> In this case I'd put the opening parenthesis on the following line, so that you
>>> don't lose too much horizontal white space when correctly indenting ...
>>
>> Well, this one I am wondering if the medicine is not worse than the disease.
>>
>> 	      _bfd_error_handler
>> 		(_("%pB: error: found 2 subsections with the same name '%s' and"
>> 		   " found conflicting values (0x%x vs 0x%x) for object "
>> 		   "attribute 'Tag_unknown_%u'"), abfd, subsec1->name,
>> 		   a1->vals.uint_val, a2->vals.uint_val, a1->tag);
>>
>> I lost one line and more horizontal space, and I don't find that the
>> indentation is better than before but rather worse.
> 
> In addition to what Richard said: The longer a statement gets, the more
> important it is for it to be reasonably easy to spot, at any position,
> how many pending open parentheses there are.
> 
> Another nit here: Please generally prefer %#x and alike over 0x%x.
> 
> Jan

The code formatting and the integer formatting using %#x is fixed in the 
next revision.

Matthieu