[PATCH 3/7] ui-plain.c: Use a context structure in walk_tree()

[Lukas Fleischer]

On Sun, Mar 03, 2013 at 09:04:28PM +0100, Ferry Huberts wrote:
> 
> 
> On 03/03/13 18:06, Lukas Fleischer wrote:
> >Do not misuse global variables to save the context. Instead, use the
> >context pointer which was designed to share information between a
> >read_tree_fn and the caller.
> >
> >Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> >---
> >  ui-plain.c | 31 ++++++++++++++++++-------------
> >  1 file changed, 18 insertions(+), 13 deletions(-)
> >
> >diff --git a/ui-plain.c b/ui-plain.c
> >index 684d5ea..e5addb9 100644
> >--- a/ui-plain.c
> >+++ b/ui-plain.c
> >@@ -11,8 +11,10 @@
> >  #include "html.h"
> >  #include "ui-shared.h"
> >
> >-int match_baselen;
> >-int match;
> >+struct walk_tree_context {
> >+	int match_baselen;
> >+	int match;
> >+};
> >
> >  static char *get_mimetype_from_file(const char *filename, const char *ext)
> >  {
> >@@ -166,18 +168,20 @@ static int walk_tree(const unsigned char *sha1, const char *base, int baselen,
> >  		     const char *pathname, unsigned mode, int stage,
> >  		     void *cbdata)
> >  {
> >-	if (baselen == match_baselen) {
> >+	struct walk_tree_context *walk_tree_ctx = cbdata;
> >+
> >+	if (baselen == walk_tree_ctx->match_baselen) {
> >  		if (S_ISREG(mode)) {
> >  			if (print_object(sha1, pathname))
> >-				match = 1;
> >+				walk_tree_ctx->match = 1;
> >  		} else if (S_ISDIR(mode)) {
> >  			print_dir(sha1, base, baselen, pathname);
> >-			match = 2;
> >+			walk_tree_ctx->match = 2;
> >  			return READ_TREE_RECURSIVE;
> >  		}
> >-	} else if (baselen > match_baselen) {
> >+	} else if (baselen > walk_tree_ctx->match_baselen) {
> >  		print_dir_entry(sha1, base, baselen, pathname, mode);
> >-		match = 2;
> >+		walk_tree_ctx->match = 2;
> >  	} else if (S_ISDIR(mode)) {
> >  		return READ_TREE_RECURSIVE;
> >  	}
> >@@ -199,6 +203,7 @@ void cgit_print_plain(struct cgit_context *ctx)
> >  	unsigned char sha1[20];
> >  	struct commit *commit;
> >  	const char *paths[] = {ctx->qry.path, NULL};
> >+	struct walk_tree_context walk_tree_ctx;
> 
> I'd prefer that you initialise the variable here

walk_tree_ctx.match_baselen cannot be initialized here, and setting a
default value doesn't really make sense if it is overwritten in each
code path later.

Setting walk_tree_ctx.match to 0 might make sense, though. I will add
that and resubmit. Thanks!

> 
> >
> >  	if (!rev)
> >  		rev = ctx->qry.head;
> >@@ -214,15 +219,15 @@ void cgit_print_plain(struct cgit_context *ctx)
> >  	}
> >  	if (!paths[0]) {
> >  		paths[0] = "";
> >-		match_baselen = -1;
> >+		walk_tree_ctx.match_baselen = -1;
> >  		print_dir(commit->tree->object.sha1, "", 0, "");
> >-		match = 2;
> >+		walk_tree_ctx.match = 2;
> >  	}
> >  	else
> >-		match_baselen = basedir_len(paths[0]);
> >-	read_tree_recursive(commit->tree, "", 0, 0, paths, walk_tree, NULL);
> >-	if (!match)
> >+		walk_tree_ctx.match_baselen = basedir_len(paths[0]);
> >+	read_tree_recursive(commit->tree, "", 0, 0, paths, walk_tree, &walk_tree_ctx);
> >+	if (!walk_tree_ctx.match)
> >  		html_status(404, "Not found", 0);
> >-	else if (match == 2)
> >+	else if (walk_tree_ctx.match == 2)
> >  		print_dir_tail();
> >  }
> >
> 
> -- 
> Ferry Huberts