linux-kernel - Re: [PATCH 1/8] staging: kpc2000: add line breaks

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Jun 2019 14:52:51 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Fabian Krueger <fabian.krueger@....de>
Cc:     devel@...verdev.osuosl.org, linux-kernel@...cs.fau.de,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Michael Scheiderer <michael.scheiderer@....de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/8] staging: kpc2000: add line breaks

On Tue, Jun 25, 2019 at 01:27:12PM +0200, Fabian Krueger wrote:
> diff --git a/drivers/staging/kpc2000/kpc2000_spi.c b/drivers/staging/kpc2000/kpc2000_spi.c
> index c3e5c1848f53..7ed0fb6b4abb 100644
> --- a/drivers/staging/kpc2000/kpc2000_spi.c
> +++ b/drivers/staging/kpc2000/kpc2000_spi.c
> @@ -30,18 +30,46 @@
>  #include "kpc.h"
>  
>  static struct mtd_partition p2kr0_spi0_parts[] = {
> -	{ .name = "SLOT_0",	.size = 7798784,		.offset = 0,                },
> -	{ .name = "SLOT_1",	.size = 7798784,		.offset = MTDPART_OFS_NXTBLK},
> -	{ .name = "SLOT_2",	.size = 7798784,		.offset = MTDPART_OFS_NXTBLK},
> -	{ .name = "SLOT_3",	.size = 7798784,		.offset = MTDPART_OFS_NXTBLK},
> -	{ .name = "CS0_EXTRA",	.size = MTDPART_SIZ_FULL,	.offset = MTDPART_OFS_NXTBLK},
> +	{.name = "SLOT_0",
> +	 .size = 7798784,
> +	 .offset = 0,},
> +
> +	{.name = "SLOT_1",
> +	 .size = 7798784,
> +	 .offset = MTDPART_OFS_NXTBLK},
> +
> +	{.name = "SLOT_2",
> +	 .size = 7798784,
> +	 .offset = MTDPART_OFS_NXTBLK},
> +
> +	{.name = "SLOT_3",
> +	 .size = 7798784,
> +	 .offset = MTDPART_OFS_NXTBLK},
> +
> +	{.name = "CS0_EXTRA",
> +	 .size = MTDPART_SIZ_FULL,
> +	 .offset = MTDPART_OFS_NXTBLK},
>  };

The original is fine/better.

> @@ -215,7 +244,9 @@ kp_spi_txrx_pio(struct spi_device *spidev, struct spi_transfer *transfer)
>  		for (i = 0 ; i < c ; i++) {
>  			char val = *tx++;
>  
> -			if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS, KP_SPI_REG_STATUS_TXS) < 0) {
> +			if (kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS,
> +						    KP_SPI_REG_STATUS_TXS)
> +			    < 0) {

I don't like how the < 0 is on the next line.  It would be better to
introduce an "int ret;"
			ret = kp_spi_wait_for_reg_bit(cs, KP_SPI_REG_STATUS,
						      KP_SPI_REG_STATUS_TXS);
			if (ret < 0)
				goto out;


>  				goto out;
>  			}
>  

> @@ -317,7 +353,8 @@ kp_spi_transfer_one_message(struct spi_master *master, struct spi_message *m)
>  			dev_dbg(kpspi->dev, "  transfer -EINVAL\n");
>  			return -EINVAL;
>  		}
> -		if (transfer->speed_hz && (transfer->speed_hz < (KP_SPI_CLK >> 15))) {
> +		if (transfer->speed_hz && (transfer->speed_hz <
> +					   (KP_SPI_CLK >> 15))) {


Move the whole conition down:

		if (transfer->speed_hz &&
		    transfer->speed_hz < (KP_SPI_CLK >> 15)) {

regards,
dan carpenter