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

[John Keeping]

On Sun, Mar 03, 2013 at 08:56:18PM +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.
> >
> > This also prevents from potential misuse of the global pointers
> > match_path and matched_sha1 after the referenced values have been
> > overwritten on the stack.
> >
> > Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> > ---
> >   ui-blob.c | 42 ++++++++++++++++++++++++++----------------
> >   1 file changed, 26 insertions(+), 16 deletions(-)
> >
> > diff --git a/ui-blob.c b/ui-blob.c
> > index 3d07ce5..4bcbc82 100644
> > --- a/ui-blob.c
> > +++ b/ui-blob.c
> > @@ -11,17 +11,22 @@
> >   #include "html.h"
> >   #include "ui-shared.h"
> >
> > -static char *match_path;
> > -static unsigned char *matched_sha1;
> > -static int found_path;
> > +struct walk_tree_context {
> > +	char *match_path;
> > +	unsigned char *matched_sha1;
> > +	int found_path;
> > +};
> >
> >   static int walk_tree(const unsigned char *sha1, const char *base, int baselen,
> > -	const char *pathname, unsigned mode, int stage, void *cbdata) {
> > -	if(strncmp(base, match_path, baselen)
> > -		|| strcmp(match_path + baselen, pathname))
> > +	const char *pathname, unsigned mode, int stage, void *cbdata)
> > +{
> > +	struct walk_tree_context *walk_tree_ctx = cbdata;
> > +
> > +	if(strncmp(base, walk_tree_ctx->match_path, baselen)
> > +		|| strcmp(walk_tree_ctx->match_path + baselen, pathname))
> >   		return READ_TREE_RECURSIVE;
> > -	memmove(matched_sha1, sha1, 20);
> > -	found_path = 1;
> > +	memmove(walk_tree_ctx->matched_sha1, sha1, 20);
> > +	walk_tree_ctx->found_path = 1;
> >   	return 0;
> >   }
> >
> > @@ -33,16 +38,19 @@ int cgit_print_file(char *path, const char *head)
> >   	unsigned long size;
> >   	struct commit *commit;
> >   	const char *paths[] = {path, NULL};
> > +	struct walk_tree_context walk_tree_ctx = {
> > +		.match_path = path,
> > +		.matched_sha1 = sha1,
> > +		.found_path = 0
> > +	};
> > +
> >   	if (get_sha1(head, sha1))
> >   		return -1;
> >   	type = sha1_object_info(sha1, &size);
> >   	if(type == OBJ_COMMIT && path) {
> >   		commit = lookup_commit_reference(sha1);
> > -		match_path = path;
> > -		matched_sha1 = sha1;
> > -		found_path = 0;
> > -		read_tree_recursive(commit->tree, "", 0, 0, paths, walk_tree, NULL);
> > -		if (!found_path)
> > +		read_tree_recursive(commit->tree, "", 0, 0, paths, walk_tree, &walk_tree_ctx);
> > +		if (!walk_tree_ctx.found_path)
> >   			return -1;
> >   		type = sha1_object_info(sha1, &size);
> >   	}
> > @@ -64,6 +72,10 @@ void cgit_print_blob(const char *hex, char *path, const char *head)
> >   	unsigned long size;
> >   	struct commit *commit;
> >   	const char *paths[] = {path, NULL};
> > +	struct walk_tree_context walk_tree_ctx = {
> > +		.match_path = path,
> > +		.matched_sha1 = sha1,
> 
> forgot to initialise found_path

Unnecessary - C99 6.7.8:

    If there are fewer initializers in a brace-enclosed list than there
    are elements or members of an aggregate, or fewer characters in a
    string literal used to initialize an array of known size than there
    are elements in the array, the remainder of the aggregate shall be
    initialized implicitly the same as objects that have static storage
    duration.

> 
> > +	};
> >
> >   	if (hex) {
> >   		if (get_sha1_hex(hex, sha1)){
> > @@ -81,9 +93,7 @@ void cgit_print_blob(const char *hex, char *path, const char *head)
> >
> >   	if((!hex) && type == OBJ_COMMIT && path) {
> >   		commit = lookup_commit_reference(sha1);
> > -		match_path = path;
> > -		matched_sha1 = sha1;
> > -		read_tree_recursive(commit->tree, "", 0, 0, paths, walk_tree, NULL);
> > +		read_tree_recursive(commit->tree, "", 0, 0, paths, walk_tree, &walk_tree_ctx);
> >   		type = sha1_object_info(sha1,&size);
> >   	}
> >
> >
> 
> -- 
> Ferry Huberts