Re: [PATCH 1/2] 8139too: clean up spaces and TABs

From: Németh Márton
Date: Sun Jun 15 2008 - 03:50:50 EST


Stefan Richter wrote:
> Németh Márton wrote:
>> From: Márton Németh <nm127@xxxxxxxxxxx>
>>
>> Clean up the following errors and warnings reported by checkpatch.pl:
>
> Is 8139too in active development or are there people actively fixing
> current bugs in it? If not, a whitespace cleanup may be considered a
> waste of time. There are even a few valid arguments that such changes
> are harmful then.
>
>> - space prohibited between function name and open parenthesis '('
>> - space required before the open brace '{'
>> - code indent should use tabs where possible
>> - line over 80 characters
>> - spaces required around that '=' (ctx:VxW)
>
> Did you check that your whitespace changes are indeed only whitespace
> changes, i.e. that resulting assembler is unchanged? If you checked it,
> it's worth mentioning in the submission.

Yes, I checked this. When I compared the "objdump -d" output I found that
only the function calls are changed where the __LINE__ is a parameter.

Sorry about taking your time. My intention was to make the 8139too source
code better as I am using this driver actively to communicate with the
digital world. I have chosen the checkpatch.pl's output to compare the
whether the old and the new code are better or not. Maybe that was a mistake.

I would like to try it again, but in a different way. I collected here
the different problems reported by checkpatch.pl. I divided it to two
groups according to your comments which I think it worth to deal with:

+ ERROR: Macros with complex values should be enclosed in parenthesis
+ WARNING: __func__ should be used instead of gcc specific __FUNCTION__
+ WARNING: plain inline is preferred over __inline__
+ WARNING: Use #include <linux/io.h> instead of <asm/io.h>
+ WARNING: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>

and the second group which it seems that may hurt somebody:

- ERROR: code indent should use tabs where possible
- ERROR: space required after that ',' (ctx:VxV)
- ERROR: space required before the open brace '{'
- ERROR: space required before the open parenthesis '('
- ERROR: spaces required around that '=' (ctx:VxW)
- ERROR: trailing statements should be on next line
- WARNING: braces {} are not necessary for single statement blocks
- WARNING: do not add new typedefs
- WARNING: line over 80 characters
- WARNING: space prohibited between function name and open parenthesis '('

This is a much smaller set of changes. What do you think?

Regards,

Márton Németh

--
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/