All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Behrens <sbehrens@giantdisaster.de>
To: Goffredo Baroncelli <kreijack@inwind.it>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v0 1/4] Add support for sysfs to btrfs.
Date: Thu, 23 Aug 2012 16:01:16 +0200	[thread overview]
Message-ID: <503637AC.7080103@giantdisaster.de> (raw)
In-Reply-To: <20120823121242.12203.50365.stgit@venice.bhome>

On Thu, 23 Aug 2012 14:12:59 +0200, Goffredo Baroncelli wrote:
> Export via sysfs some information about the btrfs devices and
> filesystem.
> ---
>  fs/btrfs/super.c |    4 
>  fs/btrfs/sysfs.c |  933 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/sysfs.h |   31 ++
>  3 files changed, 963 insertions(+), 5 deletions(-)
>  create mode 100644 fs/btrfs/sysfs.h

Hi Goffredo,

The Signed-off-by line is missing, it is mandatory.

[...]
> +struct btrfs_sysfs_fsid {
> +		struct kobject			*kobj;
> +		struct list_head		list;
> +		u8						fsid[BTRFS_FSID_SIZE];
> +};

Tabsize needs to be 8.

> +static struct list_head btrfs_sysfs_device_list;
> +static struct list_head btrfs_sysfs_fs_devices_list;
> +static struct list_head btrfs_sysfs_filesystem_list;
> +static struct list_head btrfs_sysfs_fsid_list;
> +
> +
> +void uuid_unparse( u8 *uuid, char *out ){
> +	static char *i2x = "0123456789abcdef";
> +	static int 	lengths[] = {4,2,2,2,6,0};
> +	int 		i;
> +	
> +	for(i=0; ; i++){
> +		int j;
> +		for(j=0; j < lengths[i] ; j++, uuid++){
> +			*out++ = i2x[*uuid >> 4];
> +			*out++ = i2x[*uuid & 0x0f];
> +		}
> +		if( !lengths[i+1] ){
> +			*out = 0;
> +			break;
> +		}else{
> +			*out++ = '-';
> +		}
> +	}
> +}

All the missing spaces and all the spaces that are too much...
One line contains a trailing TAB character before the end of the line.
A newline is missing before the opening curly bracket of the function body.
An empty line between the 'int j;' and the code is common, like the one
after the 'int i;'.

[more lines deleted...]

Please refer to:
Documentation/SubmittingPatches
Documentation/SubmitChecklist
Documentation/CodingStyle

And apply:
scripts/checkpatch.pl

I did not look at the patch itself after noticing the style issues.

  reply	other threads:[~2012-08-23 14:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-23 12:12 [PATCH v0 0/4] BTRFS sysfs support Goffredo Baroncelli
2012-08-23 12:12 ` [PATCH v0 1/4] Add support for sysfs to btrfs Goffredo Baroncelli
2012-08-23 14:01   ` Stefan Behrens [this message]
2012-08-23 12:13 ` [PATCH v0 2/4] Add hook " Goffredo Baroncelli
2012-08-23 12:13 ` [PATCH v0 3/4] Add a new Kconfig section to enable or disable the sysfs Goffredo Baroncelli
2012-08-23 12:13 ` [PATCH v0 4/4] Btrfs sysfs support documentation Goffredo Baroncelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503637AC.7080103@giantdisaster.de \
    --to=sbehrens@giantdisaster.de \
    --cc=kreijack@inwind.it \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.