jffs2: fix sparse errors: directive in argument list

Submitted by Erico Nunes on Nov. 17, 2013, 8:18 p.m.

Details

Message ID 1384719513-27386-1-git-send-email-nunes.erico@gmail.com
State New, archived
Headers show

Commit Message

Erico Nunes Nov. 17, 2013, 8:18 p.m.
This patch fixes the following errors reported when running sparse:
fs/jffs2/super.c:378:1: error: directive in argument list
fs/jffs2/super.c:380:1: error: directive in argument list
fs/jffs2/super.c:381:1: error: directive in argument list
fs/jffs2/super.c:383:1: error: directive in argument list
---
 fs/jffs2/super.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

Joe Perches Nov. 17, 2013, 9:40 p.m.
On Sun, 2013-11-17 at 18:18 -0200, Erico Nunes wrote:
> This patch fixes the following errors reported when running sparse:
> fs/jffs2/super.c:378:1: error: directive in argument list
> fs/jffs2/super.c:380:1: error: directive in argument list
> fs/jffs2/super.c:381:1: error: directive in argument list
> fs/jffs2/super.c:383:1: error: directive in argument list

Seems like a sparse error more than anything else.
(adding sparse mailing list)

Also you need to add a "Signed-off-by:" line
with your email address.

see: Documentation/SubmittingPatches

