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

2018-05-10 Thread Martin Ă…gren
On 10 May 2018 at 19:34, Derrick Stolee  wrote:
> 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 
> ---
>  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?

>  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


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

2018-05-10 Thread Derrick Stolee
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 
---
 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);
+
 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;
 }
-- 
2.16.2.329.gfb62395de6