Message-ID: <535F0866.2060207@fastmail.fm>
Date: Tue, 29 Apr 2014 10:03:18 +0800
From: Michalis Pappas <mpappas@fastmail.fm>
User-Agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.4.0
MIME-Version: 1.0
To: Dan Carpenter <dan.carpenter@oracle.com>,
        Greg KH <gregkh@linuxfoundation.org>
CC: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: [PATCH v4 0/3] staging: gdm72xx: Code cleanup
References: <532BF9B0.2080008@fastmail.fm> <20140418225229.GA28077@kroah.com> <53534051.2020504@fastmail.fm> <53534116.3040705@fastmail.fm> <20140422093743.GC26890@mwanda> <53570BAA.3080709@fastmail.fm> <20140423080447.GS26890@mwanda> <53577E96.10106@fastmail.fm> <20140423090557.GU26890@mwanda> <20140423090904.GV26890@mwanda>
In-Reply-To: <20140423090904.GV26890@mwanda>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org

Ok, I generated a new patchset based on running checkpatch.pl with the
--strict option, as well as any recommendations made earlier on this thread.

>From the following patches, both 8/10 and 9/10 involve whitespace fixes:
the former fixes issues reported by checkpatch.pl while the latter is
mostly indentation stuff found by inspecting the code, stray tabs etc. I
am sending them as separate patches in case one wants to keep only one
of them.

After applying the patches, the following issues are still reported by
checkpatch.pl. My comments follow:

ERROR: Macros with complex values should be enclosed in parenthesis
#34: FILE: usb_ids.h:34:
+#define USB_DEVICE_BOOTLOADER(vid, pid)	\
+	{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)},	\
+	{USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)}

We agreed on that one already.

WARNING:LONG_LINE: line over 80 characters

This comes out in cases where I slightly bent the 80 chars rule to
improve readability. Should anyone complain that it doesn't fit their
terminal I promise I'll revert it back ;)

WARNING: unchecked sscanf return value
#295: FILE: gdm_wimax.c:295:
+		sscanf(e->dev->name, "wm%d", &idx);

>From my understanding this should be ok. The value stored in e->dev>name
is generated by __dev_alloc_name which does all the necessary checks for
user supplied input etc, so it should be considered as a trusted value.

WARNING:SPACING: Missing a blank line after declarations

This is a false positive, as it refers to pointer-to-function
declarations withing structures.

CHECK:BRACES: Blank lines aren't necessary after an open brace '{'

I found it to be more readable the way it was, so I didn't touch it.

CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment

I am not sure what the locks are used for so I am unable to write a
comment on these.

Regards,

-Michalis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/