[PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[Paolo Ciarrocchi]

Before:
total: 5 errors, 0 warnings, 48 lines checked

After:
total: 0 errors, 0 warnings, 58 lines checked

paolo@paolo-desktop:~/linux.trees.git$ md5sum /tmp/bios_uv.o.*
9afe794594831166704744184e192ed8  /tmp/bios_uv.o.after
9afe794594831166704744184e192ed8  /tmp/bios_uv.o.before

Signed-off-by: Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com>
---
 arch/x86/kernel/bios_uv.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/bios_uv.c b/arch/x86/kernel/bios_uv.c
index c639bd5..100e759 100644
--- a/arch/x86/kernel/bios_uv.c
+++ b/arch/x86/kernel/bios_uv.c
@@ -25,11 +25,21 @@ x86_bios_strerror(long status)
 {
 	const char *str;
 	switch (status) {
-	case  0: str = "Call completed without error"; break;
-	case -1: str = "Not implemented"; break;
-	case -2: str = "Invalid argument"; break;
-	case -3: str = "Call completed with error"; break;
-	default: str = "Unknown BIOS status code"; break;
+	case  0:
+		str = "Call completed without error";
+		break;
+	case -1:
+		str = "Not implemented";
+		break;
+	case -2:
+		str = "Invalid argument";
+		break;
+	case -3:
+		str = "Call completed with error";
+		break;
+	default:
+		str = "Unknown BIOS status code";
+		break;
 	}
 	return str;
 }
-- 
1.5.6.rc1.21.g03300

Re: [PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[H. Peter Anvin]

Paolo Ciarrocchi wrote:
> 
> diff --git a/arch/x86/kernel/bios_uv.c b/arch/x86/kernel/bios_uv.c
> index c639bd5..100e759 100644
> --- a/arch/x86/kernel/bios_uv.c
> +++ b/arch/x86/kernel/bios_uv.c
> @@ -25,11 +25,21 @@ x86_bios_strerror(long status)
>  {
>  	const char *str;
>  	switch (status) {
> -	case  0: str = "Call completed without error"; break;
> -	case -1: str = "Not implemented"; break;
> -	case -2: str = "Invalid argument"; break;
> -	case -3: str = "Call completed with error"; break;
> -	default: str = "Unknown BIOS status code"; break;
> +	case  0:
> +		str = "Call completed without error";
> +		break;
> +	case -1:
> +		str = "Not implemented";
> +		break;
> +	case -2:
> +		str = "Invalid argument";
> +		break;
> +	case -3:
> +		str = "Call completed with error";
> +		break;
> +	default:
> +		str = "Unknown BIOS status code";
> +		break;
>  	}
>  	return str;
>  }

This should be an array in the first place...

	-hpa

Re: [PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[Stefan Richter]

H. Peter Anvin wrote:
> Paolo Ciarrocchi wrote:
>>
>> diff --git a/arch/x86/kernel/bios_uv.c b/arch/x86/kernel/bios_uv.c
>> index c639bd5..100e759 100644
>> --- a/arch/x86/kernel/bios_uv.c
>> +++ b/arch/x86/kernel/bios_uv.c
>> @@ -25,11 +25,21 @@ x86_bios_strerror(long status)
>>  {
>>      const char *str;
>>      switch (status) {
>> -    case  0: str = "Call completed without error"; break;
>> -    case -1: str = "Not implemented"; break;
>> -    case -2: str = "Invalid argument"; break;
>> -    case -3: str = "Call completed with error"; break;
>> -    default: str = "Unknown BIOS status code"; break;
>> +    case  0:
>> +        str = "Call completed without error";
>> +        break;
>> +    case -1:
>> +        str = "Not implemented";
>> +        break;
>> +    case -2:
>> +        str = "Invalid argument";
>> +        break;
>> +    case -3:
>> +        str = "Call completed with error";
>> +        break;
>> +    default:
>> +        str = "Unknown BIOS status code";
>> +        break;
>>      }
>>      return str;
>>  }
> 
> This should be an array in the first place...

Besides, by following CodingStyle to the letter, it arguably breaks 
rather than fixes coding style.  The former code was easy enough to read.
-- 
Stefan Richter
-=====-==--- =--- ---==
http://arcgraph.de/sr/

Re: [PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[Krzysztof Halasa]

Stefan Richter <stefanr@s5r6.in-berlin.de> writes:

>>>      const char *str;
>>>      switch (status) {
>>> -    case  0: str = "Call completed without error"; break;
>>> -    case -1: str = "Not implemented"; break;
>>> -    case -2: str = "Invalid argument"; break;
>>> -    case -3: str = "Call completed with error"; break;
>>> -    default: str = "Unknown BIOS status code"; break;
>>> +    case  0:
>>> +        str = "Call completed without error";
>>> +        break;
>>> +    case -1:
>>> +        str = "Not implemented";
>>> +        break;
>>> +    case -2:
>>> +        str = "Invalid argument";
>>> +        break;
>>> +    case -3:
>>> +        str = "Call completed with error";
>>> +        break;
>>> +    default:
>>> +        str = "Unknown BIOS status code";
>>> +        break;
>>>      }
>>>      return str;
>>>  }
>>
>> This should be an array in the first place...
>
> Besides, by following CodingStyle to the letter, it arguably breaks
> rather than fixes coding style.  The former code was easy enough to
> read.

Right. The latter is much worse. That's why nobody can trust
CodingStyle and/or checkpatch automatically. It's only sane mode
of operation is as a help tool for authors and maintainers, to
quickly locate _potential_ problems.
-- 
Krzysztof Halasa

Re: [PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[Paolo Ciarrocchi]

On Sun, Aug 3, 2008 at 1:54 PM, Krzysztof Halasa <khc@pm.waw.pl> wrote:
[...]
> Right. The latter is much worse. That's why nobody can trust
> CodingStyle and/or checkpatch automatically. It's only sane mode
> of operation is as a help tool for authors and maintainers, to
> quickly locate _potential_ problems.

_much worse_? Wow, I thought the opposite.
Anyway, that's just a coding style patch, feel free to simply not apply it as
it's really a matter of personal taste but be sure that I'm not
blindly changing the code
accordingly to checkpatch. That might be almost true when I started
doing this kind of patches months
ago but it's definitely not true anymore.

Thanks.

regards,
-- 
Paolo
http://paolo.ciarrocchi.googlepages.com/

Re: [PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[H. Peter Anvin]

Paolo Ciarrocchi wrote:
> 
> _much worse_? Wow, I thought the opposite.
> Anyway, that's just a coding style patch, feel free to simply not apply it as
> it's really a matter of personal taste but be sure that I'm not
> blindly changing the code
> accordingly to checkpatch. That might be almost true when I started
> doing this kind of patches months
> ago but it's definitely not true anymore.
> 

In this case, the real problem is that the original code used a switch 
statement where it really should have used a table.

	-hpa

Re: [PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[Ingo Molnar]

* Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com> wrote:

> Before:
> total: 5 errors, 0 warnings, 48 lines checked
> 
> After:
> total: 0 errors, 0 warnings, 58 lines checked
> 
> paolo@paolo-desktop:~/linux.trees.git$ md5sum /tmp/bios_uv.o.*
> 9afe794594831166704744184e192ed8  /tmp/bios_uv.o.after
> 9afe794594831166704744184e192ed8  /tmp/bios_uv.o.before
> 
> Signed-off-by: Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com>
> ---
>  arch/x86/kernel/bios_uv.c |   20 +++++++++++++++-----
>  1 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/bios_uv.c b/arch/x86/kernel/bios_uv.c
> index c639bd5..100e759 100644
> --- a/arch/x86/kernel/bios_uv.c
> +++ b/arch/x86/kernel/bios_uv.c
> @@ -25,11 +25,21 @@ x86_bios_strerror(long status)
>  {
>  	const char *str;
>  	switch (status) {
> -	case  0: str = "Call completed without error"; break;
> -	case -1: str = "Not implemented"; break;
> -	case -2: str = "Invalid argument"; break;
> -	case -3: str = "Call completed with error"; break;
> -	default: str = "Unknown BIOS status code"; break;
> +	case  0:
> +		str = "Call completed without error";
> +		break;
> +	case -1:
> +		str = "Not implemented";
> +		break;
> +	case -2:
> +		str = "Invalid argument";
> +		break;
> +	case -3:
> +		str = "Call completed with error";
> +		break;
> +	default:
> +		str = "Unknown BIOS status code";
> +		break;

hm - i changed your patch to the one below, to align the break's 
vertically, which makes the original variant a lot more readable (and 
even more readable than the new one). Checkpatch still complains about 
it but that's OK, there are always exceptions.

	Ingo

------------------->
Subject: x86: coding style fixes to arch/x86/kernel/bios_uv.c
From: Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com>
Date: Sat, 2 Aug 2008 21:24:06 +0200

paolo@paolo-desktop:~/linux.trees.git$ md5sum /tmp/bios_uv.o.*
9afe794594831166704744184e192ed8  /tmp/bios_uv.o.after
9afe794594831166704744184e192ed8  /tmp/bios_uv.o.before

Signed-off-by: Paolo Ciarrocchi <paolo.ciarrocchi@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/bios_uv.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Index: tip/arch/x86/kernel/bios_uv.c
===================================================================
--- tip.orig/arch/x86/kernel/bios_uv.c
+++ tip/arch/x86/kernel/bios_uv.c
@@ -25,11 +25,11 @@ x86_bios_strerror(long status)
 {
 	const char *str;
 	switch (status) {
-	case  0: str = "Call completed without error"; break;
-	case -1: str = "Not implemented"; break;
-	case -2: str = "Invalid argument"; break;
-	case -3: str = "Call completed with error"; break;
-	default: str = "Unknown BIOS status code"; break;
+	case  0: str = "Call completed without error";	break;
+	case -1: str = "Not implemented";		break;
+	case -2: str = "Invalid argument";		break;
+	case -3: str = "Call completed with error";	break;
+	default: str = "Unknown BIOS status code";	break;
 	}
 	return str;
 }

Re: [PATCH 2/5] x86: Coding style fixes to arch/x86/kernel/bios_uv.c

[Paolo Ciarrocchi]

On 8/15/08, Ingo Molnar <mingo@elte.hu> wrote:

> hm - i changed your patch to the one below, to align the break's
> vertically, which makes the original variant a lot more readable (and
> even more readable than the new one). Checkpatch still complains about
> it but that's OK, there are always exceptions.
>

thank you Ingo.

ciao,
-- 
Paolo
http://paolo.ciarrocchi.googlepages.com/