[PATCH] perf tools: fix type mismatch - long vs __statfs_word

[PATCH] perf tools: fix type mismatch - long vs __statfs_word

[Alexey Brodkin]

>From "include/uapi/asm-generic/statfs.h" it is seen that "statfs.f_type" is of
type "__statfs_word" which in its turn is "__u32" (unsigned int) for 32-bit
systems.

So in case of compilation with "-Werror" following breakage happens:
--->---
fs.c: In function ‘fs__valid_mount’:
fs.c:76:24: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  else if (st_fs.f_type != magic)
                        ^
cc1: all warnings being treated as errors
--->---

Note that now when fs.c is in "lib/api/fs" and in "tools/lib/api/Makefile"
CFLAGS has hard-coded "-Werror" this is inevitable even if one builds "perf"
with "WERROR=0".

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>

Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Cody P Schafer <dev@codyps.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index c1b49c3..4b2fa7b 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -75,7 +75,7 @@ static int fs__valid_mount(const char *fs, long magic)
 
 	if (statfs(fs, &st_fs) < 0)
 		return -ENOENT;
-	else if (st_fs.f_type != magic)
+	else if ((long)st_fs.f_type != magic)
 		return -ENOENT;
 
 	return 0;
-- 
1.9.3

Re: [PATCH] perf tools: fix type mismatch - long vs __statfs_word

[Vineet Gupta]

+CC acme, Peter


On Wednesday 13 August 2014 10:34 PM, Alexey Brodkin wrote:
> From "include/uapi/asm-generic/statfs.h" it is seen that "statfs.f_type" is of
> type "__statfs_word" which in its turn is "__u32" (unsigned int) for 32-bit
> systems.
>
> So in case of compilation with "-Werror" following breakage happens:
> --->---
> fs.c: In function ‘fs__valid_mount’:
> fs.c:76:24: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
>   else if (st_fs.f_type != magic)
>                         ^
> cc1: all warnings being treated as errors
> --->---
>
> Note that now when fs.c is in "lib/api/fs" and in "tools/lib/api/Makefile"
> CFLAGS has hard-coded "-Werror" this is inevitable even if one builds "perf"
> with "WERROR=0".
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
>
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Cody P Schafer <dev@codyps.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/lib/api/fs/fs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
> index c1b49c3..4b2fa7b 100644
> --- a/tools/lib/api/fs/fs.c
> +++ b/tools/lib/api/fs/fs.c
> @@ -75,7 +75,7 @@ static int fs__valid_mount(const char *fs, long magic)
>  
>  	if (statfs(fs, &st_fs) < 0)
>  		return -ENOENT;
> -	else if (st_fs.f_type != magic)
> +	else if ((long)st_fs.f_type != magic)
>  		return -ENOENT;
>  
>  	return 0;

Re: [PATCH] perf tools: fix type mismatch - long vs __statfs_word

[Alexey Brodkin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1116 bytes --]

Hi Arnaldo, Jiri,

> On Wednesday 13 August 2014 10:34 PM, Alexey Brodkin wrote:
> > From "include/uapi/asm-generic/statfs.h" it is seen that "statfs.f_type" is of
> > type "__statfs_word" which in its turn is "__u32" (unsigned int) for 32-bit
> > systems.
> >
> > So in case of compilation with "-Werror" following breakage happens:
> > --->---
> > fs.c: In function ‘fs__valid_mount’:
> > fs.c:76:24: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
> >   else if (st_fs.f_type != magic)
> >                         ^
> > cc1: all warnings being treated as errors
> > --->---
> >
> > Note that now when fs.c is in "lib/api/fs" and in "tools/lib/api/Makefile"
> > CFLAGS has hard-coded "-Werror" this is inevitable even if one builds "perf"
> > with "WERROR=0".
> >

I'm wondering if there's a chance to get this patch accepted because for
now perf building from 3.15 kernels onward is broken.

Regards,
Alexey
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] perf tools: fix type mismatch - long vs __statfs_word

[Arnaldo Carvalho de Melo]

Em Wed, Aug 13, 2014 at 09:03:49PM +0400, Alexey Brodkin escreveu:
> >From "include/uapi/asm-generic/statfs.h" it is seen that "statfs.f_type" is of
> type "__statfs_word" which in its turn is "__u32" (unsigned int) for 32-bit
> systems.
> 
> So in case of compilation with "-Werror" following breakage happens:
> --->---
> fs.c: In function ‘fs__valid_mount’:
> fs.c:76:24: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
>   else if (st_fs.f_type != magic)
>                         ^
> cc1: all warnings being treated as errors
> --->---
> 
> Note that now when fs.c is in "lib/api/fs" and in "tools/lib/api/Makefile"
> CFLAGS has hard-coded "-Werror" this is inevitable even if one builds "perf"
> with "WERROR=0".

So we have one more case of comparision involving f_type:

[acme@zoo linux]$ find tools/ -type f | xargs grep -w f_type
tools/lib/api/fs/fs.c:	else if (st_fs.f_type != magic)
tools/lib/api/fs/debugfs.c:	else if (st_fs.f_type != (long) DEBUGFS_MAGIC)
[acme@zoo linux]$

Do we have to apply the same fix to that other case as well?
Interestingly it does it on a different way, that by your description
above would make it fail on 32-bit systems, no?

- Arnaldo
 
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
> 
> Cc: Vineet Gupta <vgupta@synopsys.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Cody P Schafer <dev@codyps.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/lib/api/fs/fs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
> index c1b49c3..4b2fa7b 100644
> --- a/tools/lib/api/fs/fs.c
> +++ b/tools/lib/api/fs/fs.c
> @@ -75,7 +75,7 @@ static int fs__valid_mount(const char *fs, long magic)
>  
>  	if (statfs(fs, &st_fs) < 0)
>  		return -ENOENT;
> -	else if (st_fs.f_type != magic)
> +	else if ((long)st_fs.f_type != magic)
>  		return -ENOENT;
>  
>  	return 0;
> -- 
> 1.9.3