[PATCH] x86: boot: Fix mixed indentation in a20.c

[PATCH] x86: boot: Fix mixed indentation in a20.c

[Johannes Löthberg]

Replace all mixed indentation with tabs

Signed-off-by: Johannes Löthberg <johannes@kyriasis.com>
---
 arch/x86/boot/a20.c | 62 ++++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
index 64a31a6..c837ac6 100644
--- a/arch/x86/boot/a20.c
+++ b/arch/x86/boot/a20.c
@@ -129,37 +129,37 @@ static void enable_a20_fast(void)
 
 int enable_a20(void)
 {
-       int loops = A20_ENABLE_LOOPS;
-       int kbc_err;
+	int loops = A20_ENABLE_LOOPS;
+	int kbc_err;
 
-       while (loops--) {
-	       /* First, check to see if A20 is already enabled
+	while (loops--) {
+		/* First, check to see if A20 is already enabled
 		  (legacy free, etc.) */
-	       if (a20_test_short())
-		       return 0;
-	       
-	       /* Next, try the BIOS (INT 0x15, AX=0x2401) */
-	       enable_a20_bios();
-	       if (a20_test_short())
-		       return 0;
-	       
-	       /* Try enabling A20 through the keyboard controller */
-	       kbc_err = empty_8042();
-
-	       if (a20_test_short())
-		       return 0; /* BIOS worked, but with delayed reaction */
-	
-	       if (!kbc_err) {
-		       enable_a20_kbc();
-		       if (a20_test_long())
-			       return 0;
-	       }
-	       
-	       /* Finally, try enabling the "fast A20 gate" */
-	       enable_a20_fast();
-	       if (a20_test_long())
-		       return 0;
-       }
-       
-       return -1;
+		if (a20_test_short())
+			return 0;
+
+		/* Next, try the BIOS (INT 0x15, AX=0x2401) */
+		enable_a20_bios();
+		if (a20_test_short())
+			return 0;
+
+		/* Try enabling A20 through the keyboard controller */
+		kbc_err = empty_8042();
+
+		if (a20_test_short())
+			return 0; /* BIOS worked, but with delayed reaction */
+
+		if (!kbc_err) {
+			enable_a20_kbc();
+			if (a20_test_long())
+				return 0;
+		}
+
+		/* Finally, try enabling the "fast A20 gate" */
+		enable_a20_fast();
+		if (a20_test_long())
+			return 0;
+	}
+
+	return -1;
 }
-- 
1.8.4.2

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[H. Peter Anvin]

On 11/18/2013 09:50 AM, Johannes Löthberg wrote:
> Replace all mixed indentation with tabs
> 
> Signed-off-by: Johannes Löthberg <johannes@kyriasis.com>

NAK.  Not worth the churn in the absence of other changes.

	-hpa

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 11/18/2013 09:50 AM, Johannes Löthberg wrote:
> > Replace all mixed indentation with tabs
> > 
> > Signed-off-by: Johannes Löthberg <johannes@kyriasis.com>
> 
> NAK.  Not worth the churn in the absence of other changes.

Yes.

If a20.c was cleaned up altogether, that might be a more tempting 
change, it has a good number of inconsistencies and improving it all 
would make it more readable.
 
But it makes little sense to do an incomplete cleanup.

Thanks,

	Ingo

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Borislav Petkov]

On Tue, Nov 19, 2013 at 08:22:41AM +0100, Ingo Molnar wrote:
> 
> * H. Peter Anvin <hpa@zytor.com> wrote:
> 
> > On 11/18/2013 09:50 AM, Johannes Löthberg wrote:
> > > Replace all mixed indentation with tabs
> > > 
> > > Signed-off-by: Johannes Löthberg <johannes@kyriasis.com>
> > 
> > NAK.  Not worth the churn in the absence of other changes.
> 
> Yes.
> 
> If a20.c was cleaned up altogether, that might be a more tempting 
> change, it has a good number of inconsistencies and improving it all 
> would make it more readable.
>  
> But it makes little sense to do an incomplete cleanup.

Yeah, senseless cleanup without any benefit whatsoever (yeah, fuck
readability! :-)) should be forbidden, if you ask me. Especially
since it dilutes and muds up git history and finding the patch which
introduced a change turns into a serious undertaking.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Joe Perches]

