Introduce archive_write_open2() with free callback

The archive_write_open() function does not provide a free callback.
Freeing was done by the close callback. When the open callback fails,
the client filter is left in ARCHIVE_WRITE_FILTER_STATE_FATAL,
the close callback is not called and unfreed resources may be left behind.

Fixes #1456

8 files changed, 95 insertions, 32 deletions

diff --git a/libarchive/archive.h b/libarchive/archive.h
index a3f3bfd7..34d098fd 100644
--- a/libarchive/archive.h
+++ b/libarchive/archive.h
@@ -246,6 +246,8 @@ typedef int	archive_open_callback(struct archive *, void *_client_data);
 
 typedef int	archive_close_callback(struct archive *, void *_client_data);
 
+typedef int	archive_free_callback(struct archive *, void *_client_data);
+
 /* Switches from one client data object to the next/prev client data object.
  * This is useful for reading from different data blocks such as a set of files
  * that make up one large file.
@@ -818,9 +820,13 @@ __LA_DECL int archive_write_set_format_filter_by_ext(struct archive *a, const ch
 __LA_DECL int archive_write_set_format_filter_by_ext_def(struct archive *a, const char *filename, const char * def_ext);
 __LA_DECL int archive_write_zip_set_compression_deflate(struct archive *);
 __LA_DECL int archive_write_zip_set_compression_store(struct archive *);
+/* Deprecated; use archive_write_open2 instead */
 __LA_DECL int archive_write_open(struct archive *, void *,
 		     archive_open_callback *, archive_write_callback *,
 		     archive_close_callback *);
