Re: [GIT PULL] percpu fixes for 2.6.32-rc6

[Ingo Molnar]

* Tejun Heo <tj@xxxxxxxxxx> wrote:

> This really depends on personal tastes.  When trying to use long 
> string literals, there are several choices.
> 
> 1. Use broken strings.
> 
> 				printk("blah blah blah blah "
> 				       "blah blah blah blah\n");
> 
> 2. Push it into new line and unindent it.
> 
> 				printk(
> 	"blah blah blah blah blah blah blah blah\n");
> 
> 3. Restructure code so that the literal ends up in outer block.
> 
> 	printk("blah blah blah blah blah blah blah blah\n");
> 
> I prefer the first choice.  The third would be nice if it's trivial to 
> do but I don't think it should dictate the code structure.  The second 
> one, I don't know.  Some people like that and grep will be happy with 
> it but it just seems very disturbing to my eyes.

My preferred choice is:

  4. Change "blah blah blah blah blah blah blah blah\n" to "blah\n" - 
     the user really does not win much from the repetition.

Seriously, if you _ever_ get into a 'hm, the string is too long' 
situation, you should ask yourself two fundamental questions:

  a) Is the user really interested in this small novel?

  b) Is this a side-effect of a bloated function having too deep
     indentation?

IMHO, in the specific case at hand, both a) and b) apply:

                                err = "failed to extend area map of "
                                        "reserved chunk";

the indentation is a bit too deep, and the message is too chatty - the 
output itself is:

 PERCPU: allocation failed, size=1024 align=32, failed to extend area map of reserved chunk

A better/shorter message would be:

 percpu: 1024 bytes allocation failed: could not extend reserved chunk area map

This formulation is a bit shorter and i doubt the align parameter really 
needs printing - it's almost always the same and other context will tell 
us what it is anyway.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/