bug-indent
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 3/9] Fix several potential memory leaks


From: Tim Hentenaar
Subject: [PATCH 3/9] Fix several potential memory leaks
Date: Wed, 17 Jun 2015 20:55:59 +0200

* Provide a wrapper around free in globs (xfree)
* Provide a wrapper around exit(3) and ensure that it's the only
  routine in the application that calls exit(3). This wrapper should
  call certain cleanup functions to free() common heap-allocated
  storage.
* Provide a uninit_parser() routine to free heap memory allocated in
  init_parser().
* free() allocated buffers in read_file()/read_stdin() on error.
* free() the same buffers when we're done with them.
* Replaced a strncpy() with memcpy() since the length of the copied
  string, and thus the buffer size, is already known.
---
 ChangeLog     |  4 ++++
 src/args.c    | 31 ++++++++++++++-----------------
 src/code_io.c | 18 +++++++++---------
 src/globs.c   | 32 ++++++++++++++++++++++++++++++--
 src/globs.h   |  6 ++++++
 src/indent.c  | 36 ++++++++++++++++++++++++------------
 src/lexi.c    | 13 +++++++++++++
 src/lexi.h    |  1 +
 src/output.c  |  3 +--
 src/parse.c   | 23 +++++++++++++++++++++++
 src/parse.h   |  1 +
 src/utils.c   |  3 ++-
 12 files changed, 128 insertions(+), 43 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 672b3db..d281eba 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,10 @@
          double spaces.
        * Fix print_comment reading past the end of the buffer when reading
         less than a full comment from stdin.
+       * Ensure that there's only one point where exit(3) is used.
+       * Add xfree() to globs.
+       * xfree() common heap-allocated storage.
+       * Fixed some memory leaks in the profile stuff.
 
 2015-06-15 Tim Hentenaar <address@hidden>
        * Added -par to -kr.
diff --git a/src/args.c b/src/args.c
index 5018027..1d2bd75 100644
--- a/src/args.c
+++ b/src/args.c
@@ -789,6 +789,8 @@ extern int set_option(
     int            option_length = option_prefix (option);
     int            val           = 0;
     BOOLEAN        found         = false;
+    char          *tmp           = NULL;
+    size_t         param_len     = 0;
   
     if (option_length > 0)
     {
@@ -879,19 +881,14 @@ extern int set_option(
                 
             case PRO_SETTINGS:
                 {
-                    char *t;            /* current position */
+                    /* current position */
+                    tmp = (char *)p->p_obj;
                     
-                    t = (char *) p->p_obj;
-                    
-                    do
-                    {
-                       set_option (t, 0, 0, option_source);
+                    do {
+                        set_option (tmp, 0, 0, option_source);
                         /* advance to character following next NUL */
-                        
-                        while (*t++)
-                        {
-                        }
-                    } while (*t);
+                        while (*tmp++);
+                    } while (*tmp);
                 }
                 break;
                 
@@ -900,8 +897,6 @@ extern int set_option(
 
             case PRO_KEY:
                 {
-                    char *str;
-
                     if (*param_start == 0)
                     {
                         if (!(param_start = param))
@@ -914,9 +909,11 @@ extern int set_option(
                         }
                     }
 
-                    str = (char *) xmalloc (strlen (param_start) + 1);
-                    strcpy (str, param_start);
-                    addkey (str, rw_decl);
+                    param_len = strlen(param_start);
+                    tmp = xmalloc(param_len + 1);
+                    memcpy(tmp, param_start, param_len);
+                    tmp[param_len] = '\0';
+                    addkey(tmp, rw_decl);
                 }
                 break;
 
@@ -1267,7 +1264,7 @@ char * set_profile(void)
             }
             else
             {
-               free (fname);
+               xfree (fname);
                fname = NULL;
             }
          }
diff --git a/src/code_io.c b/src/code_io.c
index 22e484c..2f00c83 100644
--- a/src/code_io.c
+++ b/src/code_io.c
@@ -257,10 +257,10 @@ extern file_buffer_ty * read_file(
      * bytes in a 16-bit world...
      */
   
-    unsigned int size, size_to_read;
+    unsigned int size = 0, size_to_read = 0;
 #else
-    ssize_t size;
-    size_t size_to_read;
+    ssize_t size = 0;
+    size_t size_to_read = 0;
 #endif
 
     int          namelen = strlen(filename);
@@ -335,6 +335,7 @@ extern file_buffer_ty * read_file(
                 continue;
             }
 #endif
+            xfree(fileptr.data);
             fatal (_("Error reading input file %s"), filename);
         }
         size_to_read -= size;
@@ -342,6 +343,7 @@ extern file_buffer_ty * read_file(
     
     if (close (fd) < 0)
     {
+        xfree(fileptr.data);
         fatal (_("Error closing input file %s"), filename);
     }
     
@@ -364,7 +366,7 @@ extern file_buffer_ty * read_file(
         fileptr.name = (char *) xmalloc (namelen + 1);
     }
     
-    (void)strncpy(fileptr.name, filename, namelen);
+    memcpy(fileptr.name, filename, namelen);
     fileptr.name[namelen] = EOS;
 
     if (fileptr.data[fileptr.size - 1] != EOL)
@@ -372,7 +374,7 @@ extern file_buffer_ty * read_file(
         fileptr.data[fileptr.size] = EOL;
         fileptr.size++;
     }
-    
+
     fileptr.data[fileptr.size] = EOS;
 
     return &fileptr;
@@ -432,7 +434,6 @@ file_buffer_ty * read_stdin(void)
     } while (ch != EOF);
 
     stdinptr.name = "Standard Input";
-
     stdinptr.data[stdinptr.size] = EOS;
 
     return &stdinptr;
@@ -532,9 +533,8 @@ void fill_buffer(void)
         
           else if ((unsigned int) (p - current_input->data) < 
current_input->size)
           {
-             ERROR (_("File %s contains NULL-characters: cannot proceed\n"), 
current_input->name, 0);
-             exit(1);
-             p++;
+             fatal(_("File %s contains NULL-characters: cannot proceed\n"),
+                   current_input->name);
           }
         
          /* Here for EOF with no terminating newline char. */
diff --git a/src/globs.c b/src/globs.c
index 9973bcd..b89905f 100644
--- a/src/globs.c
+++ b/src/globs.c
@@ -21,6 +21,7 @@
 #include "sys.h"
 #include "indent.h"
 #include "globs.h"
+#include "parse.h"
 
 #ifdef HAVE_MALLOC_H
 #include <malloc.h>
@@ -44,7 +45,7 @@ extern char * xmalloc (
     if (!val)
     {
         fprintf (stderr, _("indent: Virtual memory exhausted.\n"));
-        exit (system_error);
+        do_exit (system_error);
     }
 
 #if defined (DEBUG)
@@ -76,6 +77,22 @@ extern char *xrealloc (
 }
 
 /**
+ * Free and nullify a pointer allocated with xmalloc().
+ */
+extern void xfree(void *ptr)
+{
+    if (!ptr)
+    {
+#if defined(DEBUG)
+        fprintf(stderr, "indent: Attempting to free a NULL pointer.\n");
+#endif
+        return;
+    }
+
+    free(ptr);
+}
+
+/**
  *
  */
 
@@ -95,6 +112,17 @@ extern void message(
 }
 
 /**
+ * Wrapper around exit to ensure things get
+ * cleaned up.
+ */
+extern void do_exit(int code)
+{
+    uninit_parser();
+    cleanup_user_specials();
+    exit(code);
+}
+
+/**
  * Print a fatal error message and exit, or, if compiled with
  * "DEBUG" defined, abort ().
  */
@@ -117,5 +145,5 @@ extern void fatal (
         perror (0);
     }
 
-    exit (indent_fatal);
+    do_exit (indent_fatal);
 }
diff --git a/src/globs.h b/src/globs.h
index 373f75d..bb81c76 100644
--- a/src/globs.h
+++ b/src/globs.h
@@ -36,6 +36,12 @@ extern char *xrealloc(
    char *ptr, 
    unsigned int size);
 
+extern void xfree(
+    void *ptr);
+
+extern void do_exit(
+    int code);
+
 extern void fatal(
    const char *string, 
    const char *a0);
diff --git a/src/indent.c b/src/indent.c
index 087b3ce..64c9cbc 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -952,9 +952,9 @@ static exit_values_ty indent_multiple_files(void)
  *
  */
 
-
 static exit_values_ty indent_single_file(BOOLEAN using_stdin)
 {
+    int            is_stdin    = false;
     exit_values_ty exit_status = total_success;
     struct stat    file_stats;
 
@@ -964,6 +964,7 @@ static exit_values_ty indent_single_file(BOOLEAN 
using_stdin)
         in_file_names[0] = "Standard input";
         in_name = in_file_names[0];
         current_input = read_stdin ();
+        is_stdin = true;
     }
     else
     {
@@ -1003,7 +1004,13 @@ static exit_values_ty indent_single_file(BOOLEAN 
using_stdin)
     {
         close_output(NULL, out_name);
     }
-    
+
+    if (current_input) {
+        if (!is_stdin && current_input->name)
+            xfree(current_input->name);
+        xfree(current_input->data);
+    }
+
     return exit_status;
 }
 
@@ -1069,17 +1076,17 @@ int main(
     /* 'size_t', 'wchar_t' and 'ptrdiff_t' are guarenteed to be
      * available in ANSI C.
      *
-     * I'm malloc'ing this in the hopes that it will be freed
-     * along with the rest of the user_specials array in
-     * lexi.c (assuming that it actually is.)
+     * These pointers will be freed in cleanup_user_specials().
      */
-    tmp = (char *)xmalloc(25);
+    tmp = xmalloc(7);
     memcpy(tmp, "size_t", 7);
     addkey(tmp, rw_decl);
-    memcpy(tmp + 7, "wchar_t", 8);
-    addkey(tmp + 7, rw_decl);
-    memcpy(tmp + 15, "ptrdiff_t", 10);
-    addkey(tmp + 15, rw_decl);
+    tmp = xmalloc(8);
+    memcpy(tmp, "wchar_t", 8);
+    addkey(tmp, rw_decl);
+    tmp = xmalloc(10);
+    memcpy(tmp, "ptrdiff_t", 10);
+    addkey(tmp, rw_decl);
 
     init_parser ();
     initialize_backups ();
@@ -1105,6 +1112,11 @@ int main(
 
         exit_status = indent_all(using_stdin);
     }
-    
-    return (exit_status);
+
+    if (profile_pathname)
+        xfree(profile_pathname);
+    xfree(in_file_names);
+    uninit_parser();
+    cleanup_user_specials();
+    return exit_status;
 }
diff --git a/src/lexi.c b/src/lexi.c
index e10f580..326b7e3 100644
--- a/src/lexi.c
+++ b/src/lexi.c
@@ -1283,3 +1283,16 @@ extern void addkey (
        p->rwcode = val;
     }
 }
+
+extern void cleanup_user_specials(void)
+{
+    if (user_specials)
+    {
+        while (--user_specials_idx > 0)
+        {
+            xfree(user_specials[user_specials_idx].rwd);
+        }
+        xfree(user_specials[0].rwd);
+        xfree(user_specials);
+    }
+}
diff --git a/src/lexi.h b/src/lexi.h
index 5cc4716..76bc872 100644
--- a/src/lexi.h
+++ b/src/lexi.h
@@ -103,5 +103,6 @@ extern void addkey(
    rwcodes_ty val);
 
 extern codes_ty lexi(void);
+extern void cleanup_user_specials(void);
 
 #endif /* INDENT_LEXI_H */
diff --git a/src/output.c b/src/output.c
index c3b5fee..7668cc1 100644
--- a/src/output.c
+++ b/src/output.c
@@ -1325,8 +1325,7 @@ void open_output(
                                           *  (but see the trunc function) */
         if (output == NULL)
         {
-            fprintf (stderr, _("indent: can't create %s\n"), filename);
-            exit (indent_fatal);
+            fatal(_("indent: can't create %s\n"), filename);
         }
     }
 }
