From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mengyan1223.wang (mengyan1223.wang [89.208.246.23]) by sourceware.org (Postfix) with ESMTPS id 987983858413 for ; Fri, 12 Nov 2021 21:08:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 987983858413 Received: from [IPv6:240e:35a:103f:9400:dc73:854d:832e:2] (unknown [IPv6:240e:35a:103f:9400:dc73:854d:832e:2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature ECDSA (P-384) server-digest SHA384) (Client did not present a certificate) (Authenticated sender: xry111@mengyan1223.wang) by mengyan1223.wang (Postfix) with ESMTPSA id 6462865F72; Fri, 12 Nov 2021 16:08:39 -0500 (EST) Message-ID: Subject: Re: [PATCH] fixincludes: fix portability issues about getcwd() [PR21283, PR80047] From: Xi Ruoyao To: Bruce Korb , gcc-patches Date: Sat, 13 Nov 2021 05:08:32 +0800 In-Reply-To: <3053902e-315b-fc4f-f2f1-fea78a630947@gnu.org> References: <109aefbeac593ab5660a71df38f1727002c19e39.camel@mengyan1223.wang> <14343662168642b2d975044fccf5e235695bedc7.camel@mengyan1223.wang> <3053902e-315b-fc4f-f2f1-fea78a630947@gnu.org> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3037.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, JMQ_SPF_NEUTRAL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Nov 2021 21:08:44 -0000 On Fri, 2021-11-12 at 12:59 -0800, Bruce Korb wrote: > If you are going to be excruciatingly, painfully correct, free() is > going to be unhappy about freeing a static string in the event > getcwd() fails for some inexplicable reason. I'd replace the free() + > return with a call to exit. Maybe even: It's free (buf), not free (cwd). buf won't point to a static string. buf may be NULL though, but free (NULL) is legal (no-op). > > if (VERY_UNLIKELY (access (pz_curr_file, R_OK) != 0)) abort() Perhaps just  if (access (pz_curr_file, R_OK) != 0)) { /* Some really inexplicable error happens. */ fprintf (stderr, "Cannot access %s: %s", pz_curr_file, xstrerror (errno)); abort(); } It will show which file can't be accessed so it's possible to diagnose. And the working directory will be outputed by "make" when the fixincl command fails anyway, so we don't need to really care it. > On 11/11/21 8:33 AM, Xi Ruoyao wrote: >   > > --- > >  fixincludes/fixincl.c | 13 +++++++++++-- > >  1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c > > index 6dba2f6e830..1580c67efec 100644 > > --- a/fixincludes/fixincl.c > > +++ b/fixincludes/fixincl.c > > @@ -1353,9 +1353,18 @@ process (void) > >    if (access (pz_curr_file, R_OK) != 0) > >      { > >        int erno = errno; > > + char *buf = NULL; > > + const char *cwd = NULL; > > + for (size_t size = 256; !cwd; size += size) > > + { > > + buf = xrealloc (buf, size); > > + cwd = getcwd (buf, size); > > + if (!cwd && errno != ERANGE) > > + cwd = "the working directory"; > > + } > >        fprintf (stderr, "Cannot access %s from %s\n\terror %d > > (%s)\n", > > - pz_curr_file, getcwd ((char *) NULL, MAXPATHLEN), > > - erno, xstrerror (erno)); > > + pz_curr_file, cwd, erno, xstrerror (erno)); > > + free (buf); > >        return; > >      } > >   >   -- Xi Ruoyao School of Aerospace Science and Technology, Xidian University