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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]