| Message ID | 57D281D1.6020208@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 09, 2016 at 05:33:05PM +0800, norton wrote: > Hi, all > > I'm a freshman in ext4 file system and I'm reading its source code now. > This patch did nothing but make it looks better.( > some lines are too long in vim :( ). We don't take line-remapping patches. It used to be the case that checkpatch would complain if lines were too long, and then it started complaining if the lines were wrapped. I don't like checkpatch, because it causes unnecessary patch churn. :-( May I suggest instead that you take a look at reconfiguring vim? http://stackoverflow.com/questions/467739/how-do-you-get-vim-to-display-wrapped-lines-without-inserting-newlines - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9/9/16 4:33 AM, norton wrote: > Hi, all > > I'm a freshman in ext4 file system and I'm reading its source code now. > This patch did nothing but make it looks better.( > some lines are too long in vim :( ). Aside from Ted's point about not taking patches which simply rewrap lines, it's worth knowing that the better practice, IMHO, when adding long text strings is not: + ext4_msg(sb, KERN_WARNING, "Remounting file " + "system with no journal so " + "ignoring journalled data option"); but: + ext4_msg(sb, KERN_WARNING, +"Remounting file system with no journal so ignoring journalled data option"); so that the strings can be found by a grep, if needed, while still avoiding 120-column lines. At least that's my time-tested opinion :) So if you ever find yourself in the position of needing to add a string like that, or are hitting it as part of other work, it's something to consider. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sep 9, 2016, at 3:33 AM, norton <norton.zhu@huawei.com> wrote: > > Hi, all > > I'm a freshman in ext4 file system and I'm reading its source code now. > This patch did nothing but make it looks better.( > some lines are too long in vim :( ). > > Thanks. > > Signed-off-by: Norton <norton.zhu@huawei.com> > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 3ec8708..09314f8 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -109,7 +109,8 @@ static void ext4_clear_request_list(void); > * transaction start -> page lock(s) -> i_data_sem (rw) > */ > > -#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT2) > +#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && \ > + defined(CONFIG_EXT4_USE_FOR_EXT2) > static struct file_system_type ext2_fs_type = { > .owner = THIS_MODULE, > .name = "ext2", > @@ -1751,7 +1752,9 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token, > } else if (m->flags & MOPT_DATAJ) { > if (is_remount) { > if (!sbi->s_journal) > - ext4_msg(sb, KERN_WARNING, "Remounting file system with no journal so ignoring journalled data option"); > + ext4_msg(sb, KERN_WARNING, "Remounting file " > + "system with no journal so " > + "ignoring journalled data option"); > else if (test_opt(sb, DATA_FLAGS) != m->mount_opt) { > ext4_msg(sb, KERN_ERR, > "Cannot change data mode on remount"); > @@ -5412,7 +5415,8 @@ static struct dentry *ext4_mount(struct file_system_type *fs_type, int flags, > return mount_bdev(fs_type, flags, dev_name, data, ext4_fill_super); > } > > -#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT2) > +#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && \ > + defined(CONFIG_EXT4_USE_FOR_EXT2) > static inline void register_as_ext2(void) > { > int err = register_filesystem(&ext2_fs_type); > @@ -5555,7 +5559,8 @@ static void __exit ext4_exit_fs(void) > ext4_exit_es(); > } > > -MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others"); > +MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, \ > + Theodore Ts'o and others"); In addition to the other comments from Ted and Eric, the above change is actually broken, as it would insert two tabs into the middle of the string. Firstly, the linefeed escape '\' is only for CPP, and not needed for C. Secondly, to split a string across lines you need to terminate it, and the compiler will merge adjacent strings: +MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, " + "Theodore Ts'o and others"); Don't forget the space inside the string at the end of the first line. Cheers, Andreas
Hi, Theodore Thanks for your suggest, I looks better now.:) On 2016/9/9 21:47, Theodore Ts'o wrote: > On Fri, Sep 09, 2016 at 05:33:05PM +0800, norton wrote: >> Hi, all >> >> I'm a freshman in ext4 file system and I'm reading its source code now. >> This patch did nothing but make it looks better.( >> some lines are too long in vim :( ). > > We don't take line-remapping patches. It used to be the case that > checkpatch would complain if lines were too long, and then it started > complaining if the lines were wrapped. I don't like checkpatch, > because it causes unnecessary patch churn. :-( > > May I suggest instead that you take a look at reconfiguring vim? > > http://stackoverflow.com/questions/467739/how-do-you-get-vim-to-display-wrapped-lines-without-inserting-newlines > > - Ted > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 3ec8708..09314f8 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -109,7 +109,8 @@ static void ext4_clear_request_list(void); * transaction start -> page lock(s) -> i_data_sem (rw) */ -#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT2) +#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && \ + defined(CONFIG_EXT4_USE_FOR_EXT2) static struct file_system_type ext2_fs_type = { .owner = THIS_MODULE, .name = "ext2", @@ -1751,7 +1752,9 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token, } else if (m->flags & MOPT_DATAJ) { if (is_remount) { if (!sbi->s_journal) - ext4_msg(sb, KERN_WARNING, "Remounting file system with no journal so ignoring journalled data option"); + ext4_msg(sb, KERN_WARNING, "Remounting file " + "system with no journal so " + "ignoring journalled data option"); else if (test_opt(sb, DATA_FLAGS) != m->mount_opt) { ext4_msg(sb, KERN_ERR, "Cannot change data mode on remount"); @@ -5412,7 +5415,8 @@ static struct dentry *ext4_mount(struct file_system_type *fs_type, int flags, return mount_bdev(fs_type, flags, dev_name, data, ext4_fill_super); } -#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && defined(CONFIG_EXT4_USE_FOR_EXT2) +#if !defined(CONFIG_EXT2_FS) && !defined(CONFIG_EXT2_FS_MODULE) && \ + defined(CONFIG_EXT4_USE_FOR_EXT2) static inline void register_as_ext2(void) { int err = register_filesystem(&ext2_fs_type); @@ -5555,7 +5559,8 @@ static void __exit ext4_exit_fs(void) ext4_exit_es(); } -MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others"); +MODULE_AUTHOR("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, \ + Theodore Ts'o and others"); MODULE_DESCRIPTION("Fourth Extended Filesystem"); MODULE_LICENSE("GPL"); module_init(ext4_init_fs)
Hi, all I'm a freshman in ext4 file system and I'm reading its source code now. This patch did nothing but make it looks better.( some lines are too long in vim :( ). Thanks. Signed-off-by: Norton <norton.zhu@huawei.com>