Re: [PATCH v4] ld: Make archive member file extension comparisons case insensitive when cross compiling too

["Martin Storsjö" <martin@martin.st>]

On Wed, 24 Aug 2022, Nick Clifton wrote:

> Hi Martin,
>
>  Sorry to be persnickety, but ...

No problem, I don't mind iterating on it to get it properly right, when 
the iteration time is reasonable :-)

>
>> +/* A case insensitive comparison, regardless of the host platform, used 
>> for
>> +   comparing file extensions. Parameter s1 points at the extension in a 
>> file
>> +   name (pointing at the starting '.'). Parameter s2 is a lower case 
>> string
>> +   without the leading '.'. */
>
> Comment formatting: two spaces before the closing */

Oh, I hadn't noticed that aspect - will fix.

>> +static int fileext_cmp (const char *s1, const char *s2)
>
> Function name formatting - name on a separate line from the return type.

Ok, sure.

> And yes, I know that the same problem exists for the is_underscoring()
> function directly before this one.  A patch to fix that formatting is
> pre-approved. :-)
>
>
>> +  if (*s1 != '.')
>> +    return 1;
>
> Strictly speaking this should be an error return value.  Maybe return
> INT_MAX ?  Or generate an error message and then return 1.  This test
> will probably never trigger however, so just returning 1 is OK really.
> In fact just ignore me on this one...

Ok, keeping this as is. Yes, it'd be unexpected if this isn't true, but 
from the point of view of the comparison, it's at least not a match for 
the extension.

>
>> +      int c2 = *s2++; /* Assumed to be lower case from the caller. */
>
> Assumptions are bad.  But testing this assumption does lead to needless
> performance penalties.  So ignore me on this one too.
>
> But please do fix up the comment formatting.

Ok.

>
>> diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
>
> Similar comment apply here, of course.
>
>
> Hmm.  We really ought to move the code duplicated in pe.em and pep.em into a
> single file...

Yep, totally. Unfortunately that aspect is way out of depth for me at the 
moment - I'm not familiar with binutils nearly well enough to take on 
that.

Currently I try to keep things in sync by doing the modifications on one 
and then applying the patch on the other file.

// Martin