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

[Jan Beulich]

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