Re: [PATCH 1/3] KVM: x86 emulator: Fix task switch privilege checks

[Gleb Natapov <gleb@redhat.com>]

On Tue, Jan 24, 2012 at 11:38:24AM +0100, Kevin Wolf wrote:
> Am 24.01.2012 11:17, schrieb Gleb Natapov:
> > On Tue, Jan 24, 2012 at 11:09:09AM +0100, Kevin Wolf wrote:
> >>>> +	} else if (reason != TASK_SWITCH_IRET) {
> >>>> +		dpl = next_tss_desc.dpl;
> >>>>  	}
> >>> No need parentheses around one statement.
> >>
> >> Documentation/CodingStyle says:
> >>
> >> "This does not apply if only one branch of a conditional statement is a
> >> single statement; in the latter case use braces in both branches:"
> >>
> > Then you need to put parentheses around "if (reason != TASK_SWITCH_IRET)"
> > if you want to follow the letter of the CodingStyle :)
> 
> Not sure what you mean. If it is 'else { if (...) { ..." then no, the
> document isn't crazy like that.
> 
The document says:

if (condition) {
        do_this();
        do_that();
} else {
        otherwise();
}

So I do not see how you can interpret it otherwise.

> > But I do not see this coding stile part widely used in core kernel code:
> > $ git grep "} else$" kernel | wc -l
> > 122
> > 
> > Can't think of re to check when the rule is followed :(
> 
> Seem to be at least 77 occurences (git grep -A 2 "} else {" into a file
> as git grep doesn't seem to do multi-line expressions and then on that
> file "} else {\n.*\n.*}$")
> 
> But anyway, I don't really want to discuss the right coding style here
> and I'll apply whatever is considered right. Though if you think that
> checkpatch.pl and Documentation/CodingStyle are both wrong, please get
> them fixed. People might take them serious.
> 
The code looked strange for kernel code. If checkpatch.pl complains about
missing parentheses then they should of course stay. Our interpretation
of CodingStyle is different.

--
			Gleb.