From: Johannes Schindelin Date: Sat, 28 Apr 2007 12:52:25 GMT Subject: Re: [PATCH 1/5] Introduces for_each_revision() helper Message-ID: In-Reply-To: <20070428115059.GA4888@steel.home> Hi, On Sat, 28 Apr 2007, Alex Riesen wrote: > Johannes Schindelin, Sat, Apr 28, 2007 04:46:41 +0200: > > > +#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. > > No, it wont. Check it. This code is correct. Yes, sorry, as I admitted in my reply to Junio, there was some serious mental temporary disability involved. > > 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. > > It is not about libification. It is plain readability issue. Look at > what list_for_each_* macros did to the source of Linux kernel. Personally, I find the prepare/get_revision stuff not really too unreadable. Ciao, Dscho