Patchwork ext4: remove redundant 'extern' keyword for function-definition

login
register
mail settings
Submitter Robin Dong
Date May 6, 2011, 8:19 a.m.
Message ID <1304669967-5677-1-git-send-email-sanbai@taobao.com>
Download mbox | patch
Permalink /patch/94344/
State Rejected
Headers show

Comments

Robin Dong - May 6, 2011, 8:19 a.m.
Signed-off-by: Robin Dong <sanbai@taobao.com>
---
 fs/ext4/ialloc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Theodore Ts'o - May 6, 2011, 10:50 a.m.
I don't really see the point to removing redundant keywords
like this.   To my mind, it's like fixing whitespaces.   If we are 
making changes to that part of the code, it's good to fix it,
and I'd rather not introduce such things in new code, but
as far as fixing code like this, i don't think an extra "extern"
is that harmful that it's worth fixing up.

What do other people think?

-- Ted


On May 6, 2011, at 4:19 AM, Robin Dong wrote:

> Signed-off-by: Robin Dong <sanbai@taobao.com>
> ---
> fs/ext4/ialloc.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 21bb2f6..ace7aef 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -1235,7 +1235,7 @@ unsigned long ext4_count_dirs(struct super_block * sb)
>  * inode allocation from the current group, so we take alloc_sem lock, to
>  * block ext4_claim_inode until we are finished.
>  */
> -extern int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
> +int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
> 				 int barrier)
> {
> 	struct ext4_group_info *grp = ext4_get_group_info(sb, group);
> -- 
> 1.7.1
> 
> --
> 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

--
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
Lukas Czerner - May 6, 2011, 11:36 a.m.
On Fri, 6 May 2011, Theodore Tso wrote:

> I don't really see the point to removing redundant keywords
> like this.   To my mind, it's like fixing whitespaces.   If we are 
> making changes to that part of the code, it's good to fix it,
> and I'd rather not introduce such things in new code, but
> as far as fixing code like this, i don't think an extra "extern"
> is that harmful that it's worth fixing up.
> 
> What do other people think?

Agreed! And since I am going to make some changes in lazyinit code anyway,
I can very well make that change (and of course give Robin the credit for
it:).

On the other hand I do not see a *huge* problem with commits like that, but
it is just additional unnecessary commit. Usually people complain that
git blame need some more digging with commits like this changing
otherwise perfectly sane code, but when one is going to make that change
when making some changes "at that area" it would mess up git blame
anyway.

Thanks!
-Lukas

> 
> -- Ted
> 
> 
> On May 6, 2011, at 4:19 AM, Robin Dong wrote:
> 
> > Signed-off-by: Robin Dong <sanbai@taobao.com>
> > ---
> > fs/ext4/ialloc.c |    2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> > index 21bb2f6..ace7aef 100644
> > --- a/fs/ext4/ialloc.c
> > +++ b/fs/ext4/ialloc.c
> > @@ -1235,7 +1235,7 @@ unsigned long ext4_count_dirs(struct super_block * sb)
> >  * inode allocation from the current group, so we take alloc_sem lock, to
> >  * block ext4_claim_inode until we are finished.
> >  */
> > -extern int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
> > +int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
> > 				 int barrier)
> > {
> > 	struct ext4_group_info *grp = ext4_get_group_info(sb, group);
> > -- 
> > 1.7.1
> > 
> > --
> > 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
> 
> --
> 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
> 
--
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
Eric Sandeen - May 6, 2011, 4:19 p.m.
On 5/6/11 6:36 AM, Lukas Czerner wrote:
> On Fri, 6 May 2011, Theodore Tso wrote:
> 
>> I don't really see the point to removing redundant keywords
>> like this.   To my mind, it's like fixing whitespaces.   If we are 
>> making changes to that part of the code, it's good to fix it,
>> and I'd rather not introduce such things in new code, but
>> as far as fixing code like this, i don't think an extra "extern"
>> is that harmful that it's worth fixing up.
>>
>> What do other people think?
> 
> Agreed! And since I am going to make some changes in lazyinit code anyway,
> I can very well make that change (and of course give Robin the credit for
> it:).
> 
> On the other hand I do not see a *huge* problem with commits like that, but
> it is just additional unnecessary commit. Usually people complain that
> git blame need some more digging with commits like this changing
> otherwise perfectly sane code, but when one is going to make that change
> when making some changes "at that area" it would mess up git blame
> anyway.
> 
> Thanks!
> -Lukas

FWIW, I had noticed this too, and there is another one in fsync.c:

fsync.c:extern int ext4_flush_completed_IO(struct inode *inode)

so, next time someone touches that code... ;)

-Eric
--
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/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 21bb2f6..ace7aef 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1235,7 +1235,7 @@  unsigned long ext4_count_dirs(struct super_block * sb)
  * inode allocation from the current group, so we take alloc_sem lock, to
  * block ext4_claim_inode until we are finished.
  */
-extern int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
+int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
 				 int barrier)
 {
 	struct ext4_group_info *grp = ext4_get_group_info(sb, group);