archive_write_client: Free state in freer, not in closer

The closer will not be called if a fatal error occurs, so the current
arrangement results in a memory leak.  The downside is that the freer
may be called even if we were not fully constructed, so it needs to
perform additional checks.  On the other hand, knowing that the freer
always gets called and will free the client state simplifies error
handling in the opener.

diff --git a/libarchive/archive_write.c b/libarchive/archive_write.c
index a8e7b63b5bfed86b577c35d42199685056914602..292f719dfc0fb91d3d1a9938db48d00db418c18d 100644 (file)
--- a/libarchive/archive_write.c
+++ b/libarchive/archive_write.c
@@ -360,7 +360,6 @@ archive_write_client_open(struct archive_write_filter *f)
        struct archive_none *state;
        void *buffer;
        size_t buffer_size;
-       int ret;
 
        f->bytes_per_block = archive_write_get_bytes_per_block(f->archive);
        f->bytes_in_last_block =
@@ -385,13 +384,7 @@ archive_write_client_open(struct archive_write_filter *f)
 
        if (a->client_opener == NULL)
                return (ARCHIVE_OK);
-       ret = a->client_opener(f->archive, a->client_data);
-       if (ret != ARCHIVE_OK) {
-               free(state->buffer);
-               free(state);
-               f->data = NULL;
-       }
-       return (ret);
+       return (a->client_opener(f->archive, a->client_data));
 }
 
 static int
@@ -480,6 +473,7 @@ static int
 archive_write_client_free(struct archive_write_filter *f)
 {
        struct archive_write *a = (struct archive_write *)f->archive;
+       struct archive_none *state = (struct archive_none *)f->data;
 
        if (a->client_freer)
                (*a->client_freer)(&a->archive, a->client_data);
@@ -492,6 +486,13 @@ archive_write_client_free(struct archive_write_filter *f)
                a->passphrase = NULL;
        }
 
+       /* Free state. */
+       if (state != NULL) {
+               free(state->buffer);
+               free(state);
+               f->data = NULL;
+       }
+
        return (ARCHIVE_OK);
 }
 
@@ -548,8 +549,6 @@ archive_write_client_close(struct archive_write_filter *f)
        }
        if (a->client_closer)
                (*a->client_closer)(&a->archive, a->client_data);
-       free(state->buffer);
-       free(state);
 
        /* Clear the close handler myself not to be called again. */
        f->state = ARCHIVE_WRITE_FILTER_STATE_CLOSED;