On Tue, 2013-11-19 at 11:34 +0100, Borislav Petkov wrote:
> On Tue, Nov 19, 2013 at 08:22:41AM +0100, Ingo Molnar wrote:
> > * H. Peter Anvin <hpa@zytor.com> wrote:
> > > On 11/18/2013 09:50 AM, Johannes Löthberg wrote:
> > > > Replace all mixed indentation with tabs
> > > > Signed-off-by: Johannes Löthberg <johannes@kyriasis.com>
> > > NAK.  Not worth the churn in the absence of other changes.
> > Yes.
> > If a20.c was cleaned up altogether, that might be a more tempting 
> > change, it has a good number of inconsistencies and improving it all 
> > would make it more readable.
> >  
> > But it makes little sense to do an incomplete cleanup.
> 
> Yeah, senseless cleanup without any benefit whatsoever (yeah, fuck
> readability! :-)) should be forbidden, if you ask me. Especially
> since it dilutes and muds up git history and finding the patch which
> introduced a change turns into a serious undertaking.

just fyi:

git diff -w and git blame -w both ignore whitespace changes
and can help in tracking down actual modifications.

If a20.c was to be modified for whatever reason, maybe this
below would be OK, but I too think it's not particularly
worthwhile as the file hasn't had or needed a significant
change in quite awhile.

This is sent as an example for Johannes, who it seems hasn't
ever submitted a patch before, more than to be applied.

Changes include:

o whitespace space->tab conversions
o spacing around arithmetic
o comment style neatening

Unsigned...

---

 arch/x86/boot/a20.c | 97 +++++++++++++++++++++++++++++------------------------
 1 file changed, 53 insertions(+), 44 deletions(-)

diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
index 64a31a6..e1dec7e 100644
--- a/arch/x86/boot/a20.c
+++ b/arch/x86/boot/a20.c
@@ -22,7 +22,7 @@ static int empty_8042(void)
 {
 	u8 status;
 	int loops = MAX_8042_LOOPS;
-	int ffs   = MAX_8042_FF;
+	int ffs = MAX_8042_FF;
 
 	while (loops--) {
 		io_delay();
@@ -46,17 +46,21 @@ static int empty_8042(void)
 	return -1;
 }
 
-/* Returns nonzero if the A20 line is enabled.  The memory address
-   used as a test is the int $0x80 vector, which should be safe. */
+/*
+ * Returns nonzero if the A20 line is enabled.
+ * The memory address used as a test is the int $0x80 vector,
+ * which should be safe.
+ */
 
-#define A20_TEST_ADDR	(4*0x80)
-#define A20_TEST_SHORT  32
-#define A20_TEST_LONG	2097152	/* 2^21 */
+#define A20_TEST_ADDR	(4 * 0x80)
+#define A20_TEST_SHORT	32
+#define A20_TEST_LONG	(1 << 21)	/* 2^21 = 2097152 */
 
 static int a20_test(int loops)
 {
 	int ok = 0;
-	int saved, ctr;
+	int saved;
+	int ctr;
 
 	set_fs(0x0000);
 	set_gs(0xffff);
@@ -66,7 +70,7 @@ static int a20_test(int loops)
 	while (loops--) {
 		wrfs32(++ctr, A20_TEST_ADDR);
 		io_delay();	/* Serialize and make delay constant */
-		ok = rdgs32(A20_TEST_ADDR+0x10) ^ ctr;
+		ok = rdgs32(A20_TEST_ADDR + 0x10) ^ ctr;
 		if (ok)
 			break;
 	}
@@ -81,8 +85,10 @@ static int a20_test_short(void)
 	return a20_test(A20_TEST_SHORT);
 }
 
-/* Longer test that actually waits for A20 to come on line; this
-   is useful when dealing with the KBC or other slow external circuitry. */
+/*
+ * Longer test that actually waits for A20 to come on line;
+ * this is useful when dealing with the KBC or other slow external circuitry.
+ */
 static int a20_test_long(void)
 {
 	return a20_test(A20_TEST_LONG);
@@ -116,7 +122,7 @@ static void enable_a20_fast(void)
 	u8 port_a;
 
 	port_a = inb(0x92);	/* Configuration port A */
-	port_a |=  0x02;	/* Enable A20 */
+	port_a |= 0x02;		/* Enable A20 */
 	port_a &= ~0x01;	/* Do not reset machine */
 	outb(port_a, 0x92);
 }
@@ -129,37 +135,40 @@ static void enable_a20_fast(void)
 
 int enable_a20(void)
 {
-       int loops = A20_ENABLE_LOOPS;
-       int kbc_err;
-
-       while (loops--) {
-	       /* First, check to see if A20 is already enabled
-		  (legacy free, etc.) */
-	       if (a20_test_short())
-		       return 0;
-	       
-	       /* Next, try the BIOS (INT 0x15, AX=0x2401) */
-	       enable_a20_bios();
-	       if (a20_test_short())
-		       return 0;
-	       
-	       /* Try enabling A20 through the keyboard controller */
-	       kbc_err = empty_8042();
-
-	       if (a20_test_short())
-		       return 0; /* BIOS worked, but with delayed reaction */
-	
-	       if (!kbc_err) {
-		       enable_a20_kbc();
-		       if (a20_test_long())
-			       return 0;
-	       }
-	       
-	       /* Finally, try enabling the "fast A20 gate" */
-	       enable_a20_fast();
-	       if (a20_test_long())
-		       return 0;
-       }
-       
-       return -1;
+	int loops = A20_ENABLE_LOOPS;
+
+	while (loops--) {
+		int kbc_err;
+
+		/*
+		 * First, check to see if A20 is already enabled
+		 * (legacy free, etc.)
+		 */
+		if (a20_test_short())
+			return 0;
+
+		/* Next, try the BIOS (INT 0x15, AX=0x2401) */
+		enable_a20_bios();
+		if (a20_test_short())
+			return 0;
+
+		/* Try enabling A20 through the keyboard controller */
+		kbc_err = empty_8042();
+
+		if (a20_test_short())
+			return 0; /* BIOS worked, but with delayed reaction */
+
+		if (!kbc_err) {
+			enable_a20_kbc();
+			if (a20_test_long())
+				return 0;
+		}
+
+		/* Finally, try enabling the "fast A20 gate" */
+		enable_a20_fast();
+		if (a20_test_long())
+			return 0;
+	}
+
+	return -1;
 }

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Borislav Petkov]

