On Sun, Feb 24, 2008 at 07:08:39PM -0800, Junio C Hamano wrote:
> > diff --git a/Makefile b/Makefile
> > index a9b5a67..3b356f8 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -303,7 +303,7 @@ LIB_H = \
> > run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
> > tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
> > utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \
> > - mailmap.h remote.h parse-options.h transport.h diffcore.h hash.h ll-merge.h
> > + mailmap.h remote.h parse-options.h transport.h diffcore.h hash.h ll-merge.h fsck.h
>
> I'd rather see a series does not depend on things in next that
> you do not have to depend on, pretty please?
I usually develop my patch on next. I can offer you two things:
* base my patches on something different (master?)
* add fsck.h/o some lines above
What do you prefer?
> > +static int fsck_walk_commit(struct commit *commit, fsck_walk_func walk, void *data)
> > +{
> > + struct commit_list *parents = commit->parents;
> > + int result;
> > +
> > + if(parse_commit(commit))
> > + return -1;
> > +
> > + result = walk((struct object*)commit->tree, OBJ_TREE, data);
> > + if (result)
> > + return result;
> > +
> > + while (parents) {
> > + result = walk((struct object*)parents->item, OBJ_COMMIT, data);
> > + if (result)
> > + return result;
> > + parents = parents->next;
> > + }
> > + return 0;
> > +}
>
> Hmm. For the purpose of proving there is _no_ error (or an
> error or more), it would be Ok to return early like this, but
> won't there be cases where you would want to get as many
> coverage as possible?
>
> For example, I do not think you can use this to mark reachable
> objects. Even if you find error walking the first parent
> history, you would want to still mark a healthy second parent
> history reachable.
How should I define the return value of fsck_walk in the presence of
multiple errors?
It would not be necessary for all my users:
* in unpack-object and index-pack (I'll send an updated patch in the
next days), any error means that we can abort. Further checking would
mean wasting of resources.
* in fsck (patch 2) the error is signaled by the errors_found variable, so
all callbacks can return 0, even in the case of an error. Checking the
return value of fsck_walk would mean duplicate error messages.
mfg Martin Kögler
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
