linux-kernel - Re: [PATCH] staging: wlan-ng: prism: coding style fixes, removed space-indentations

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: <20111213182752.GH3503@mwanda>
Date:	Tue, 13 Dec 2011 21:27:52 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Dan Luedtke <mail@...rl.de>
Cc:	gregkh@...e.de, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: wlan-ng: prism: coding style fixes, removed
 space-indentations

On Tue, Dec 13, 2011 at 12:35:37PM +0100, Dan Luedtke wrote:
> Fixed some coding style issues. Converted multiple spaces to tabs.
> 

Multiple spaces are allowed.  See below.

> (Please provide feedback on problems, it's my first kernel patch ever)

Everyone's first patch is rejected, so don't feel bad.  :)  Btw put
these kind of comments which you don't want in the permanent git log
under the --- dashes.  git am removes that information.

> 
> Signed-off-by: Dan Luedtke <mail@...rl.de>
> ---
>  drivers/staging/wlan-ng/prism2fw.c |  190 +++++++++++++++++++-----------------
>  1 files changed, 99 insertions(+), 91 deletions(-)
> 
> diff --git a/drivers/staging/wlan-ng/prism2fw.c b/drivers/staging/wlan-ng/prism2fw.c
> index 3c40096..e035839 100644
> --- a/drivers/staging/wlan-ng/prism2fw.c
> +++ b/drivers/staging/wlan-ng/prism2fw.c
> @@ -172,13 +172,14 @@ static int read_cardpda(struct pda *pda, wlandevice_t *wlandev);
>  static int mkpdrlist(struct pda *pda);
>  
>  static int plugimage(struct imgchunk *fchunk, unsigned int nfchunks,
> -	      struct s3plugrec *s3plug, unsigned int ns3plug, struct pda * pda);
> +	struct s3plugrec *s3plug, unsigned int ns3plug, struct pda * pda);
>  
>  static int crcimage(struct imgchunk *fchunk, unsigned int nfchunks,
> -	     struct s3crcrec *s3crc, unsigned int ns3crc);
> +				struct s3crcrec *s3crc, unsigned int ns3crc);
>  
>  static int writeimage(wlandevice_t *wlandev, struct imgchunk *fchunk,
> -	       unsigned int nfchunks);
> +							unsigned int nfchunks);

It looks like you've tried to make everything aligned with the right
hand side of the screen.  I think that's the first time I've seen
someone do that.  I don't think it's normal.

> +
>  static void free_chunks(struct imgchunk *fchunk, unsigned int *nfchunks);
>  
>  static void free_srecs(void);
> @@ -200,23 +201,23 @@ static int validate_identity(void);
>  * Returns:
>  *	0	- success
>  *	~0	- failure
> -----------------------------------------------------------------*/
> +*----------------------------------------------------------------*/

There is a normal kernel comment style described in Chapter 8 of
Documentation/CodingStyle.  Or for documenting function parameters,
if you want to be super awesome you could use kernel doc format.
Documentation/kernel-doc-nano-HOWTO.txt

Btw, if you do something complicated with comments, that should be
sent as a separate patch.

>  int prism2_fwtry(struct usb_device *udev, wlandevice_t *wlandev)
>  {
>  	const struct firmware *fw_entry = NULL;
>  
>  	printk(KERN_INFO "prism2_usb: Checking for firmware %s\n",
> -	       PRISM2_USB_FWFILE);
> +		PRISM2_USB_FWFILE);

The original is fine here.  It uses tabs as far as possible and then
spaces to make it align with the start of the function.  Same for
the rest below.

> @@ -951,7 +958,8 @@ int read_fwfile(const struct ihex_binrec *record)
>  			s3data[ns3data].data = (uint8_t *) record->data;
>  			ns3data++;
>  			if (ns3data == S3DATA_MAX) {
> -				printk(KERN_ERR "S3 datarec limit reached - aborting\n");
> +				printk(KERN_ERR "S3 datarec limit reached"
> +					" - aborting\n");

printks are allowed to break the 80 character limit.  checkpatch.pl
won't complain about this.  We might want to grep for the string and
breaking up the lines makes that harder.

You could do something like:
			printk(KERN_ERR
			       "S3 datarec limit reached - aborting\n");
This one would fit under the limit like that, but I would probably
just leave it as is.

regards,
dan carpenter



Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)