+__LA_DECL int archive_write_open2(struct archive *, void *,
+		     archive_open_callback *, archive_write_callback *,
+		     archive_close_callback *, archive_free_callback *);
 __LA_DECL int archive_write_open_fd(struct archive *, int _fd);
 __LA_DECL int archive_write_open_filename(struct archive *, const char *_file);
 __LA_DECL int archive_write_open_filename_w(struct archive *,
diff --git a/libarchive/archive_write.c b/libarchive/archive_write.c
index 98a55fb2..8d70f51a 100644
--- a/libarchive/archive_write.c
+++ b/libarchive/archive_write.c
@@ -456,6 +456,25 @@ archive_write_client_write(struct archive_write_filter *f,
 }
 
 static int
+archive_write_client_free(struct archive_write_filter *f)
+{
+	struct archive_write *a = (struct archive_write *)f->archive;
+
+	if (a->client_freer)
+		(*a->client_freer)(&a->archive, a->client_data);
+	a->client_data = NULL;
+
+	/* Clear passphrase. */
+	if (a->passphrase != NULL) {
+		memset(a->passphrase, 0, strlen(a->passphrase));
+		free(a->passphrase);
+		a->passphrase = NULL;
+	}
+
+	return (ARCHIVE_OK);
+}
+
+static int
 archive_write_client_close(struct archive_write_filter *f)
 {
 	struct archive_write *a = (struct archive_write *)f->archive;
@@ -493,13 +512,7 @@ archive_write_client_close(struct archive_write_filter *f)
 		(*a->client_closer)(&a->archive, a->client_data);
 	free(state->buffer);
 	free(state);
-	a->client_data = NULL;
-	/* Clear passphrase. */
-	if (a->passphrase != NULL) {
-		memset(a->passphrase, 0, strlen(a->passphrase));
-		free(a->passphrase);
-		a->passphrase = NULL;
-	}
+
 	/* Clear the close handler myself not to be called again. */
 	f->state = ARCHIVE_WRITE_FILTER_STATE_CLOSED;
 	return (ret);
@@ -509,9 +522,9 @@ archive_write_client_close(struct archive_write_filter *f)
  * Open the archive using the current settings.
  */
 int
-archive_write_open(struct archive *_a, void *client_data,
+archive_write_open2(struct archive *_a, void *client_data,
     archive_open_callback *opener, archive_write_callback *writer,
-    archive_close_callback *closer)
+    archive_close_callback *closer, archive_free_callback *freer)
 {
 	struct archive_write *a = (struct archive_write *)_a;
 	struct archive_write_filter *client_filter;
@@ -524,12 +537,14 @@ archive_write_open(struct archive *_a, void *client_data,
 	a->client_writer = writer;
 	a->client_opener = opener;
 	a->client_closer = closer;
+	a->client_freer = freer;
 	a->client_data = client_data;
 
 	client_filter = __archive_write_allocate_filter(_a);
 	client_filter->open = archive_write_client_open;
 	client_filter->write = archive_write_client_write;
 	client_filter->close = archive_write_client_close;
+	client_filter->free = archive_write_client_free;
 
 	ret = __archive_write_filters_open(a);
 	if (ret < ARCHIVE_WARN) {
@@ -544,6 +559,15 @@ archive_write_open(struct archive *_a, void *client_data,
 	return (ret);
 }
 
+int
+archive_write_open(struct archive *_a, void *client_data,
+    archive_open_callback *opener, archive_write_callback *writer,
+    archive_close_callback *closer)
+{
+	return archive_write_open2(_a, client_data, opener, writer,
+	    closer, NULL);
+}
+
 /*
  * Close out the archive.
  */
diff --git a/libarchive/archive_write_open.3 b/libarchive/archive_write_open.3
index 0129d10b..2f6e6270 100644
--- a/libarchive/archive_write_open.3
+++ b/libarchive/archive_write_open.3
@@ -28,7 +28,7 @@
 .Dt ARCHIVE_WRITE_OPEN 3
 .Os
 .Sh NAME
-.Nm archive_write_open ,
+.Nm archive_write_open2 ,
 .Nm archive_write_open_fd ,
 .Nm archive_write_open_FILE ,
 .Nm archive_write_open_filename ,
@@ -39,12 +39,13 @@ Streaming Archive Library (libarchive, -larchive)
 .Sh SYNOPSIS
 .In archive.h
 .Ft int
-.Fo archive_write_open
+.Fo archive_write_open2
 .Fa "struct archive *"
 .Fa "void *client_data"
 .Fa "archive_open_callback *"
 .Fa "archive_write_callback *"
 .Fa "archive_close_callback *"
+.Fa "archive_free_callback *"
 .Fc
 .Ft int
 .Fn archive_write_open_fd "struct archive *" "int fd"
@@ -61,10 +62,10 @@ Streaming Archive Library (libarchive, -larchive)
 .Fc
 .Sh DESCRIPTION
 .Bl -tag -width indent
-.It Fn archive_write_open
+.It Fn archive_write_open2
 Freeze the settings, open the archive, and prepare for writing entries.
 This is the most generic form of this function, which accepts
-pointers to three callback functions which will be invoked by
+pointers to four callback functions which will be invoked by
 the compression layer to write the constructed archive.
 This does not alter the default archive padding.
 .It Fn archive_write_open_fd
@@ -106,14 +107,14 @@ to a character or block device node, it will disable padding otherwise.
 You can override this by manually invoking
 .Fn archive_write_set_bytes_in_last_block
 before calling
-.Fn archive_write_open .
+.Fn archive_write_open2 .
 The
 .Fn archive_write_open_filename
 function is safe for use with tape drives or other
 block-oriented devices.
 .It Fn archive_write_open_memory
 A convenience form of
-.Fn archive_write_open
+.Fn archive_write_open2
 that accepts a pointer to a block of memory that will receive
 the archive.
 The final
@@ -145,7 +146,7 @@ To use this library, you will need to define and register
 callback functions that will be invoked to write data to the
 resulting archive.
 These functions are registered by calling
-.Fn archive_write_open :
+.Fn archive_write_open2 :
 .Bl -item -offset indent
 .It
 .Ft typedef int
@@ -192,7 +193,8 @@ to register an error code and message and return -1.
 .El
 .Pp
 The close callback is invoked by archive_close when
-the archive processing is complete.
+the archive processing is complete. If the open callback fails, the close
+callback is not invoked.
 The callback should return
 .Cm ARCHIVE_OK
 on success.
@@ -200,7 +202,14 @@ On failure, the callback should invoke
 .Fn archive_set_error
 to register an error code and message and
 return
-.Cm ARCHIVE_FATAL .
+.Bl -item -offset indent
+.It
+.Ft typedef int
+.Fn archive_free_callback "struct archive *" "void *client_data"
+.El
+.Pp
+The free callback is always invoked on archive_free.
+The return code of this callback is not processed.
 .Pp
 Note that if the client-provided write callback function
 returns a non-zero value, that error will be propagated back to the caller
diff --git a/libarchive/archive_write_open_fd.c b/libarchive/archive_write_open_fd.c
index d5c426cf..b8d491fa 100644
--- a/libarchive/archive_write_open_fd.c
+++ b/libarchive/archive_write_open_fd.c
@@ -54,7 +54,7 @@ struct write_fd_data {
 	int		fd;
 };
 
-static int	file_close(struct archive *, void *);
+static int	file_free(struct archive *, void *);
 static int	file_open(struct archive *, void *);
 static ssize_t	file_write(struct archive *, void *, const void *buff, size_t);
 
@@ -72,8 +72,8 @@ archive_write_open_fd(struct archive *a, int fd)
 #if defined(__CYGWIN__) || defined(_WIN32)
 	setmode(mine->fd, O_BINARY);
 #endif
-	return (archive_write_open(a, mine,
-		    file_open, file_write, file_close));
+	return (archive_write_open2(a, mine,
+		    file_open, file_write, NULL, file_free));
 }
 
 static int
@@ -134,11 +134,13 @@ file_write(struct archive *a, void *client_data, const void *buff, size_t length
 }
 
 static int
-file_close(struct archive *a, void *client_data)
+file_free(struct archive *a, void *client_data)
 {
 	struct write_fd_data	*mine = (struct write_fd_data *)client_data;
 
 	(void)a; /* UNUSED */
+	if (mine == NULL)
+		return (ARCHIVE_OK);
 	free(mine);
 	return (ARCHIVE_OK);
 }
diff --git a/libarchive/archive_write_open_file.c b/libarchive/archive_write_open_file.c
index f6b14123..bf5b55a6 100644
--- a/libarchive/archive_write_open_file.c
+++ b/libarchive/archive_write_open_file.c
@@ -51,7 +51,7 @@ struct write_FILE_data {
 	FILE		*f;
 };
 
-static int	file_close(struct archive *, void *);
+static int	file_free(struct archive *, void *);
 static int	file_open(struct archive *, void *);
 static ssize_t	file_write(struct archive *, void *, const void *buff, size_t);
 
@@ -66,8 +66,8 @@ archive_write_open_FILE(struct archive *a, FILE *f)
 		return (ARCHIVE_FATAL);
 	}
 	mine->f = f;
-	return (archive_write_open(a, mine,
-		    file_open, file_write, file_close));
+	return (archive_write_open2(a, mine, file_open, file_write,
+	    NULL, file_free));
 }
 
 static int
@@ -99,11 +99,13 @@ file_write(struct archive *a, void *client_data, const void *buff, size_t length
 }
 
 static int
-file_close(struct archive *a, void *client_data)
+file_free(struct archive *a, void *client_data)
 {
 	struct write_FILE_data	*mine = client_data;
 
 	(void)a; /* UNUSED */
+	if (mine == NULL)
+		return (ARCHIVE_OK);
 	free(mine);
 	return (ARCHIVE_OK);
 }
diff --git a/libarchive/archive_write_open_filename.c b/libarchive/archive_write_open_filename.c
index 66e0dfee..9ceefb19 100644
--- a/libarchive/archive_write_open_filename.c
+++ b/libarchive/archive_write_open_filename.c
@@ -62,6 +62,7 @@ struct write_file_data {
 };
 
 static int	file_close(struct archive *, void *);
+static int	file_free(struct archive *, void *);
 static int	file_open(struct archive *, void *);
 static ssize_t	file_write(struct archive *, void *, const void *buff, size_t);
 static int	open_filename(struct archive *, int, const void *);
@@ -123,8 +124,8 @@ open_filename(struct archive *a, int mbs_fn, const void *filename)
 		return (ARCHIVE_FAILED);
 	}
 	mine->fd = -1;
-	return (archive_write_open(a, mine,
-		file_open, file_write, file_close));
+	return (archive_write_open2(a, mine,
+		    file_open, file_write, file_close, file_free));
 }
 
 static int
