[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <536CA86B.7000109@fastmail.fm>
Date: Fri, 09 May 2014 18:05:31 +0800
From: Michalis Pappas <mpappas@...tmail.fm>
To: Dan Carpenter <dan.carpenter@...cle.com>,
Greg KH <gregkh@...uxfoundation.org>
CC: devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: [PATCH v5 0/10] staging: gdm72xx: Code cleanup
On 04/29/2014 10:03 AM, Michalis Pappas wrote:
> 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
> _______________________________________________
> devel mailing list
> devel@...uxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
>
Updates from v4:
* Fixed message subjects
* Improved commit messages
* Added forgotten signed-of-by's
Thanks,
-Michalis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists