linux-kernel - Re: [PATCH 3/3] Fixed coding style issues.

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]

Message-ID: <1495006264.30577.4.camel@suse.com>
Date:   Wed, 17 May 2017 09:31:04 +0200
From:   Oliver Neukum <oneukum@...e.com>
To:     Alberto Ladron <alberto.ladron@...il.com>, alberto.ladron@...i.com,
        gregkh@...uxfoundation.org, stern@...land.harvard.edu
Cc:     usb-storage@...ts.one-eyed-alien.net, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCH 3/3] Fixed coding style issues.

Am Dienstag, den 16.05.2017, 21:17 -0500 schrieb Alberto Ladron:
> Signed-off-by: Alberto Ladron <alberto.ladron@...il.com>
> ---
>  drivers/usb/storage/shuttle_usbat.c | 83 ++++++++++++++++++-------------------
>  1 file changed, 41 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/usb/storage/shuttle_usbat.c b/drivers/usb/storage/shuttle_usbat.c
> index 3b0294e..9eddc40 100644
> --- a/drivers/usb/storage/shuttle_usbat.c
> +++ b/drivers/usb/storage/shuttle_usbat.c
> @@ -152,13 +152,13 @@ struct usbat_info {
>  	unsigned long sense_ascq;  /* additional sense code qualifier */
>  };
>  
> -#define short_pack(LSB,MSB) ( ((u16)(LSB)) | ( ((u16)(MSB))<<8 ) )
> +#define short_pack(LSB, MSB) (((u16)(LSB)) | (((u16)(MSB))<<8))
>  #define LSB_of(s) ((s)&0xFF)
>  #define MSB_of(s) ((s)>>8)
>  
> -static int transferred = 0;
> +static int transferred;
>  
> -static int usbat_flash_transport(struct scsi_cmnd * srb, struct us_data *us);
> +static int usbat_flash_transport(struct scsi_cmnd *srb, struct us_data *us);
>  static int usbat_hp8200e_transport(struct scsi_cmnd *srb, struct us_data *us);
>  
>  static int init_usbat_cd(struct us_data *us);
> @@ -172,7 +172,7 @@ static int init_usbat_flash(struct us_data *us);
>  		    vendorName, productName, useProtocol, useTransport, \
>  		    initFunction, flags) \
>  { USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \
> -  .driver_info = (flags) }
> +.driver_info = (flags) }
>  
>  static struct usb_device_id usbat_usb_ids[] = {
>  #	include "unusual_usbat.h"
> @@ -225,7 +225,7 @@ static void usbat_pack_ata_sector_cmd(unsigned char *buf,
>   */
>  static int usbat_get_device_type(struct us_data *us)
>  {
> -	return ((struct usbat_info*)us->extra)->devicetype;
> +	return ((struct usbat_info *)us->extra)->devicetype;
>  }
>  
>  /*
> @@ -268,7 +268,7 @@ static int usbat_write(struct us_data *us,
>   * Convenience function to perform a bulk read
>   */
>  static int usbat_bulk_read(struct us_data *us,
> -			   void* buf,
> +			   void *buf,
>  			   unsigned int len,
>  			   int use_sg)
>  {
> @@ -283,7 +283,7 @@ static int usbat_bulk_read(struct us_data *us,
>   * Convenience function to perform a bulk write
>   */
>  static int usbat_bulk_write(struct us_data *us,
> -			    void* buf,
> +			    void *buf,
>  			    unsigned int len,
>  			    int use_sg)
>  {
> @@ -416,11 +416,11 @@ static int usbat_wait_not_busy(struct us_data *us, int minutes)
>  	 * minutes!
>  	 */
>  
> -	for (i=0; i<1200+minutes*60; i++) {
> +	for (i = 0; i < 1200+minutes*60; i++) {
>  
> - 		result = usbat_get_status(us, status);
> +		result = usbat_get_status(us, status);
>  
> -		if (result!=USB_STOR_XFER_GOOD)
> +		if (result != USB_STOR_XFER_GOOD)
>  			return USB_STOR_TRANSPORT_ERROR;
>  		if (*status & 0x01) { /* check condition */
>  			result = usbat_read(us, USBAT_ATA, 0x10, status);
> @@ -429,16 +429,16 @@ static int usbat_wait_not_busy(struct us_data *us, int minutes)
>  		if (*status & 0x20) /* device fault */
>  			return USB_STOR_TRANSPORT_FAILED;
>  
> -		if ((*status & 0x80)==0x00) { /* not busy */
> +		if ((*status & 0x80) == 0x00) { /* not busy */

Would you care to make a second pass and submit patches using the BIT()
macro where appropriate?

> [..]
> @@ -1753,13 +1752,13 @@ static int usbat_flash_transport(struct scsi_cmnd * srb, struct us_data *us)
>  		 * I don't think we'll ever see a READ_12 but support it anyway
>  		 */
>  		block = ((u32)(srb->cmnd[2]) << 24) | ((u32)(srb->cmnd[3]) << 16) |
> -		        ((u32)(srb->cmnd[4]) <<  8) | ((u32)(srb->cmnd[5]));
> +			((u32)(srb->cmnd[4]) <<  8) | ((u32)(srb->cmnd[5]));
>  
>  		blocks = ((u32)(srb->cmnd[6]) << 24) | ((u32)(srb->cmnd[7]) << 16) |
> -		         ((u32)(srb->cmnd[8]) <<  8) | ((u32)(srb->cmnd[9]));
> +			((u32)(srb->cmnd[8]) <<  8) | ((u32)(srb->cmnd[9]));

There is no point in beautifying such things. Just make another pass
and use the macros for endianness conversion with unaligned access.

	Regards
		Oliver