linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH]:ext4 these lines are too long while reading
@ 2016-09-09  9:33 norton
  2016-09-09 13:47 ` Theodore Ts'o
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: norton @ 2016-09-09  9:33 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, norton.zhu

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");
 MODULE_DESCRIPTION("Fourth Extended Filesystem");
 MODULE_LICENSE("GPL");
 module_init(ext4_init_fs)
-- 
1.8.4.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH]:ext4 these lines are too long while reading
  2016-09-09  9:33 [PATCH]:ext4 these lines are too long while reading norton
@ 2016-09-09 13:47 ` Theodore Ts'o
  2016-09-12  1:35   ` norton
  2016-09-09 14:34 ` Eric Sandeen
  2016-09-09 20:05 ` Andreas Dilger
  2 siblings, 1 reply; 5+ messages in thread
From: Theodore Ts'o @ 2016-09-09 13:47 UTC (permalink / raw)
  To: norton; +Cc: linux-ext4, linux-fsdevel

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]:ext4 these lines are too long while reading
  2016-09-09  9:33 [PATCH]:ext4 these lines are too long while reading norton
  2016-09-09 13:47 ` Theodore Ts'o
@ 2016-09-09 14:34 ` Eric Sandeen
  2016-09-09 20:05 ` Andreas Dilger
  2 siblings, 0 replies; 5+ messages in thread
From: Eric Sandeen @ 2016-09-09 14:34 UTC (permalink / raw)
  To: norton, linux-ext4; +Cc: linux-fsdevel

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]:ext4 these lines are too long while reading
  2016-09-09  9:33 [PATCH]:ext4 these lines are too long while reading norton
  2016-09-09 13:47 ` Theodore Ts'o
  2016-09-09 14:34 ` Eric Sandeen
@ 2016-09-09 20:05 ` Andreas Dilger
  2 siblings, 0 replies; 5+ messages in thread
From: Andreas Dilger @ 2016-09-09 20:05 UTC (permalink / raw)
  To: norton; +Cc: linux-ext4, linux-fsdevel

[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]

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






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]:ext4 these lines are too long while reading
  2016-09-09 13:47 ` Theodore Ts'o
@ 2016-09-12  1:35   ` norton
  0 siblings, 0 replies; 5+ messages in thread
From: norton @ 2016-09-12  1:35 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: linux-ext4, linux-fsdevel

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-09-12  1:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-09  9:33 [PATCH]:ext4 these lines are too long while reading norton
2016-09-09 13:47 ` Theodore Ts'o
2016-09-12  1:35   ` norton
2016-09-09 14:34 ` Eric Sandeen
2016-09-09 20:05 ` Andreas Dilger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).