[PATCH] fixincludes: simplify handling for access() failure [PR21283, PR80047]

Xi Ruoyao xry111@mengyan1223.wang
Fri Nov 12 21:58:07 GMT 2021


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]

It's suggested by POSIX to call getcwd() with progressively larger
buffers until it does not give an [ERANGE] error. However, it's highly
unlikely that this error-handling route is ever used.

So we can simplify it instead of writting too much code.  We give up to
use getcwd(), because `make` will output a `Leaving directory ...` message
containing the path to cwd when we call abort().

fixincludes/ChangeLog:

	PR other/21823
	PR bootstrap/80047
	* fixincl.c (process): Simplify the handling for highly
	  unlikely access() failure, to avoid using non-standard
	  extensions.
---
 fixincludes/fixincl.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c
index 6dba2f6e830..ee57fbf61b4 100644
--- a/fixincludes/fixincl.c
+++ b/fixincludes/fixincl.c
@@ -1352,11 +1352,10 @@ process (void)
 
   if (access (pz_curr_file, R_OK) != 0)
     {
-      int erno = errno;
-      fprintf (stderr, "Cannot access %s from %s\n\terror %d (%s)\n",
-               pz_curr_file, getcwd ((char *) NULL, MAXPATHLEN),
-               erno, xstrerror (erno));
-      return;
+      /* Some really strange error happened. */
+      fprintf (stderr, "Cannot access %s: %s\n", pz_curr_file,
+	       xstrerror (errno));
+      abort();
     }
 
   pz_curr_data = load_file (pz_curr_file);
-- 
2.33.1

> 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.


More information about the Gcc-patches mailing list