linux-kernel - Re: [PATCH v3] staging: rts5208: Clean up coding style in rtsx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 3 Oct 2014 09:35:59 -0700
From:	Greg KH <gregkh@...uxfoundation.org>
To:	Giedrius Statkevicius <giedrius.statkevicius@...il.com>
Cc:	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
	fabio.falzoi84@...il.com
Subject: Re: [PATCH v3] staging: rts5208: Clean up coding style in rtsx_chip.c

On Fri, Oct 03, 2014 at 07:16:13PM +0300, Giedrius Statkevicius wrote:
> Make the second line of a divided line match the opening paranthesis.
> Combine two if's in form of 'if (a) if (b) { [...] }' into one to lower the indentation level.
> To further lower indentation level and make the code more concise use the ternary operator where possible and sensible.
> Make the names of labels all lower case to avoid Camel Case.
> Remove unnecessary parantheses around variables that are after a dereference operator i.e. &(a) => &a
> Switch the if condition around in 'if (0 == (value & (1 << bit))) {' to make it have the same form as the rest of the code.
> Remove unnecessary braces in rtsx_enable_aspm() around one statement
> 
> After this patch checkpatch.pl without --strict doesn't complain about rtsx_chip.c at all and with --strict it only complains about the unmatched parantheses.
> I am reluctant to move that code with unmatched parantheses into another function because I don't have the hardware needed to test this driver and don't have
> enough experience to do it properly. Doing so would just probably hide the other problems this code has. My patch should be purely about changing the code style without
> creating more functions and moving code to them. 
> 
> This patch is against Greg KH's staging-next tree.
> 
> Signed-off-by: Giedrius Statkevicius <giedrius.statkevicius@...il.com>

That is a _lot_ to do in just one patch (also note you should wrap your
changelog text at 72 characters, like git asked you to...)

Please break this up into small, logical patches, each only doing one
thing.  Trying to review a large patch like this is almost impossible.

thanks,

greg k-h
--
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/