2016-09-09 09:33:36

by norton

[permalink] [raw]
Subject: [PATCH]:ext4 these lines are too long while reading

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 <[email protected]>

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)
--
1.8.4.3



2016-09-09 13:48:11

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH]:ext4 these lines are too long while reading

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

2016-09-09 14:34:16

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH]:ext4 these lines are too long while reading

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



2016-09-09 20:06:02

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH]:ext4 these lines are too long while reading

On Sep 9, 2016, at 3:33 AM, norton <[email protected]> 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 <[email protected]>
>
> 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






Attachments:
signature.asc (833.00 B)
Message signed with OpenPGP using GPGMail

2016-09-12 01:35:44

by norton

[permalink] [raw]
Subject: Re: [PATCH]:ext4 these lines are too long while reading

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
>
> .
>