From: Johannes Schindelin Date: Sat, 28 Apr 2007 02:46:41 GMT Subject: Re: [PATCH 1/5] Introduces for_each_revision() helper Message-ID: In-Reply-To: <1177693212202-git-send-email-lcapitulino@mandriva.com.br> Hi, On Fri, 27 Apr 2007, Luiz Fernando N. Capitulino wrote: > diff --git a/revision.h b/revision.h > index cdf94ad..7be3fc7 100644 > --- a/revision.h > +++ b/revision.h > @@ -133,4 +133,15 @@ extern void add_object(struct object *obj, > extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name); > extern void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode); > > +/* helpers */ > + > +/** > + * for_each_revision - iterate over revisions > + * @commit: pointer to a commit object returned for each iteration > + * @rev: revision pointer > + */ > +#define for_each_revision(commit, rev) \ > + for (prepare_revision_walk(rev); \ > + (commit = get_revision(rev)) != NULL; ) > + > #endif I object to this, additionally to the magic argument that I agree to, on the grounds that it is actually wrong. The first iteration will work on an _uninitialized_ "commit" variable. Furthermore, it is not like it was a huge piece of code that is being replaced by a shortcut. There are better places to do some libification than this. Ciao, Dscho