> ---
>  fs/jffs2/super.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
> index 0defb1c..ec9e16f 100644
> --- a/fs/jffs2/super.c
> +++ b/fs/jffs2/super.c
> @@ -361,6 +361,14 @@ MODULE_ALIAS_FS("jffs2");
>  static int __init init_jffs2_fs(void)
>  {
>  	int ret;
> +	const char desc[] = "version 2.2."
> +#ifdef CONFIG_JFFS2_FS_WRITEBUFFER
> +		" (NAND)"
> +#endif
> +#ifdef CONFIG_JFFS2_SUMMARY
> +		" (SUMMARY) "
> +#endif
> +		" © 2001-2006 Red Hat, Inc.";
>  
>  	/* Paranoia checks for on-medium structures. If we ask GCC
>  	   to pack them with __attribute__((packed)) then it _also_
> @@ -374,14 +382,7 @@ static int __init init_jffs2_fs(void)
>  	BUILD_BUG_ON(sizeof(struct jffs2_raw_inode) != 68);
>  	BUILD_BUG_ON(sizeof(struct jffs2_raw_summary) != 32);
>  
> -	pr_info("version 2.2."
> -#ifdef CONFIG_JFFS2_FS_WRITEBUFFER
> -	       " (NAND)"
> -#endif
> -#ifdef CONFIG_JFFS2_SUMMARY
> -	       " (SUMMARY) "
> -#endif
> -	       " © 2001-2006 Red Hat, Inc.\n");
> +	pr_info("%s\n", desc);
>  
>  	jffs2_inode_cachep = kmem_cache_create("jffs2_i",
>  					     sizeof(struct jffs2_inode_info),
Erico Nunes Nov. 17, 2013, 10:34 p.m.
Thanks for your reply.

I'll resubmit shortly.

Do you mean it as an error in the sparse tool?
I don't think so, I took a look and it seems to be designed to output
an error for that case.


On Sun, Nov 17, 2013 at 7:40 PM, Joe Perches <joe@perches.com> wrote:
> On Sun, 2013-11-17 at 18:18 -0200, Erico Nunes wrote:
>> This patch fixes the following errors reported when running sparse:
>> fs/jffs2/super.c:378:1: error: directive in argument list
>> fs/jffs2/super.c:380:1: error: directive in argument list
>> fs/jffs2/super.c:381:1: error: directive in argument list
>> fs/jffs2/super.c:383:1: error: directive in argument list
>
> Seems like a sparse error more than anything else.
> (adding sparse mailing list)
>
> Also you need to add a "Signed-off-by:" line
> with your email address.
>
> see: Documentation/SubmittingPatches
>
>> ---
>>  fs/jffs2/super.c | 17 +++++++++--------
>>  1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
>> index 0defb1c..ec9e16f 100644
>> --- a/fs/jffs2/super.c
>> +++ b/fs/jffs2/super.c
>> @@ -361,6 +361,14 @@ MODULE_ALIAS_FS("jffs2");
>>  static int __init init_jffs2_fs(void)
>>  {
>>       int ret;
>> +     const char desc[] = "version 2.2."
>> +#ifdef CONFIG_JFFS2_FS_WRITEBUFFER
>> +             " (NAND)"
>> +#endif
>> +#ifdef CONFIG_JFFS2_SUMMARY
>> +             " (SUMMARY) "
>> +#endif
>> +             " © 2001-2006 Red Hat, Inc.";
>>
>>       /* Paranoia checks for on-medium structures. If we ask GCC
>>          to pack them with __attribute__((packed)) then it _also_
>> @@ -374,14 +382,7 @@ static int __init init_jffs2_fs(void)
>>       BUILD_BUG_ON(sizeof(struct jffs2_raw_inode) != 68);
>>       BUILD_BUG_ON(sizeof(struct jffs2_raw_summary) != 32);
>>
>> -     pr_info("version 2.2."
>> -#ifdef CONFIG_JFFS2_FS_WRITEBUFFER
>> -            " (NAND)"
>> -#endif
>> -#ifdef CONFIG_JFFS2_SUMMARY
>> -            " (SUMMARY) "
>> -#endif
>> -            " © 2001-2006 Red Hat, Inc.\n");
>> +     pr_info("%s\n", desc);
>>
>>       jffs2_inode_cachep = kmem_cache_create("jffs2_i",
>>                                            sizeof(struct jffs2_inode_info),
>
>
>
Joe Perches Nov. 17, 2013, 10:45 p.m.
On Sun, 2013-11-17 at 20:34 -0200, Erico Nunes wrote:
> Do you mean it as an error in the sparse tool?

Yes.  I think it's a defect in how sparse
treats string concatenation.

That style:

	printk("%s\n",
#ifdef FOO
	"foo"
#endif
#ifdef BAR
	"bar"
#endif
	"string");

is pretty common in the kernel sources.

The patch itself is otherwise fine, but
perhaps unnecessary.
richard -rw- weinberger Nov. 17, 2013, 10:53 p.m.
On Sun, Nov 17, 2013 at 11:45 PM, Joe Perches <joe@perches.com> wrote:
> On Sun, 2013-11-17 at 20:34 -0200, Erico Nunes wrote:
>> Do you mean it as an error in the sparse tool?
>
> Yes.  I think it's a defect in how sparse
> treats string concatenation.
>
> That style:
>
>         printk("%s\n",
> #ifdef FOO
>         "foo"
> #endif
> #ifdef BAR
>         "bar"
> #endif
>         "string");
>
> is pretty common in the kernel sources.
>
> The patch itself is otherwise fine, but
> perhaps unnecessary.

I agree with Joe, the patch fixes a non-issue.
Al Viro Nov. 18, 2013, 1:33 a.m.
On Sun, Nov 17, 2013 at 02:45:05PM -0800, Joe Perches wrote:
> On Sun, 2013-11-17 at 20:34 -0200, Erico Nunes wrote:
> > Do you mean it as an error in the sparse tool?
> 
> Yes.  I think it's a defect in how sparse
> treats string concatenation.
> 
> That style:
> 
> 	printk("%s\n",
> #ifdef FOO
> 	"foo"
> #endif
> #ifdef BAR
> 	"bar"
> #endif
> 	"string");
> 
> is pretty common in the kernel sources.

... and it's perfectly fine, until somebody starts playing in nasal
daemon country and do that in *macro* arguments.  And a nasal daemon
country it is - it's an undefined behaviour.  See 6.10.3p11 in C99.
And trying to define a semantics for that gets real ugly real fast.
sparse matches gcc behaviour (I hope), but it warns about such abuses.
It's a defect, all right - one being reported by sparse.

Folks, please, RTFStandard if you decide to play clever games with
preprocessing.  Chapter 6.10 is not particulary long or complicated.
C99 has improved the preprocessor semantics a whole lot compared to
the earlier horrible mess (mostly by defining it in terms of token
stream transformations rather then text ones), but it's still very
easy to abuse...

Patch hide | download patch | download mbox

diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 0defb1c..ec9e16f 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -361,6 +361,14 @@  MODULE_ALIAS_FS("jffs2");
 static int __init init_jffs2_fs(void)
 {
 	int ret;
+	const char desc[] = "version 2.2."
+#ifdef CONFIG_JFFS2_FS_WRITEBUFFER
+		" (NAND)"
+#endif
+#ifdef CONFIG_JFFS2_SUMMARY
+		" (SUMMARY) "
+#endif
+		" © 2001-2006 Red Hat, Inc.";
 
 	/* Paranoia checks for on-medium structures. If we ask GCC
 	   to pack them with __attribute__((packed)) then it _also_
@@ -374,14 +382,7 @@  static int __init init_jffs2_fs(void)
 	BUILD_BUG_ON(sizeof(struct jffs2_raw_inode) != 68);
 	BUILD_BUG_ON(sizeof(struct jffs2_raw_summary) != 32);
 
-	pr_info("version 2.2."
-#ifdef CONFIG_JFFS2_FS_WRITEBUFFER
-	       " (NAND)"
-#endif
-#ifdef CONFIG_JFFS2_SUMMARY
-	       " (SUMMARY) "
-#endif
-	       " © 2001-2006 Red Hat, Inc.\n");
+	pr_info("%s\n", desc);
 
 	jffs2_inode_cachep = kmem_cache_create("jffs2_i",
 					     sizeof(struct jffs2_inode_info),