On Tue, Nov 19, 2013 at 09:48:43AM -0800, Joe Perches wrote:
> git diff -w and git blame -w both ignore whitespace changes and can
> help in tracking down actual modifications.

That's not the point - I don't want those commits there getting in the
way.

If it

 * doesn't fix a real bug,
 * doesn't improve code generation
 * doesn't have any other positive effect on the resulting object code

then it doesn't need to be there.

Whitespace jerking off doesn't belong in the kernel.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[H. Peter Anvin]

On 11/19/2013 10:23 AM, Borislav Petkov wrote:
> On Tue, Nov 19, 2013 at 09:48:43AM -0800, Joe Perches wrote:
>> git diff -w and git blame -w both ignore whitespace changes and can
>> help in tracking down actual modifications.
> 
> That's not the point - I don't want those commits there getting in the
> way.
> 
> If it
> 
>  * doesn't fix a real bug,
>  * doesn't improve code generation
>  * doesn't have any other positive effect on the resulting object code
> 
> then it doesn't need to be there.
> 
> Whitespace jerking off doesn't belong in the kernel.
> 

I'll take a proper cleanup, but just whitespace... just pointless churn.

	-hpa

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Joe Perches]

On Tue, 2013-11-19 at 10:24 -0800, H. Peter Anvin wrote:
> I'll take a proper cleanup,
> but just whitespace... just pointless churn.

What in your view would be "a proper cleanup" for a20.c?

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[H. Peter Anvin]

On 11/19/2013 10:28 AM, Joe Perches wrote:
> On Tue, 2013-11-19 at 10:24 -0800, H. Peter Anvin wrote:
>> I'll take a proper cleanup,
>> but just whitespace... just pointless churn.
> 
> What in your view would be "a proper cleanup" for a20.c?
> 

I don't know (or even if it needs one, although Ingo seems to think so.)
 I haven't really bothered with spending any mental energy on what seems
to be largely a non-problem.

It was more of a general statement.

	-hpa

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Borislav Petkov]

On Tue, Nov 19, 2013 at 10:45:00AM -0800, H. Peter Anvin wrote:
> I don't know (or even if it needs one,...

Exactly.

With the amount of real bugs/issues *actually* being reported out there,
why would people waste energy on senseless cleanups is beyond me.

a20.c is fine the way it is, it doesn't bother anyone, it works. So why
even touch it?

If people want to fix real issues, they can do lots of other sensible
work. One example would be: do randconfig builds and stare at the
warnings - there are a gazillion of them and I'm sure there are real
bugs hiding in there.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Ingo Molnar]

* Joe Perches <joe@perches.com> wrote:

> On Tue, 2013-11-19 at 10:24 -0800, H. Peter Anvin wrote:
>
> > I'll take a proper cleanup,
> > but just whitespace... just pointless churn.
> 
> What in your view would be "a proper cleanup" for a20.c?

If a newbie does a meaningful, complete, well done cleanup patch then 
congratulations and any such help is welcome.

If you as a more experienced kernel developer do a cleanup as part of 
some real work then sure, all such cleanups are welcome and they are a 
natural part of development work.

So a standalone cleanup patch to a20.c from _you_ would probably not 
qualify, almost by definition: your first patch was applied 7 years 
ago, you are by far not a newbie anymore, yet you seem to be mostly 
stuck on the 'cleanups and trivialities' level! Sheesh!

My message to the buerocrat Joe Perches is: please leave trivial and 
printk patches to newbies, you need to raise to the next level of 
kernel development already.

FYI, Linux is a meritocracy, not a bureaucracy: creating self-serving 
churn and attention-seeking but unimportant patches is not the way to 
gain kernel development credibility long term, and eventually people 
start protecting against your increasing abuse of the development 
process. IMHO.

Thanks,

	Ingo

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Borislav Petkov]

On Mon, Nov 25, 2013 at 11:18:07AM +0100, Ingo Molnar wrote:
> If a newbie does a meaningful, complete, well done cleanup patch then
> congratulations and any such help is welcome.
>
> If you as a more experienced kernel developer do a cleanup as part of
> some real work then sure, all such cleanups are welcome and they are a
> natural part of development work.
>
> So a standalone cleanup patch to a20.c from _you_ would probably not
> qualify, almost by definition: your first patch was applied 7 years
> ago, you are by far not a newbie anymore, yet you seem to be mostly
> stuck on the 'cleanups and trivialities' level! Sheesh!
>
> My message to the buerocrat Joe Perches is: please leave trivial and
> printk patches to newbies, you need to raise to the next level of
> kernel development already.
>
> FYI, Linux is a meritocracy, not a bureaucracy: creating self-serving
> churn and attention-seeking but unimportant patches is not the way to
> gain kernel development credibility long term, and eventually people
> start protecting against your increasing abuse of the development
> process. IMHO.

... and not only that - sending patches to the kernel shouldn't be about
the *sending* of patches itself but actually about doing some real
improvement to it.

Thanks Ingo for hitting the nail on the head!

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Joe Perches]

On Mon, 2013-11-25 at 11:18 +0100, Ingo Molnar wrote:
> * Joe Perches <joe@perches.com> wrote:
> > On Tue, 2013-11-19 at 10:24 -0800, H. Peter Anvin wrote:
> > > I'll take a proper cleanup,
> > > but just whitespace... just pointless churn.
> > What in your view would be "a proper cleanup" for a20.c?
> 
> If a newbie does a meaningful, complete, well done cleanup patch then 
> congratulations and any such help is welcome.
> 
> If you as a more experienced kernel developer do a cleanup as part of 
> some real work then sure, all such cleanups are welcome and they are a 
> natural part of development work.
> 
> So a standalone cleanup patch to a20.c from _you_ would probably not 
> qualify, almost by definition: your first patch was applied 7 years 
> ago,

More than 10, but I only note that by looking in a mirror.

> you are by far not a newbie anymore, yet you seem to be mostly 
> stuck on the 'cleanups and trivialities' level! Sheesh!
> 
> My message to the buerocrat Joe Perches is: please leave trivial and 
> printk patches to newbies, you need to raise to the next level of 
> kernel development already.

Were I employed to improve the kernel, I'd probably subsume
my neatening tendencies to the needs of the employer.

Thankfully, I'm not employed to improve Linux and I can
choose what I want to do without need to fix bugs that cause
entities to lose money, customers or profit.  Nor do I have
to add hardware support for whatever new gadget comes along.

> FYI, Linux is a meritocracy, not a bureaucracy:

I don't believe that Linux is a meritocracy at all.

It has become a business entity all to itself and is nearly
completely supported and maintained by profit-seeking
ventures such as the one that employs you.

Is any major maintainer an unpaid, unremunerated volunteer?

> creating self-serving 
> churn and attention-seeking

I think you've a odd view of what I submit.

I've a hard time believing I'm either self-serving or
attention-seeking.  I'll advance my belief that I'm rather
unassuming and not prone to spotlights.

If I see what I believe a defect in either logic or style,
I either comment on it if it's a patch, or submit a patch
myself.  I provide comments a fair amount.

> but unimportant patches is not the way to 
> gain kernel development credibility long term, and eventually people 
> start protecting against your increasing abuse of the development 
> process.

Do please reread of this thread as what I did was comment
on a particular patch by Johannes and suggested that it was
not likely a necessary thing to change.  What I tried to help
him with was understanding what, if anything, actually needed
to be or could be changed in the file.

>  IMHO.

<smile>
Perhaps you could dial down your humility a bit.
Perhaps you'll reread and see if it seems humble to you.

cheers, Joe

Re: [PATCH] x86: boot: Fix mixed indentation in a20.c

[Borislav Petkov]

On Mon, Nov 25, 2013 at 04:51:59PM -0800, Joe Perches wrote:

<snip senseless comebacks>

> Perhaps you could dial down your humility a bit. Perhaps you'll reread
> and see if it seems humble to you.

So I vivid reader of LKML and I were wondering how you'd react to the
urge from a kernel maintainer to get you to finally drop that senseless
whitespace jerking off and do some real work.

My money was on "he'll come back with a bunch of petty and irrelevant
arguments instead of *actually* listen to a maintainer".

I guess I won. :-)

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--