@@ -244,9 +245,25 @@ file_close(struct archive *a, void *client_data)
 
 	(void)a; /* UNUSED */
 
+	if (mine == NULL)
+		return (ARCHIVE_FATAL);
+
 	if (mine->fd >= 0)
 		close(mine->fd);
 
+	return (ARCHIVE_OK);
+}
+
+static int
+file_free(struct archive *a, void *client_data)
+{
+	struct write_file_data	*mine = (struct write_file_data *)client_data;
+
+	(void)a; /* UNUSED */
+
+	if (mine == NULL)
+		return (ARCHIVE_OK);
+
 	archive_mstring_clean(&mine->filename);
 	free(mine);
 	return (ARCHIVE_OK);
diff --git a/libarchive/archive_write_open_memory.c b/libarchive/archive_write_open_memory.c
index ea6ae0ac..a8a0b817 100644
--- a/libarchive/archive_write_open_memory.c
+++ b/libarchive/archive_write_open_memory.c
@@ -39,7 +39,7 @@ struct write_memory_data {
 	unsigned char * buff;
 };
 
-static int	memory_write_close(struct archive *, void *);
+static int	memory_write_free(struct archive *, void *);
 static int	memory_write_open(struct archive *, void *);
 static ssize_t	memory_write(struct archive *, void *, const void *buff, size_t);
 
@@ -61,8 +61,8 @@ archive_write_open_memory(struct archive *a, void *buff, size_t buffSize, size_t
 	mine->buff = buff;
 	mine->size = buffSize;
 	mine->client_size = used;
-	return (archive_write_open(a, mine,
-		    memory_write_open, memory_write, memory_write_close));
+	return (archive_write_open2(a, mine,
+		    memory_write_open, memory_write, NULL, memory_write_free));
 }
 
 static int
@@ -103,11 +103,13 @@ memory_write(struct archive *a, void *client_data, const void *buff, size_t leng
 }
 
 static int
-memory_write_close(struct archive *a, void *client_data)
+memory_write_free(struct archive *a, void *client_data)
 {
 	struct write_memory_data *mine;
 	(void)a; /* UNUSED */
 	mine = client_data;
+	if (mine == NULL)
+		return (ARCHIVE_OK);
 	free(mine);
 	return (ARCHIVE_OK);
 }
diff --git a/libarchive/archive_write_private.h b/libarchive/archive_write_private.h
index 27cba039..155fdd73 100644
--- a/libarchive/archive_write_private.h
+++ b/libarchive/archive_write_private.h
@@ -89,6 +89,7 @@ struct archive_write {
 	archive_open_callback	*client_opener;
 	archive_write_callback	*client_writer;
 	archive_close_callback	*client_closer;
+	archive_free_callback	*client_freer;
 	void			*client_data;
 
 	/*