[PATCH] intel-rng: Undo mess made by an 80 column extremist

[PATCH] intel-rng: Undo mess made by an 80 column extremist

[Alan Cox]

The intel-rng printed a nice well formatted message when the port was
disabled. Someone then came along and blindly trashed it by screwing up a
trim down to 80 columns.

Put it back into the right format and keep the overlong lines as the
result is also MUCH easier to read in this specific case.

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.22-rc4-mm2/drivers/char/hw_random/intel-rng.c linux-2.6.22-rc4-mm2/drivers/char/hw_random/intel-rng.c
--- linux.vanilla-2.6.22-rc4-mm2/drivers/char/hw_random/intel-rng.c	2007-06-07 14:25:57.000000000 +0100
+++ linux-2.6.22-rc4-mm2/drivers/char/hw_random/intel-rng.c	2007-06-07 14:32:36.000000000 +0100
@@ -296,12 +296,10 @@
 	     (BIOS_CNTL_LOCK_ENABLE_MASK|BIOS_CNTL_WRITE_ENABLE_MASK))
 	    == BIOS_CNTL_LOCK_ENABLE_MASK) {
 		static __initdata /*const*/ char warning[] =
-			KERN_WARNING PFX "Firmware space is locked read-only. "
-			KERN_WARNING PFX "If you can't or\n don't want to "
-			KERN_WARNING PFX "disable this in firmware setup, and "
-			KERN_WARNING PFX "if\n you are certain that your "
-			KERN_WARNING PFX "system has a functional\n RNG, try"
-			KERN_WARNING PFX "using the 'no_fwh_detect' option.\n";
+			KERN_WARNING PFX "Firmware space is locked read-only. If you can't or\n"
+			KERN_WARNING PFX "don't want to disable this in firmware setup, and if\n"
+			KERN_WARNING PFX "you are certain that your system has a functional\n"
+			KERN_WARNING PFX "RNG, try using the 'no_fwh_detect' option.\n";
 
 		if (no_fwh_detect)
 			return -ENODEV;

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Jeff Garzik]

On Thu, Jun 07, 2007 at 04:22:51PM +0100, Alan Cox wrote:
> The intel-rng printed a nice well formatted message when the port was
> disabled. Someone then came along and blindly trashed it by screwing up a
> trim down to 80 columns.
> 
> Put it back into the right format and keep the overlong lines as the
> result is also MUCH easier to read in this specific case.
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

Acked-by: Jeff Garzik <jeff@garzik.org>

I'm not the driver maintainer anymore, so I'm not actually going to push
this one.

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Alan Cox]

On Thu, 7 Jun 2007 11:23:08 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> On Thu, Jun 07, 2007 at 04:22:51PM +0100, Alan Cox wrote:
> > The intel-rng printed a nice well formatted message when the port was
> > disabled. Someone then came along and blindly trashed it by screwing up a
> > trim down to 80 columns.
> > 
> > Put it back into the right format and keep the overlong lines as the
> > result is also MUCH easier to read in this specific case.
> > 
> > Signed-off-by: Alan Cox <alan@redhat.com>
> 
> Acked-by: Jeff Garzik <jeff@garzik.org>
> 
> I'm not the driver maintainer anymore, so I'm not actually going to push
> this one.

Then please send a patch for the MAINTAINERS file

Alan

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Jeff Garzik]

On Thu, Jun 07, 2007 at 04:44:09PM +0100, Alan Cox wrote:
> On Thu, 7 Jun 2007 11:23:08 -0400
> Jeff Garzik <jeff@garzik.org> wrote:
> 
> > On Thu, Jun 07, 2007 at 04:22:51PM +0100, Alan Cox wrote:
> > > The intel-rng printed a nice well formatted message when the port was
> > > disabled. Someone then came along and blindly trashed it by screwing up a
> > > trim down to 80 columns.
> > > 
> > > Put it back into the right format and keep the overlong lines as the
> > > result is also MUCH easier to read in this specific case.
> > > 
> > > Signed-off-by: Alan Cox <alan@redhat.com>
> > 
> > Acked-by: Jeff Garzik <jeff@garzik.org>
> > 
> > I'm not the driver maintainer anymore, so I'm not actually going to push
> > this one.
> 
> Then please send a patch for the MAINTAINERS file

hrm, I thought that had already been fixed, by the new maintainer.

Will do.

	Jeff

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Krzysztof Halasa]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

> The intel-rng printed a nice well formatted message when the port was
> disabled. Someone then came along and blindly trashed it by screwing up a
> trim down to 80 columns.

Perhaps we should drop that 80-column style and use some 120+?
X or no X, almost all people now have more lines and columns on their
displays than MDA 20 years ago.

132 to match text VGA perhaps?
80 can be left for the actual _output_, I mean number of chars printed
by kernel code.
-- 
Krzysztof Halasa

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Alistair John Strachan]

On Thursday 07 June 2007 22:17:18 Krzysztof Halasa wrote:
> Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> > The intel-rng printed a nice well formatted message when the port was
> > disabled. Someone then came along and blindly trashed it by screwing up a
> > trim down to 80 columns.
>
> Perhaps we should drop that 80-column style and use some 120+?
> X or no X, almost all people now have more lines and columns on their
> displays than MDA 20 years ago.
>
> 132 to match text VGA perhaps?
> 80 can be left for the actual _output_, I mean number of chars printed
> by kernel code.

I personally buy the argument that 80 cols helps remind people that they've 
used too many indentation depths and should redesign their code. I think it's 
a good thing to stick to where possible, even if just from a design 
perspective.

There are some cases such as the one Alan has pointed out here where being 
strictly 80 cols seems more destructive than useful, but those are the 
exception, not the rule (IMO).

-- 
Cheers,
Alistair.

Final year Computer Science undergraduate.
1F2 55 South Clerk Street, Edinburgh, UK.

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Krzysztof Halasa]

Alistair John Strachan <s0348365@sms.ed.ac.uk> writes:

> I personally buy the argument that 80 cols helps remind people that they've 
> used too many indentation depths and should redesign their code.
> I think it's 
> a good thing to stick to where possible, even if just from a design 
> perspective.

How many is too many? A function with 2 "if" means 3 tabs = 24 characters
for just indentation. Add a printk(KERN_DEBUG "\n"); and you can print
ca. 30 characters without having to break the string. This is crazy.

> There are some cases such as the one Alan has pointed out here where being 
> strictly 80 cols seems more destructive than useful, but those are the 
> exception, not the rule (IMO).

Well... I was forced to split functions making the code less readable
and more complicated just to avoid these 80-columns. A short function
with more indent levels may be way more readable than a redesigned one
with this requirement in mind.

I'd just state the code has to be readable and laid out with some sanity
but 80 columns give us nothing WRT this.

We already review new code and spot such problems (and smaller ones)
without using the 80-cols rule.
-- 
Krzysztof Halasa

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[H. Peter Anvin]

Krzysztof Halasa wrote:
> Alistair John Strachan <s0348365@sms.ed.ac.uk> writes:
> 
>> I personally buy the argument that 80 cols helps remind people that they've 
>> used too many indentation depths and should redesign their code.
>> I think it's 
>> a good thing to stick to where possible, even if just from a design 
>> perspective.
> 
> How many is too many? A function with 2 "if" means 3 tabs = 24 characters
> for just indentation. Add a printk(KERN_DEBUG "\n"); and you can print
> ca. 30 characters without having to break the string. This is crazy.
> 

My big concern with the 80-column rule is that it discourages commenting.

	-hpa

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Jeff Garzik]

On Thu, Jun 07, 2007 at 06:08:32PM -0700, H. Peter Anvin wrote:
> My big concern with the 80-column rule is that it discourages commenting.

My concern with that logic is that encourages random, super-wide code
lines that varies with each coder.  You are left to the mercy of he with
the widest text window.

The 80-column rule is good as a general guideline, though there are
obvious exceptions.  Comments IMO are not one of them.  That rapidly
creates unreadable code.

	Jeff

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Peter Zijlstra]

On Thu, 2007-06-07 at 21:25 -0400, Jeff Garzik wrote:
> On Thu, Jun 07, 2007 at 06:08:32PM -0700, H. Peter Anvin wrote:
> > My big concern with the 80-column rule is that it discourages commenting.
> 
> My concern with that logic is that encourages random, super-wide code
> lines that varies with each coder.  You are left to the mercy of he with
> the widest text window.

I just counted, I can get 380 chars on 1 screen, if I let it straddle
the two monitors I have then I can get 760 chars.

> The 80-column rule is good as a general guideline, though there are
> obvious exceptions.  Comments IMO are not one of them.  That rapidly
> creates unreadable code.

I agree, its easier on the eyes to have dense columns of text to read
than having to scan all over the place. More so, its more economical of
screen-estate as well, a few long lines take up valuable space which
could otherwise have been used to create more columns (yes, from the guy
sitting behind the two 24" screen).

I usually work with 4 columns side by side per screen, that gives me
about 92x110 columns. Its easy on they eye (it need not wander all over
the place to read stuff) and it can handle the occasional line that
exceeds 80 chars (they never exceed very far - yes there are
exceptions :-( )

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Krzysztof Halasa]

Peter Zijlstra <peterz@infradead.org> writes:

> I agree, its easier on the eyes to have dense columns of text to read
> than having to scan all over the place.

Sure, though I don't propose writing 130 chars in each line. Tabs
don't need much scanning.
-- 
Krzysztof Halasa

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Daniel Hazelton]

On Thursday 07 June 2007 17:17:18 Krzysztof Halasa wrote:
> Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> > The intel-rng printed a nice well formatted message when the port was
> > disabled. Someone then came along and blindly trashed it by screwing up a
> > trim down to 80 columns.
>
> Perhaps we should drop that 80-column style and use some 120+?
> X or no X, almost all people now have more lines and columns on their
> displays than MDA 20 years ago.
>
> 132 to match text VGA perhaps?
> 80 can be left for the actual _output_, I mean number of chars printed
> by kernel code.

Why? My consoles are *all* still 80x24 text mode. It's only if I decide to 
monkey with the settings (and why fix what isn't broken?) or when I'm in X 
that I get a bigger screen than that.

I think the general consensus on the 80 character lines isn't for the average 
home user, but for the people that have things like old Wyse terminals and 
such hooked up.

DRH

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Krzysztof Halasa]

Daniel Hazelton <dhazelton@enter.net> writes:

> Why? My consoles are *all* still 80x24 text mode. It's only if I decide to 
> monkey with the settings (and why fix what isn't broken?) or when I'm in X 
> that I get a bigger screen than that.
>
> I think the general consensus on the 80 character lines isn't for the average 
> home user, but for the people that have things like old Wyse terminals and 
> such hooked up.

Of course! But I don't write code using the system console nor old
Wyse etc. serial terminal and I suspect (almost?) nobody does that.

Do you?

Number of characters printed by the kernel (printk()) - sure, still
80 chars to suid everybody (and yes, printk messages show up on system
consoles).
-- 
Krzysztof Halasa

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Daniel Hazelton]

On Thursday 07 June 2007 18:37:45 Krzysztof Halasa wrote:
> Daniel Hazelton <dhazelton@enter.net> writes:
> > Why? My consoles are *all* still 80x24 text mode. It's only if I decide
> > to monkey with the settings (and why fix what isn't broken?) or when I'm
> > in X that I get a bigger screen than that.
> >
> > I think the general consensus on the 80 character lines isn't for the
> > average home user, but for the people that have things like old Wyse
> > terminals and such hooked up.
>
> Of course! But I don't write code using the system console nor old
> Wyse etc. serial terminal and I suspect (almost?) nobody does that.
>
> Do you?

No, I don't use a Wyse terminal, but I do a lot of my coding without X 
running. I don't know why, but it might be because I can then take off my 
glasses and not have to worry about the characters getting too blurred to 
read.


DRH

> Number of characters printed by the kernel (printk()) - sure, still
> 80 chars to suid everybody (and yes, printk messages show up on system
> consoles).

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Jesper Juhl]

On 08/06/07, Daniel Hazelton <dhazelton@enter.net> wrote:
> On Thursday 07 June 2007 17:17:18 Krzysztof Halasa wrote:
> > Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> > > The intel-rng printed a nice well formatted message when the port was
> > > disabled. Someone then came along and blindly trashed it by screwing up a
> > > trim down to 80 columns.
> >
> > Perhaps we should drop that 80-column style and use some 120+?
> > X or no X, almost all people now have more lines and columns on their
> > displays than MDA 20 years ago.
> >
> > 132 to match text VGA perhaps?
> > 80 can be left for the actual _output_, I mean number of chars printed
> > by kernel code.
>
> Why? My consoles are *all* still 80x24 text mode. It's only if I decide to
> monkey with the settings (and why fix what isn't broken?) or when I'm in X
> that I get a bigger screen than that.
>
> I think the general consensus on the 80 character lines isn't for the average
> home user, but for the people that have things like old Wyse terminals and
> such hooked up.
>

Hmm, perhaps my eyes are just not as good as other peoples, but my X
runs at 1600x1200 and I have konsole in KDE configured so that when it
is maximized it is very close to a 80x25 window (actually it is 82x31
with a size 18 font).
Nice and readable, if I make the font any smaller to fit more
cols/rows then the text gets too small and my eyes hurt.
Reading kernel code formatted for 80cols fits perfectly for me.


-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Daniel Hazelton]

On Thursday 07 June 2007 18:46:25 Jesper Juhl wrote:
> On 08/06/07, Daniel Hazelton <dhazelton@enter.net> wrote:
> > On Thursday 07 June 2007 17:17:18 Krzysztof Halasa wrote:
> > > Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> > > > The intel-rng printed a nice well formatted message when the port was
> > > > disabled. Someone then came along and blindly trashed it by screwing
> > > > up a trim down to 80 columns.
> > >
> > > Perhaps we should drop that 80-column style and use some 120+?
> > > X or no X, almost all people now have more lines and columns on their
> > > displays than MDA 20 years ago.
> > >
> > > 132 to match text VGA perhaps?
> > > 80 can be left for the actual _output_, I mean number of chars printed
> > > by kernel code.
> >
> > Why? My consoles are *all* still 80x24 text mode. It's only if I decide
> > to monkey with the settings (and why fix what isn't broken?) or when I'm
> > in X that I get a bigger screen than that.
> >
> > I think the general consensus on the 80 character lines isn't for the
> > average home user, but for the people that have things like old Wyse
> > terminals and such hooked up.
>
> Hmm, perhaps my eyes are just not as good as other peoples, but my X
> runs at 1600x1200 and I have konsole in KDE configured so that when it
> is maximized it is very close to a 80x25 window (actually it is 82x31
> with a size 18 font).
> Nice and readable, if I make the font any smaller to fit more
> cols/rows then the text gets too small and my eyes hurt.
> Reading kernel code formatted for 80cols fits perfectly for me.

I know the feeling Jesper. There are times when I like to take my glasses off 
and set them to the side. When I do that I usually also kill X and work using 
nothing more than the console. The why is simple - my eyes are just that bad. 
If I stay in X and try to use it I wind up with a massive headache after only 
thirty minutes or so.

DRH

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[John Stoffel]

Krzysztof> Perhaps we should drop that 80-column style and use some
Krzysztof> 120+?  X or no X, almost all people now have more lines and
Krzysztof> columns on their displays than MDA 20 years ago.

I don't.  I use a pair of 80x48 xterms or emacs windows side by side
on my monitor with a nice big clear easy to read font.  Itty bitty
fonts are a total pain...

John

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Jeff Garzik]

On Thu, Jun 07, 2007 at 09:44:57PM -0400, John Stoffel wrote:
> I don't.  I use a pair of 80x48 xterms or emacs windows side by side
> on my monitor with a nice big clear easy to read font.  Itty bitty

That's pretty much what I do.  For me at least, it's a more efficient
use of screen real estate, and of course the overwhelming majority of
source code looks great in an 80-column text window.

	Jeff

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[John Stoffel]

Jeff> On Thu, Jun 07, 2007 at 09:44:57PM -0400, John Stoffel wrote:
>> I don't.  I use a pair of 80x48 xterms or emacs windows side by side
>> on my monitor with a nice big clear easy to read font.  Itty bitty

Jeff> That's pretty much what I do.  For me at least, it's a more
Jeff> efficient use of screen real estate, and of course the
Jeff> overwhelming majority of source code looks great in an 80-column
Jeff> text window.

