Messages in this thread | | | Date | Tue, 5 Sep 2006 13:19:26 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 07/16] GFS2: Directory handling |
| |
* Jan Engelhardt <jengelh@linux01.gwdg.de> wrote:
> >> > >+ if ((char *)cur + cur_rec_len >= bh_end) { > >> > >+ if ((char *)cur + cur_rec_len > bh_end) { > >> > >+ gfs2_consist_inode(dip); > >> > >+ return -EIO; > >> > >+ } > >> > >+ return -ENOENT; > >> > >+ } > >> > > >> > if((char *)cur + cur_rec_len > bh_end) { > >> > gfs2_consist_inode(dip); > >> > return -EIO; > >> > } else if((char *)cur + cur_rec_len == bh_end) > >> > return -ENOENT; > >> > > >> ok > > > >this one is not OK! Firstly, Jan, and i mentioned this before, please > >stop using 'if(', it is highly inconsistent and against basic taste. We > >only use this construct for function calls (and macros), not for C > >statements. > > Now there is no rule in CodingStyle for this yet. [...]
as i told you before, it is basic taste. Show me the CodingStyle rules that forbids this construct:
if((((i+-+-+-+-+1-1)))) var=0;;;;;;;
instead of:
if (i) var = 0;
?
You will find no explicit rule in CodingStyle that explicitly forbids any aspect of this line, because CodingStyle mainly concentrates on the harder to follow rules. So please use common sense in combination with CodingStyle.
(And even if you dont agree with the taste, "if ( )" is done by 98% of core code, so for the sake of consistency it's the thing to follow.)
> 11:17 gwdg-wb04A:~/linux > grep -Pri '(if|for|while)\(' . | wc -l > 24242 > > [To be honest, I also present the other number:] > 11:17 gwdg-wb04A:~/linux > grep -Pri '(if|for|while) \(' . | wc -l > 380895 > > Although a minority, it does not seem so uncommon.
What is your point?
ugly code is not at all uncommon, sadly - but i'm glad that it's only 6% in this particular case (and 1.9% for core kernel code).
> [...] Plus, I was wanting to show how to reorder the construct, so > change in whitespace between "if" and "(" is outoftopic.
it is not offtopic, because you _are_ nitpicking over style issues (which is OK and desirable, if not overdone), while still seeming to believe that "if(" is fine :-) The one who judges others is judged by higher standards. In other words, i'm trying to fix the guy who is fixing others ;-)
Ingo - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |