From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by sourceware.org (Postfix) with ESMTPS id 6CBE13858CDB for ; Wed, 24 Aug 2022 10:03:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6CBE13858CDB Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=martin.st Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=martin.st Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 27OA3J6K025564-27OA3J6L025564; Wed, 24 Aug 2022 13:03:19 +0300 Received: from foo.martin.st (host-97-187.parnet.fi [77.234.97.187]) by mail9.parnet.fi (Postfix) with ESMTPS id ABC02A144B; Wed, 24 Aug 2022 13:03:18 +0300 (EEST) Date: Wed, 24 Aug 2022 13:03:17 +0300 (EEST) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: Nick Clifton cc: binutils@sourceware.org Subject: Re: [PATCH v4] ld: Make archive member file extension comparisons case insensitive when cross compiling too In-Reply-To: <8451b476-1777-2fb5-74bb-042ee17aefee@redhat.com> Message-ID: References: <20220824082316.3213884-1-martin@martin.st> <8451b476-1777-2fb5-74bb-042ee17aefee@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed X-FE-Policy-ID: 3:14:2:SYSTEM X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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