From: Thiago Farina <tfransosi@gmail.com>
Date: Thu, 23 Jun 2011 17:05:35 GMT
Subject: Re: [PATCHv2 3/9] archive: refactor list of archive formats
Message-ID: <BANLkTim7O3pcJAy4U1d6QiS6cvv2-Og21A@mail.gmail.com>
In-Reply-To: <20110622012333.GC30604@sigill.intra.peff.net>

On Tue, Jun 21, 2011 at 10:23 PM, Jeff King <peff@peff.net> wrote:
> Most of the tar and zip code was nicely split out into two
> abstracted files which knew only about their specific
> formats. The entry point to this code was a single "write
> archive" function.
>
> However, as these basic formats grow more complex (e.g., by
> handling multiple file extensions and format names), a
> static list of the entry point functions won't be enough.
> Instead, let's provide a way for the tar and zip code to
> tell the main archive code what they support by registering
> archiver names and functions.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> New in v2. This turns archivers more into proper objects, rather than a
> hard-coded list of functions, and makes the rest of the series much
> cleaner.
>
>  archive-tar.c |   16 +++++++++++++---
>  archive-zip.c |   13 ++++++++++++-
>  archive.c     |   33 +++++++++++++++++----------------
>  archive.h     |   17 ++++++++++-------
>  4 files changed, 52 insertions(+), 27 deletions(-)
>
> diff --git a/archive-tar.c b/archive-tar.c
> index 1ab1a2c..930375b 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -234,12 +234,10 @@ static int git_tar_config(const char *var, const char *value, void *cb)
>        return 0;
>  }
>
> -int write_tar_archive(struct archiver_args *args)
> +static int write_tar_archive(struct archiver_args *args)
>  {
>        int err = 0;
>
> -       git_config(git_tar_config, NULL);
> -
>        if (args->commit_sha1)
>                err = write_global_extended_header(args);
>        if (!err)
> @@ -248,3 +246,15 @@ int write_tar_archive(struct archiver_args *args)
>                write_trailer();
>        return err;
>  }
> +
> +static struct archiver tar_archiver = {
> +       "tar",
> +       write_tar_archive,
> +       0
A named constant instead of 0, like you did with
ARCHIVER_WANT_COMPRESSION_LEVELS, would be better? 0 here means the
archiver does not want compression?