Thinking about it more, I wonder if Krysztof is bitching more about
the tab width of 8 characters?   I know that it ticks me off,
indenting by two spaces is plenty for me to follow the flow, along
with Emacs and braces closure indication.  

I'm constantly un-tabifying code and making my indents be just two
spaces, though I guess I should really just change my tab-width to be
2 consistently.

What do you think Krysztof?

John

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Jeff Garzik]

On Thu, Jun 07, 2007 at 09:56:06PM -0400, John Stoffel wrote:
> Thinking about it more, I wonder if Krysztof is bitching more about
> the tab width of 8 characters?   I know that it ticks me off,

Even if he is, _that_ is definitely not getting changed.

That was the very first item Linus wrote in Documentation/CodingStyle,
and for good reasons.

If code starts creeping way right due to indentation levels, create a
new function.

	Jeff

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[John Stoffel]

Jeff> On Thu, Jun 07, 2007 at 09:56:06PM -0400, John Stoffel wrote:
>> Thinking about it more, I wonder if Krysztof is bitching more about
>> the tab width of 8 characters?   I know that it ticks me off,

Jeff> Even if he is, _that_ is definitely not getting changed.

Oh sure... I know that part is written in stone.  

Jeff> That was the very first item Linus wrote in
Jeff> Documentation/CodingStyle, and for good reasons.

I could argue that 4 spaces would be a better level, but it's a
personal preference and not worth the battle.  

Jeff> If code starts creeping way right due to indentation levels,
Jeff> create a new function.

Sure... compilers are good, us humans haven't gotten much better, make
it easier on us and harder on the computer.   

John

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Ingo Oeser]

On Friday 08 June 2007, John Stoffel wrote:
> Jeff> On Thu, Jun 07, 2007 at 09:56:06PM -0400, John Stoffel wrote:
> >> Thinking about it more, I wonder if Krysztof is bitching more about
> >> the tab width of 8 characters?   I know that it ticks me off,
> 
> Jeff> Even if he is, _that_ is definitely not getting changed.
> 
> Oh sure... I know that part is written in stone.  

Yes, and as a person doing Linux code review for 12 years now,
I'm really thankful for it. 8 char tab, 80 column rule and 25-50 lines
of code per function actually enable effective review of code snippets.
Because you can see more code flow per patch.

And enables high code reuse. If you can get within 1-5min, 
what a functions does and match it with your actually 
written down last 20 code lines, you just reuse it more often.

If you have more to choose from, you reuse naturally. 
Personally I find best candidates by code position in tree and 
function signature.

> Jeff> If code starts creeping way right due to indentation levels,
> Jeff> create a new function.
> 
> Sure... compilers are good, us humans haven't gotten much better, make
> it easier on us and harder on the computer.   

Yes, let compile remove all the abstraction overhead. 
GCC does a pretty good job there, I think.

I recently analyzed some code and it took much, much longer (factor 2-3), 
because of laxer coding rules similiar to the ones you suggest.

I even asked the developers, who wrote that code and to ones who work 
daily with that code base and they had the same problems. They all couldn't 
explain the "Why?" only the "How?". Not to mention, that this was a core 
component.

After refactoring some big messy parts into smaller functions,
identical, missing, unhandled cases became visible, inappropriate usages
were identified and even some loops could be removed.

Now try to find such problems within Linux. They should be a small percentage 
and not within core components.

So a big THANKS to all the code cops here: You actually make the 
	damn fast change rate of Linux possible by keeping the base clean 
	and neat.

Best Regards

Ingo oeser

Re: [PATCH] intel-rng: Undo mess made by an 80 column extremist

[Krzysztof Halasa]

"John Stoffel" <john@stoffel.org> writes:

> Thinking about it more, I wonder if Krysztof is bitching more about
> the tab width of 8 characters?   I know that it ticks me off,
> indenting by two spaces is plenty for me to follow the flow, along
> with Emacs and braces closure indication.

Well, 8 might be too long but I wouldn't be able to read the code
with 2-chars tabs. Even on a large wall display.

4 chars, maybe, but I'm not sure. 8 chars is great except that the
lines are too short.

Obviously it's the space taken by indentation, the "real" line
length of ~ 70 chars (such as in this msg) is ok.
-- 
Krzysztof Halasa