Patchwork [01/10] ext4: balloc: Fixed coding style issue

login
register
mail settings
Submitter Adil Mujeeb
Date Oct. 16, 2012, 7:12 p.m.
Message ID <1350414785-7848-1-git-send-email-mujeeb.adil@gmail.com>
Download mbox | patch
Permalink /patch/191845/
State Rejected
Headers show

Comments

Adil Mujeeb - Oct. 16, 2012, 7:12 p.m.
Fixed checkpatch.pl reported ERRORs

Signed-off-by: Adil Mujeeb <mujeeb.adil@gmail.com>
---
 linux-3.7-rc1/fs/ext4/balloc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Lukas Czerner - Oct. 17, 2012, 12:05 p.m.
On Wed, 17 Oct 2012, Adil Mujeeb wrote:

> Date: Wed, 17 Oct 2012 00:42:56 +0530
> From: Adil Mujeeb <mujeeb.adil@gmail.com>
> To: tytso@mit.edu, adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
>     linux-kernel@vger.kernel.org
> Cc: Adil Mujeeb <mujeeb.adil@gmail.com>
> Subject: [PATCH 01/10] ext4: balloc: Fixed coding style issue
> 
> Fixed checkpatch.pl reported ERRORs

Hi Adil,

let me ask you something. How useful do you think those changes are ?
Have you learned anything by creating those patches ?

Just to clarify why I am asking such weird questions. It's not one of
those sneer questions, I would really like to know.

Thanks!
-Lukas

> 
> Signed-off-by: Adil Mujeeb <mujeeb.adil@gmail.com>
> ---
>  linux-3.7-rc1/fs/ext4/balloc.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-3.7-rc1/fs/ext4/balloc.c b/linux-3.7-rc1/fs/ext4/balloc.c
> index 1b50890..395418d 100644
> --- a/linux-3.7-rc1/fs/ext4/balloc.c
> +++ b/linux-3.7-rc1/fs/ext4/balloc.c
> @@ -246,7 +246,7 @@ unsigned ext4_free_clusters_after_init(struct super_block *sb,
>   * @bh:			pointer to the buffer head to store the block
>   *			group descriptor
>   */
> -struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
> +struct ext4_group_desc *ext4_get_group_desc(struct super_block *sb,
>  					     ext4_group_t block_group,
>  					     struct buffer_head **bh)
>  {
> @@ -700,7 +700,7 @@ static unsigned long ext4_bg_num_gdb_nometa(struct super_block *sb,
>  	if (!ext4_bg_has_super(sb, group))
>  		return 0;
>  
> -	if (EXT4_HAS_INCOMPAT_FEATURE(sb,EXT4_FEATURE_INCOMPAT_META_BG))
> +	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_META_BG))
>  		return le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
>  	else
>  		return EXT4_SB(sb)->s_gdb_count;
> @@ -721,11 +721,11 @@ unsigned long ext4_bg_num_gdb(struct super_block *sb, ext4_group_t group)
>  			le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
>  	unsigned long metagroup = group / EXT4_DESC_PER_BLOCK(sb);
>  
> -	if (!EXT4_HAS_INCOMPAT_FEATURE(sb,EXT4_FEATURE_INCOMPAT_META_BG) ||
> +	if (!EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_META_BG) ||
>  			metagroup < first_meta_bg)
>  		return ext4_bg_num_gdb_nometa(sb, group);
>  
> -	return ext4_bg_num_gdb_meta(sb,group);
> +	return ext4_bg_num_gdb_meta(sb, group);
>  
>  }
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
richard -rw- weinberger - Oct. 17, 2012, 12:13 p.m.
On Wed, Oct 17, 2012 at 2:05 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> On Wed, 17 Oct 2012, Adil Mujeeb wrote:
>
>> Date: Wed, 17 Oct 2012 00:42:56 +0530
>> From: Adil Mujeeb <mujeeb.adil@gmail.com>
>> To: tytso@mit.edu, adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
>>     linux-kernel@vger.kernel.org
>> Cc: Adil Mujeeb <mujeeb.adil@gmail.com>
>> Subject: [PATCH 01/10] ext4: balloc: Fixed coding style issue
>>
>> Fixed checkpatch.pl reported ERRORs
>
> Hi Adil,
>
> let me ask you something. How useful do you think those changes are ?
> Have you learned anything by creating those patches ?
>
> Just to clarify why I am asking such weird questions. It's not one of
> those sneer questions, I would really like to know.
>>  linux-3.7-rc1/fs/ext4/balloc.c |    8 ++++----

