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 <[email protected]>
> >---
> > 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
_______________________________________________
cgit mailing list
[email protected]
http://hjemli.net/mailman/listinfo/cgit
