Re: [PATCH 1/5] Introduces for_each_revision() helper

Hi,

On Sat, 28 Apr 2007, Alex Riesen wrote:

Johannes Schindelin, Sat, Apr 28, 2007 04:46:41 +0200:

quoted

quoted

+#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.

quoted

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