lkml.org 
[lkml]   [2006]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2006-09-05 13:31    [W:0.063 / U:1.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site