Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only

[JÃrn Engel]

On Tue, 25 March 2008 14:45:56 +0100, Ingo Molnar wrote:
> 
> and let me give an example with the your very own code that you wrote 
> and maintain, drivers/mtd/devices/block2mtd.c:
> 
>                                     errors   lines of code   errors/KLOC
> drivers/mtd/devices/block2mtd.c         10             490          20.4
> 
> that's pretty OK code, but not perfect, the 10 errors are:
> 
>  ERROR: do not use C99 // comments
>  ERROR: need spaces around that '=' (ctx:VxV)
>  ERROR: need spaces around that '<' (ctx:VxV)
>  ERROR: do not use C99 // comments
>  ERROR: do not use C99 // comments
>  ERROR: do not use C99 // comments
>  ERROR: do not use C99 // comments
>  ERROR: do not use C99 // comments
>  ERROR: do not use C99 // comments
>  ERROR: do not initialise statics to 0 or NULL

The last should and will be fixed.  The // I don't really care about.
Send a patch if you do.

Going over my logfs patch, I found several things that are either false
positives or rather questionable in my book.  <adds Andy to Cc:>


(foo*) should be (foo *)
	What does that extra space gain us?


ERROR: no space before that close parenthesis ')'
#2565: FILE: fs/logfs/gc.c:294:
+                       seg_ofs + sizeof(oh) < super->s_segsize; ) {

Actual code is this:
	for (seg_ofs = LOGFS_SEGMENT_HEADERSIZE;
			seg_ofs + sizeof(oh) < super->s_segsize; ) {
The for() loop is missing one of its three terms.  I assume this is one
of the effects of not having a perfect C parser.


ERROR: trailing statements should be on next line
#5000: FILE: fs/logfs/readwrite.c:203:
+       } else while (unlikely(TestSetPageLocked(page))) {

We have an explicit exception for "else if".  "else while" makes imo
just as much (or as little) sense.


ERROR: need space after that ',' (ctx:VxV)
#5801: FILE: fs/logfs/readwrite.c:1004:
+               ret = logfs_segment_read(inode, ipage, this_wc->ofs, bix,level);
                                      
One of those examples where a missing space is the lesser of two or
three evils.


ERROR: need consistent spacing around '|' (ctx:WxV)
#8293: FILE: fs/logfs/dev_mtd.c:376:
+                       |SLAB_MEM_SPREAD|SLAB_PANIC),

I have no idea what this is about.  Original code:
	mtd_cache = kmem_cache_create("mtd_cache", sizeof(struct mtd_inode), 0,
			(SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT
			 |SLAB_MEM_SPREAD|SLAB_PANIC),

JÃrn

-- 
Victory in war is not repetitious.
-- Sun Tzu
--
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/