[groff] 01/02: [libgroff]: Save errno after fopen before free.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit 542a8309c725defa51e1cb2974fea75f4d5f9942
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Sun Feb 14 09:02:04 2021 +1100

    [libgroff]: Save errno after fopen before free.
    
    * src/libs/libgroff/searchpath.cpp (search_path::open_file,
      search_path::open_file_cautious): Save errno before calling free() and
      restore it afterwards.  A future version of POSIX will require that
      free() not change errno if it succeeds[1]; some C library
      implementations, including recent versions of glibc[2], lack this
      property.  free() is called in these libgroff functions to clean up
      after an unsuccessful fopen() of a heap-allocated file name string,
      and because the errno from fopen() may be passed to strerror() in a
      diagnostic message, it needs to be accurate.  I checked the rest of
      groff's codebase and found no other instances of free() being used to
      clean up after fopen() failure.
    
    [1] https://www.austingroupbugs.net/view.php?id=385
    [2] https://sourceware.org/bugzilla/attachment.cgi?id=13073
    
    Also add editor aid comment.
---
 ChangeLog                        | 18 ++++++++++++++++++
 src/libs/libgroff/searchpath.cpp | 22 ++++++++++++++++------
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a3792b5..d718c25 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2021-02-14  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       * src/libs/libgroff/searchpath.cpp (search_path::open_file,
+       search_path::open_file_cautious): Save errno before calling
+       free() and restore it afterwards.  A future version of POSIX
+       will require that free() not change errno if it succeeds[1];
+       some C library implementations, including recent versions of
+       glibc[2], lack this property.  free() is called in these
+       libgroff functions to clean up after an unsuccessful fopen() of
+       a heap-allocated file name string, and because the errno from
+       fopen() may be passed to strerror() in a diagnostic message, it
+       needs to be accurate.  I checked the rest of groff's codebase
+       and found no other instances of free() being used to clean up
+       after fopen() failure.
+
+       [1] https://www.austingroupbugs.net/view.php?id=385
+       [2] https://sourceware.org/bugzilla/attachment.cgi?id=13073
+
 2021-02-11  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        Address build failure on macOS.
diff --git a/src/libs/libgroff/searchpath.cpp b/src/libs/libgroff/searchpath.cpp
index d2fc08c..d478c51 100644
--- a/src/libs/libgroff/searchpath.cpp
+++ b/src/libs/libgroff/searchpath.cpp
@@ -126,14 +126,18 @@ FILE *search_path::open_file(const char *name, char 
**pathp)
     fprintf(stderr, "trying '%s'\n", path);
 #endif
     FILE *fp = fopen(path, "r");
+    int err = errno;
     if (fp) {
       if (pathp)
        *pathp = path;
-      else
+      else {
        free(path);
+       errno = err;
+      }
       return fp;
     }
     free(path);
+    errno = err;
     if (*end == '\0')
       break;
     p = end + 1;
@@ -183,20 +187,20 @@ FILE *search_path::open_file_cautious(const char *name, 
char **pathp,
     fprintf(stderr, "trying '%s'\n", path);
 #endif
     FILE *fp = fopen(path, mode);
+    int err = errno;
     if (fp) {
       if (pathp)
        *pathp = path;
-      else
+      else {
        free(path);
+       errno = err;
+      }
       return fp;
     }
-    int err = errno;
     free(path);
+    errno = err;
     if (err != ENOENT)
-    {
-      errno = err;
       return 0;
-    }
     if (*end == '\0')
       break;
     p = end + 1;
@@ -204,3 +208,9 @@ FILE *search_path::open_file_cautious(const char *name, 
char **pathp,
   errno = ENOENT;
   return 0;
 }
+
+// Local Variables:
+// fill-column: 72
+// mode: C++
+// End:
+// vim: set cindent noexpandtab shiftwidth=2 textwidth=72: