[PATCH 3/9] Fix several potential memory leaks
Tim Hentenaar <[email protected]> Wed, 17 Jun 2015 20:55:59 +0200
| Newsgroups | gmane.comp.gnu.indent.bugs |
|---|---|
| Message-ID | <[email protected]> |
* 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 <[email protected]> * 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