On Sat, Mar 02, 2013 at 08:11:48PM +0100, 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 <c...@cryptocrack.de> > --- > Note that this is completely untested and I don't have time to test it > now since my cgit development environment is down. I decided to submit > nonetheless, so that someone else can have a look.
This looks reasonable to me (module one nit below) and it passes the test suite here. > ui-blob.c | 38 +++++++++++++++++++++++--------------- > 1 file changed, 23 insertions(+), 15 deletions(-) > > diff --git a/ui-blob.c b/ui-blob.c > index 3de4473..b6f01cf 100644 > --- a/ui-blob.c > +++ b/ui-blob.c > @@ -11,17 +11,20 @@ > #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) ) > + struct walk_tree_context *ctx = cbdata; > + if(strncmp(base,ctx->match_path,baselen) > + || strcmp(ctx->match_path+baselen,pathname) ) > return READ_TREE_RECURSIVE; > - memmove(matched_sha1,sha1,20); > - found_path = 1; > + memmove(ctx->matched_sha1,sha1,20); > + ctx->found_path = 1; If we're touching these lines anyway, can we introduce some more whitespace around commas and operators to match the style elsewhere in the file? > return 0; > } > > @@ -40,16 +43,19 @@ int cgit_print_file(char *path, const char *head) > .nr = 1, > .items = &path_items > }; > + 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); > } > @@ -78,6 +84,10 @@ void cgit_print_blob(const char *hex, char *path, const > char *head) > .nr = 1, > .items = &path_items > }; > + struct walk_tree_context walk_tree_ctx = { > + .match_path = path, > + .matched_sha1 = sha1, > + }; > > if (hex) { > if (get_sha1_hex(hex, sha1)){ > @@ -95,9 +105,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); > } > _______________________________________________ cgit mailing list cgit@hjemli.net http://hjemli.net/mailman/listinfo/cgit