linux-kernel - Re: [PATCH] Staging: pi433: Coding style fixes

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]

Message-ID: <20170718133445.GA16949@kroah.com>
Date:   Tue, 18 Jul 2017 15:34:45 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Simo Koskinen <koskisoft@...il.com>
Cc:     linux@...f-Entwicklungen.de, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Staging: pi433: Coding style fixes

On Tue, Jul 18, 2017 at 02:41:33PM +0200, Simo Koskinen wrote:
> Fixes if-statement related warning and errors reported
> by checkpatch.pl
> 
> Signed-off-by: Simo Koskinen <koskisoft@...il.com>
> ---
>  drivers/staging/pi433/pi433_if.c | 253 +++++++++++++++++----------------------
>  drivers/staging/pi433/rf69.c     |  69 ++++++-----
>  2 files changed, 148 insertions(+), 174 deletions(-)
> 
> diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
> index 1bc478a..3e4be2e 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -131,19 +131,14 @@ DIO0_irq_handler(unsigned int irq, void *dev_id, struct pt_regs *regs)
>  {
>  	struct pi433_device *device = dev_id;
>  
> -	if      (device->irq_state[DIO0] == DIO_PacketSent)
> -	{
> +	if (device->irq_state[DIO0] == DIO_PacketSent) {
>  		device->free_in_fifo = FIFO_SIZE;
>  		printk("DIO0 irq: Packet sent\n"); // TODO: printk() should include KERN_ facility level
>  		wake_up_interruptible(&device->fifo_wait_queue);
> -	}
> -	else if (device->irq_state[DIO0] == DIO_Rssi_DIO0)
> -	{
> +	} else if (device->irq_state[DIO0] == DIO_Rssi_DIO0) {
>  		printk("DIO0 irq: RSSI level over threshold\n");
>  		wake_up_interruptible(&device->rx_wait_queue);
> -	}
> -	else if (device->irq_state[DIO0] == DIO_PayloadReady)
> -	{
> +	} else if (device->irq_state[DIO0] == DIO_PayloadReady) {
>  		printk("DIO0 irq: PayloadReady\n");
>  		device->free_in_fifo = 0;
>  		wake_up_interruptible(&device->fifo_wait_queue);
> @@ -157,15 +152,15 @@ DIO1_irq_handler(unsigned int irq, void *dev_id, struct pt_regs *regs)
>  {
>  	struct pi433_device *device = dev_id;
>  
> -	if      (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1)
> -	{
> +	if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1) {
>  		device->free_in_fifo = FIFO_SIZE;
> +	} else if (device->irq_state[DIO1] == DIO_FifoLevel) {
> +		if (device->rx_active)
> +			device->free_in_fifo = FIFO_THRESHOLD - 1;
> +		else
> +			device->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1;
>  	}
> -	else if (device->irq_state[DIO1] == DIO_FifoLevel)
> -	{
> -		if (device->rx_active)	device->free_in_fifo = FIFO_THRESHOLD - 1;
> -		else			device->free_in_fifo = FIFO_SIZE - FIFO_THRESHOLD - 1;
> -	}
> +
>  	printk("DIO1 irq: %d bytes free in fifo\n", device->free_in_fifo); // TODO: printk() should include KERN_ facility level
>  	wake_up_interruptible(&device->fifo_wait_queue);
>  
> @@ -200,43 +195,38 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
>  	/* packet config */
>  	/* enable */
>  	SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
> -	if (rx_cfg->enable_sync == optionOn)
> -	{
> +	if (rx_cfg->enable_sync == optionOn) {
>  		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
> -	}
> -	else
> -	{
> +	} else {
>  		SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
>  	}
> +
>  	SET_CHECKED(rf69_set_packet_format  (dev->spi, rx_cfg->enable_length_byte));
>  	SET_CHECKED(rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering));
>  	SET_CHECKED(rf69_set_crc_enable	    (dev->spi, rx_cfg->enable_crc));
>  
>  	/* lengths */
>  	SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
> -	if (rx_cfg->enable_length_byte == optionOn)
> -	{
> +	if (rx_cfg->enable_length_byte == optionOn) {
>  		SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
> -	}
> -	else if (rx_cfg->fixed_message_length != 0)
> -	{
> +	} else if (rx_cfg->fixed_message_length != 0) {
>  		payload_length = rx_cfg->fixed_message_length;
> -		if (rx_cfg->enable_length_byte  == optionOn) payload_length++;
> -		if (rx_cfg->enable_address_filtering != filteringOff) payload_length++;
> +		if (rx_cfg->enable_length_byte == optionOn)
> +			payload_length++;
> +		

Don't add new coding style issues :(

Always be careful, and ideally, you would show the before and after
results are the same for such a large patch...

thanks,

greg k-h