Message ID | 14343662168642b2d975044fccf5e235695bedc7.camel@mengyan1223.wang |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 347E93858430 for <patchwork@sourceware.org>; Thu, 11 Nov 2021 16:34:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 347E93858430 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1636648454; bh=PNOvntBtQc5UuCbff965DhoouDVKIchw1gSv1LNaWA8=; h=Subject:To:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=s/5AZbKusqILDrP3yYUl4Z7+gkmcWWUOKqR7HgkGoO+1OWcWP9pQtd0oe7gic8d8Y iRI7TLa5/xz9h9GnYyjiwcdmXUhE/73VOUXiaN9LNjSmqivLb/bHYbUAZtxUDe6sXa pMfyoEnydarLyBph9YrADqttvRzZwxo2gT1+/L18= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mengyan1223.wang (mengyan1223.wang [89.208.246.23]) by sourceware.org (Postfix) with ESMTPS id 62E7C385840A for <gcc-patches@gcc.gnu.org>; Thu, 11 Nov 2021 16:33:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 62E7C385840A Received: from [IPv6:240e:35a:10db:700:dc73:854d:832e:2] (unknown [IPv6:240e:35a:10db:700: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 D40AA65B63; Thu, 11 Nov 2021 11:33:38 -0500 (EST) Message-ID: <14343662168642b2d975044fccf5e235695bedc7.camel@mengyan1223.wang> Subject: [PATCH] fixincludes: fix portability issues about getcwd() [PR21283, PR80047] To: gcc-patches <gcc-patches@gcc.gnu.org> Date: Fri, 12 Nov 2021 00:33:32 +0800 In-Reply-To: <CAMfHzOsApOACvVJmXwUkTjtaXAO3h+v=-ekkX294zKc5p9ETaA@mail.gmail.com> References: <109aefbeac593ab5660a71df38f1727002c19e39.camel@mengyan1223.wang> <CAMfHzOsApOACvVJmXwUkTjtaXAO3h+v=-ekkX294zKc5p9ETaA@mail.gmail.com> 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=-3039.3 required=5.0 tests=BAYES_00, 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Xi Ruoyao <xry111@mengyan1223.wang> Cc: Bruce Korb <bkorb@gnu.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
fixincludes: fix portability issues about getcwd() [PR21283, PR80047]
|
|
Commit Message
Xi Ruoyao
Nov. 11, 2021, 4:33 p.m. UTC
[Revised to handle PR 21283.] POSIX says: On some implementations, if buf is a null pointer, getcwd() may obtain size bytes of memory using malloc(). In this case, the pointer returned by getcwd() may be used as the argument in a subsequent call to free(). Invoking getcwd() with buf as a null pointer is not recommended in conforming applications. This produces an error building GCC with --enable-werror-always: ../../../fixincludes/fixincl.c: In function ‘process’: ../../../fixincludes/fixincl.c:1356:7: error: argument 1 is null but the corresponding size argument 2 value is 4096 [-Werror=nonnull] And, at least we've been leaking memory even if getcwd() supports this non-standard extension. And, MAXPATHLEN may be not unavailable on certain platform. PATH_MAX is POSIX, but getcwd() may produce a path with length larger than it. So it's suggested by POSIX [1] to call getcwd() with progressively larger buffers until it does not give an [ERANGE] error. [1]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/getcwd.html fixincludes/ChangeLog: PR other/21823 PR bootstrap/80047 * fixincl.c (process): Allocate and deallocate the buffer for getcwd() progressively. --- fixincludes/fixincl.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
Comments
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: if (VERY_UNLIKELY (access (pz_curr_file, R_OK) != 0)) abort() 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; > } >
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; > > } > > >
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; }