The really sad thing is that this patch does not even apply because
the path is malformed.
Lukas Czerner - Oct. 17, 2012, 2:27 p.m.
On Wed, 17 Oct 2012, Adil Mujeeb wrote:

> Date: Wed, 17 Oct 2012 19:35:40 +0530
> From: Adil Mujeeb <mujeeb.adil@gmail.com>
> To: richard -rw- weinberger <richard.weinberger@gmail.com>,
>     Lukáš Czerner <lczerner@redhat.com>
> Subject: Re: [PATCH 01/10] ext4: balloc: Fixed coding style issue
> 
> Hi,
> 
> On Wed, Oct 17, 2012 at 5:43 PM, richard -rw- weinberger
> <richard.weinberger@gmail.com> wrote:
> > On Wed, Oct 17, 2012 at 2:05 PM, Lukáš Czerner <lczerner@redhat.com> wrote:
> >> On Wed, 17 Oct 2012, Adil Mujeeb wrote:
> >>
> >>> Date: Wed, 17 Oct 2012 00:42:56 +0530
> >>> From: Adil Mujeeb <mujeeb.adil@gmail.com>
> >>> To: tytso@mit.edu, adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
> >>>     linux-kernel@vger.kernel.org
> >>> Cc: Adil Mujeeb <mujeeb.adil@gmail.com>
> >>> Subject: [PATCH 01/10] ext4: balloc: Fixed coding style issue
> >>>
> >>> Fixed checkpatch.pl reported ERRORs
> >>
> >> Hi Adil,
> >>
> >> let me ask you something. How useful do you think those changes are ?
> >> Have you learned anything by creating those patches ?
> 
> I am newbie. I just thought of starting with cleanup thing. I know
> that from functionality point of view, I am not adding anything but
> thought if it helps in coding guideline compliance and uniformity of
> code.
> Now I understand that this is not required. In future, I will avoid
> submitting such patches.

So you've learned something :). But seriously such patches are
causing more problems than it solves. It's perfectly fine to clean
up the code in the area you're changing anyway, but making coding
style patches just for a sake of it is not usually useful. Moreover
it is not useful for you as well, because you're not going to learn
anything.

There are plenty of real problem to be solved, but it actually
involves reading and understanding the code first.

Just for example:

 - get familiar with xfstests
 - there might still be problems with unified ext4 driver where
   we might use options not suited for respective file system
 - there are still issues with bigalloc
 - I suspect that there will be some problems with file system >16TB
 - recently there has been some reports from bigzilla.kernel.org
 - or you can just read the code, trying to understand how it works
   and soon or later you will find something to fix :) I am sure
   about that.

Maybe someone else have better suggestions...

Good luck!
-Lukas

