Hi Bernhard,

I cannot comment on the gcc/ parts, but

Am 05.11.21 um 22:17 schrieb Bernhard Reutner-Fischer via Gcc-patches:
From: Bernhard Reutner-Fischer <al...@gcc.gnu.org>

gcc/fortran/ChangeLog:

        PR fortran/68800
        * cpp.h (gfc_cpp_free_cpp_dirs): New declaration.
        * cpp.c (gfc_cpp_free_cpp_dirs): New definition.
        (gfc_cpp_add_include_path, gfc_cpp_add_include_path_after): Add
        comment.
        * f95-lang.c (gfc_finish): Call gfc_cpp_free_cpp_dirs.
        * scanner.c (add_path_to_list): Allocate unzeroed memory.
        (gfc_add_include_path): Add comment and fix formatting.

---
Bootstrapped and regtested without regressions on x86_64-unknown-linux.
Ok for trunk?

Note: in add_path_to_list i changed dir->path = XCNEWVEC to XNEWVEC
since strcpy and strcat add a terminating null byte for us anyway.
Fixes:

-== 68 bytes in 1 blocks are still reachable in loss record 228 of 371
-==    at : malloc (vg_replace_malloc.c:380)
-==    by : xmalloc (xmalloc.c:149)
-==    by : xstrdup (xstrdup.c:34)
-==    by : gfc_add_include_path(char const*, bool, bool, bool, bool) (scanner.c
:428)
-==    by : gfc_handle_option(unsigned long, char const*, long, int, unsigned in
t, cl_option_handlers const*) (options.c:702)
-==    by : handle_option(gcc_options*, gcc_options*, cl_decoded_option const*,
unsigned int, int, unsigned int, cl_option_handlers const*, bool, diagnostic_con
text*) (opts-common.c:1181)
-==    by : read_cmdline_option(gcc_options*, gcc_options*, cl_decoded_option*,
unsigned int, unsigned int, cl_option_handlers const*, diagnostic_context*) (opt
s-common.c:1431)
-==    by : read_cmdline_options (opts-global.c:237)
-==    by : decode_options(gcc_options*, gcc_options*, cl_decoded_option*, unsig
ned int, unsigned int, diagnostic_context*, void (*)()) (opts-global.c:319)
-==    by : toplev::main(int, char**) (toplev.c:2273)
-==    by : main (main.c:39)
---
  gcc/fortran/cpp.c      | 13 +++++++++++--
  gcc/fortran/cpp.h      |  1 +
  gcc/fortran/f95-lang.c |  2 +-
  gcc/fortran/scanner.c  |  7 ++++---
  4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
index e86386c8b17..04fe8fe460b 100644
--- a/gcc/fortran/cpp.c
+++ b/gcc/fortran/cpp.c
@@ -728,12 +728,20 @@ gfc_cpp_done (void)
    cpp_clear_file_cache (cpp_in);
  }

why do you introduce a wrapper for something outside of fortran
that is used only once,

-/* PATH must be malloc-ed and NULL-terminated.  */
+/* Free all cpp include dirs.  */
+void
+gfc_cpp_free_cpp_dirs (void)
+{
+  free_cpp_dirs ();
+}
+
+/* PATH must be NULL-terminated.  */
  void
  gfc_cpp_add_include_path (char *path, bool user_supplied)
  {
    /* CHAIN sets cpp_dir->sysp which differs from 0 if PATH is a system
-     include path. Fortran does not define any system include paths.  */
+     include path. Fortran does not define any system include paths.
+     incpath.c manages the incoming, xstrdup()ed path.  */
    int cxx_aware = 0;

    add_path (path, INC_BRACKET, cxx_aware, user_supplied);
@@ -742,6 +750,7 @@ gfc_cpp_add_include_path (char *path, bool user_supplied)
  void
  gfc_cpp_add_include_path_after (char *path, bool user_supplied)
  {
+  /* incpath.c manages the incoming, xstrdup()ed path.  */
    int cxx_aware = 0;
    add_path (path, INC_AFTER, cxx_aware, user_supplied);
  }
diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
index 44644a2a333..963b9a9c89e 100644
--- a/gcc/fortran/cpp.h
+++ b/gcc/fortran/cpp.h
@@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool);
  bool gfc_cpp_preprocess (const char *source_file);

  void gfc_cpp_done (void);
+void gfc_cpp_free_cpp_dirs (void);

  void gfc_cpp_add_include_path (char *path, bool user_supplied);
  void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 58dcaf01d75..ec4c2cf01d9 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -275,7 +275,7 @@ gfc_finish (void)
    gfc_cpp_done ();
    gfc_done_1 ();
    gfc_release_include_path ();
-  return;

namely here?

+  gfc_cpp_free_cpp_dirs ();
  }

Why not call free_cpp_dirs () here directly, omit all unnecessary
stuff, and maybe only add a brief comment here?

  /* These functions and variables deal with binding contours.  We only
diff --git a/gcc/fortran/scanner.c b/gcc/fortran/scanner.c
index 69b81ab97f8..268d1e3e7fb 100644
--- a/gcc/fortran/scanner.c
+++ b/gcc/fortran/scanner.c
@@ -370,7 +370,7 @@ add_path_to_list (gfc_directorylist **list, const char 
*path,
    char *q;
    size_t len;
    int i;
-
+
    p = path;
    while (*p == ' ' || *p == '\t')  /* someone might do "-I include" */
      if (*p++ == '\0')
@@ -409,7 +409,7 @@ add_path_to_list (gfc_directorylist **list, const char 
*path,
      *list = dir;
    dir->use_for_modules = use_for_modules;
    dir->warn = warn; > -  dir->path = XCNEWVEC (char, strlen (p) + 2);
+  dir->path = XNEWVEC (char, strlen (p) + 2);
    strcpy (dir->path, p);
    strcat (dir->path, "/");       /* make '/' last character */
  }
@@ -424,8 +424,9 @@ gfc_add_include_path (const char *path, bool 
use_for_modules, bool file_dir,
                    defer_warn);

    /* For '#include "..."' these directories are automatically searched.  */
+  /* CPP manages 'path' on it's own via incpath.c so give it it's own copy.  */
    if (!file_dir)
-    gfc_cpp_add_include_path (xstrdup(path), true);
+    gfc_cpp_add_include_path (xstrdup (path), true);
  }




Harald

Reply via email to