Re: USB virt 2.6 split driver patch series

[Oleg Goldshmidt <pub@goldshmidt.org>]

harry <harry@hebutterworth.freeserve.co.uk> writes:

> I have read  Documentation/CodingStyle quite carefully and there is no 
> mention of using braces inside functions.  I'm used to using braces to
> define minimal scopes for local variables which makes the code easier to
> read by minimising the number of variables you need to keep track of
> when reading it and by declaring variables closer to where they are used
> so it is easier to verify that they have been correctly initialised.
> 
> Is this really banned?

Not explicitly, but:

1) It increases the number of almost empty lines, which is explicitly
   frowned upon. An opening brace in an empty line by itself is
   allowed in the beginning of a function body, as per K&R style, and
   it is an exception.

2) It deepens block nesting, which is also explicitly frowned upon.

3) CodingStyle explicitly says that the number of local variables in a
   function should be small (over 5-10 and you are in trouble as far
   as both style and design are concerned). This is a corollary to the
   rule that a function should do one thing and one thing only (but do
   it well). If you follow this rule then introducing blocks to limit
   variable scope is never really justified. If you find yourself in a
   situation where you need such blocking then maybe you should think
   of more modularization (at the function level). Disclaimer: I have
   not studied the patches attentively enough to suggest that you need
   this.

Another style comment that I have is that I think that

struct xenidc_channel_struct {
	void (*submit_message)
	 (xenidc_channel * channel, xenidc_channel_message * message);

is inferior to

struct xenidc_channel_struct {
	void (*submit_message)(xenidc_channel * channel,
		               xenidc_channel_message * message);

in terms of readability.

-- 
Oleg Goldshmidt | pub@NOSPAM.goldshmidt.org | http://www.goldshmidt.org