> 
> >>
> >> Just to clarify why I am asking such weird questions. It's not one of
> >> those sneer questions, I would really like to know.
> >>>  linux-3.7-rc1/fs/ext4/balloc.c |    8 ++++----
> >
> > The really sad thing is that this patch does not even apply because
> > the path is malformed.
> 
> I am sorry, seems I am missing something. I tried with my local repo
> and it worked :(
> Its not the right place to ask, so I'll look what went wrong with the patches.
> 
> Rgds,
> Adil
> 
> >
> > --
> > Thanks,
> > //richard
>
Adil Mujeeb - Oct. 17, 2012, 4:52 p.m.
Hi,

>> >> Hi Adil,
>> >>
>> >> let me ask you something. How useful do you think those changes are ?
>> >> Have you learned anything by creating those patches ?
>>
>> I am newbie. I just thought of starting with cleanup thing. I know
>> that from functionality point of view, I am not adding anything but
>> thought if it helps in coding guideline compliance and uniformity of
>> code.
>> Now I understand that this is not required. In future, I will avoid
>> submitting such patches.
>
> So you've learned something :). But seriously such patches are
> causing more problems than it solves. It's perfectly fine to clean
> up the code in the area you're changing anyway, but making coding
> style patches just for a sake of it is not usually useful. Moreover
> it is not useful for you as well, because you're not going to learn
> anything.

I agree with you, it makes sense.

>
> There are plenty of real problem to be solved, but it actually
> involves reading and understanding the code first.
>
> Just for example:
>
>  - get familiar with xfstests
>  - there might still be problems with unified ext4 driver where
>    we might use options not suited for respective file system
>  - there are still issues with bigalloc
>  - I suspect that there will be some problems with file system >16TB
>  - recently there has been some reports from bigzilla.kernel.org
>  - or you can just read the code, trying to understand how it works
>    and soon or later you will find something to fix :) I am sure
>    about that.
>
> Maybe someone else have better suggestions...

Thanks a lot Lukas for your suggestions. I'll definitely look into
that as I really wanted to learn and contribute. I have seen bits and
pieces in xfs but didnt get more documentation on implementation part,
shall try to work upon that. More suggestions for newbie's like me,
are welcome :)

Thanks a lot.

Rgds,
Adil

>
> Good luck!
> -Lukas
>
>>
>> >>
>> >> Just to clarify why I am asking such weird questions. It's not one of
>> >> those sneer questions, I would really like to know.
>> >>>  linux-3.7-rc1/fs/ext4/balloc.c |    8 ++++----
>> >
>> > The really sad thing is that this patch does not even apply because
>> > the path is malformed.
>>
>> I am sorry, seems I am missing something. I tried with my local repo
>> and it worked :(
>> Its not the right place to ask, so I'll look what went wrong with the patches.
>>
>> Rgds,
>> Adil
>>
>> >
>> > --
>> > Thanks,
>> > //richard
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/linux-3.7-rc1/fs/ext4/balloc.c b/linux-3.7-rc1/fs/ext4/balloc.c
index 1b50890..395418d 100644
--- a/linux-3.7-rc1/fs/ext4/balloc.c
+++ b/linux-3.7-rc1/fs/ext4/balloc.c
@@ -246,7 +246,7 @@  unsigned ext4_free_clusters_after_init(struct super_block *sb,
  * @bh:			pointer to the buffer head to store the block
  *			group descriptor
  */
-struct ext4_group_desc * ext4_get_group_desc(struct super_block *sb,
+struct ext4_group_desc *ext4_get_group_desc(struct super_block *sb,
 					     ext4_group_t block_group,
 					     struct buffer_head **bh)
 {
@@ -700,7 +700,7 @@  static unsigned long ext4_bg_num_gdb_nometa(struct super_block *sb,
 	if (!ext4_bg_has_super(sb, group))
 		return 0;
 
-	if (EXT4_HAS_INCOMPAT_FEATURE(sb,EXT4_FEATURE_INCOMPAT_META_BG))
+	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_META_BG))
 		return le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
 	else
 		return EXT4_SB(sb)->s_gdb_count;
@@ -721,11 +721,11 @@  unsigned long ext4_bg_num_gdb(struct super_block *sb, ext4_group_t group)
 			le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
 	unsigned long metagroup = group / EXT4_DESC_PER_BLOCK(sb);
 
-	if (!EXT4_HAS_INCOMPAT_FEATURE(sb,EXT4_FEATURE_INCOMPAT_META_BG) ||
+	if (!EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_META_BG) ||
 			metagroup < first_meta_bg)
 		return ext4_bg_num_gdb_nometa(sb, group);
 
-	return ext4_bg_num_gdb_meta(sb,group);
+	return ext4_bg_num_gdb_meta(sb, group);
 
 }