diff --git a/src/parse.c b/src/parse.c
index 880333e..143d804 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -88,6 +88,29 @@ extern void init_parser(void)
 }
 
 /**
+ * Free all memory allocated in init_parser().
+ */
+extern void uninit_parser(void)
+{
+    if (!parser_state_tos)
+    {
+        return;
+    }
+
+    xfree(parser_state_tos->p_stack);
+    xfree(parser_state_tos->il);
+    xfree(parser_state_tos->cstk);
+    xfree(parser_state_tos->paren_indents);
+    xfree(parser_state_tos);
+    xfree(save_com.ptr);
+    xfree(combuf);
+    xfree(labbuf);
+    xfree(codebuf);
+    xfree(di_stack);
+    parser_state_tos = NULL;
+}
+
+/**
  *
  */
 
diff --git a/src/parse.h b/src/parse.h
index b7ff311..602a981 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -53,6 +53,7 @@ extern int inc_pstack (void);
 extern void parse_lparen_in_decl (void);
 extern exit_values_ty parse (codes_ty tk);
 extern void init_parser (void);
+extern void uninit_parser (void);
 extern void reset_parser (void);
 extern void reduce (void);
 
diff --git a/src/utils.c b/src/utils.c
index 614e8f7..d8043df 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 
 #include "utils.h"
+#include "globs.h"
 
 /**
  *
@@ -40,5 +41,5 @@ extern void DieError(
 
     va_end(ap);
 
-    exit(errval);
+    do_exit(errval);
 }
-- 
2.3.6




reply via email to

[Prev in Thread] Current Thread [Next in Thread]