Re: [PATCH 02/12] commit-graph: verify file header information

From: Martin Ågren <martin.agren@gmail.com>
Date: May 10, 2018, 18:21 UTC
Message-ID: <CAN0heSrNdHGyrZ2qF6J2daD0VdjXDhErs1nVzO+KT5GvOPfAPw@mail.gmail.com>
In-Reply-To: <20180510173345.40577-3-dstolee@microsoft.com>

On 10 May 2018 at 19:34, Derrick Stolee <dstolee@microsoft.com> wrote:

Show 25 quoted lines

> During a run of 'git commit-graph verify', list the issues with the
> header information in the commit-graph file. Some of this information
> is inferred from the loaded 'struct commit_graph'. Some header
> information is checked as part of load_commit_graph_one().
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  commit-graph.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index b25aaed128..c3b8716c14 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -818,7 +818,28 @@ void write_commit_graph(const char *obj_dir,
>         oids.nr = 0;
>  }
>
> +static int verify_commit_graph_error;
> +#define graph_report(...) \
> +       do {\
> +               verify_commit_graph_error = 1;\
> +               printf(__VA_ARGS__);\
> +       } while (0);
> +

It seems to me that other users of __VA_ARGS__ are protected with a check for HAVE_VARIADIC_MACROS and provide an alternative non-__VA_ARGS__-implementation. Or maybe I've missed something in my grepping and we are actually (slowly) moving towards assuming __VA_ARGS__ is always available?

Show 18 quoted lines

>  int verify_commit_graph(struct commit_graph *g)
>  {
> -       return !g;
> +       if (!g) {
> +               graph_report(_("no commit-graph file loaded"));
> +               return 1;
> +       }
> +
> +       verify_commit_graph_error = 0;
> +
> +       if (!g->chunk_oid_fanout)
> +               graph_report(_("commit-graph is missing the OID Fanout chunk"));
> +       if (!g->chunk_oid_lookup)
> +               graph_report(_("commit-graph is missing the OID Lookup chunk"));
> +       if (!g->chunk_commit_data)
> +               graph_report(_("commit-graph is missing the Commit Data chunk"));
> +
> +       return verify_commit_graph_error;

If you can't rely on __VA_ARGS__, maybe bite the bullet and introduce braces... The expanded code wouldn't be too horrible, albeit a bit repetitive.

Martin