Skip to content

Commit

Permalink
Free FILE* and buffer from util.c process_lines()
Browse files Browse the repository at this point in the history
This feels really messy, although the actual diff is relatively small.  It's
primarily icky because it involves the callback configfile_helper(), which can
invoke bsdtar_errc().  According to the comment for process_lines(), the
callback is supposed to return 0 or non-zero.  But since configfile_helper()
or dooption() is exiting upon error instead of returning, process_lines()
can't clean up after itself.

My thought right now is that a better fix would involve a much larger change
to libarchive code, but I'm open to other ideas.
  • Loading branch information
gperciva committed Feb 3, 2016
1 parent 2a88fb5 commit a52cd2e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
8 changes: 8 additions & 0 deletions tar/bsdtar.c
Expand Up @@ -170,6 +170,8 @@ bsdtar_init(void)
bsdtar->conffile = NULL;
bsdtar->conf_opt = NULL;
bsdtar->conf_arg = NULL;
bsdtar->conffile_actual = NULL;
bsdtar->conffile_buffer = NULL;

/* We don't have bsdtar->progname yet, so we can't use bsdtar_errc. */
if (atexit(bsdtar_atexit)) {
Expand Down Expand Up @@ -199,6 +201,12 @@ bsdtar_atexit(void)
free(bsdtar->conf_opt);
free(bsdtar->conf_arg);

/* Free file-parsing variables from util.c. */
free(bsdtar->conffile_buffer);
if ((bsdtar->conffile_actual != NULL) &&
(bsdtar->conffile_actual != stdin))
fclose(bsdtar->conffile_actual);

/* Free matching and (if applicable) substitution patterns. */
cleanup_exclusions(bsdtar);
#if HAVE_REGEX_H
Expand Down
2 changes: 2 additions & 0 deletions tar/bsdtar.h
Expand Up @@ -132,6 +132,8 @@ struct bsdtar {
char *conffile;
char *conf_opt;
char *conf_arg;
FILE *conffile_actual;
char *conffile_buffer;

/* Used for --dryrun with tarsnap.conf.sample with a missing keyfile. */
int config_file_keyfile_failed;
Expand Down
13 changes: 13 additions & 0 deletions tar/util.c
Expand Up @@ -311,10 +311,18 @@ process_lines(struct bsdtar *bsdtar, const char *pathname,
f = fopen(pathname, "r");
if (f == NULL)
bsdtar_errc(bsdtar, 1, errno, "Couldn't open %s", pathname);

/* Record pointer for freeing upon error. */
bsdtar->conffile_actual = f;

buff_length = 8192;
buff = malloc(buff_length);
if (buff == NULL)
bsdtar_errc(bsdtar, 1, ENOMEM, "Can't read %s", pathname);

/* Record pointer for freeing upon error. */
bsdtar->conffile_buffer = buff;

line_start = line_end = buff_end = buff;
for (;;) {
/* Get some more data into the buffer. */
Expand Down Expand Up @@ -381,6 +389,11 @@ process_lines(struct bsdtar *bsdtar, const char *pathname,
free(buff);
if (f != stdin)
fclose(f);

/* Memory has been freed. */
bsdtar->conffile_actual = NULL;
bsdtar->conffile_buffer = NULL;

return (ret);
}

Expand Down

0 comments on commit a52cd2e

Please sign in to comment.