lkml.org 
[lkml]   [2011]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] x86-64: use relative 32-bit pointers in exception tables

* Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 17.02.11 at 18:25, Ingo Molnar <mingo@elte.hu> wrote:
>
> > Nice patch. I've got a really small code readability nitpick:
> >
> >> +#ifndef ex_insn /* until all architectures have this accessor */
> >> +#define ex_insn(x) (x)->insn
> >> +#endif
> >
> >> +#else
> >> +#define swap_ex NULL
> >> +#endif
> >
> > In the x86 architecture we tend to write this as:
> >
> >> +#else
> >> +# define swap_ex NULL
> >> +#endif
> >
> > So that the conditional structure stands out more, visually. (There might be
> > more
> > such cases in these patches as well.)
>
> I can certainly fix this, but got a comment from (I think) Andrew
> Morton to do exactly the opposite quite some time ago, with the
> rationale that this indentation leads to more involved patches
> when further conditionals get added around them.

Well, the patch impact argument is a valid concern, but by that logic we should also
drop the visual structure of other conditionals such as:

if (x) {
if (y)
z;
else
k;
} else {
l;
}

and write:

if (x) {
if (y)
z;
else
k;
} else {
l;
}

? I don't think so.

There might be other cases where marking CPP code this way looks ugly but in this
patch it's clearly helpful to readability.

So i think for consistency's (and eyeball health's) sake lets bring as much
meaningful geometric structure into source code as possible. Future patch size
worries are secondary IMO.

Thanks,

Ingo


\
 
 \ /
  Last update: 2011-02-18 09:17    [W:0.056 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site