Re: [Powertop] [PATCH] src/cpu/cpu.cpp: Add suffix `u` to `4294967295` to signify an unsigned number

Re: [Powertop] [PATCH] src/cpu/cpu.cpp: Add suffix `u` to `4294967295` to signify an unsigned number

[Sergey Senozhatsky]

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

On (02/02/13 23:19), Paul Menzel wrote:
> Date: Sat, 2 Feb 2013 22:27:58 +0100
> 
> The C++ compiler
> 
>         $ g++ --version
>         g++-4.7.real (Debian 4.7.2-15) 4.7.2
>         Copyright (C) 2012 Free Software Foundation, Inc.
>         This is free software; see the source for copying conditions.  There is NO
>         warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> reports the following warning.
> 
>         $ make
>         […]
>           CXX    cpu/powertop-cpu.o
>         cpu/cpu.cpp:776:3: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]
>         […]
> 
> Adding the suffix to the constant gets rid of the warning [1].
> 
> [1] http://stackoverflow.com/questions/2347936/cant-get-rid-of-this-decimal-constant-is-unsigned-only-in-iso-c90-warning
> ---
>  src/cpu/cpu.cpp |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
> index f875e45..401bade 100644
> --- a/src/cpu/cpu.cpp
> +++ b/src/cpu/cpu.cpp
> @@ -797,7 +797,7 @@ void perf_power_bundle::handle_trace_point(void *trace, int cpunr, uint64_t time
>                          exit(-1);
>                  }
>  
> -		if (val == 4294967295)
> +		if (val == 4294967295u)

NACK.

cpu_idle tracepoint defined as

	 TP_PROTO(unsigned int state, unsigned int cpu_id)

please use `== (unsigned int)-1', which is, by the way, more clear than 4294967295.

	-ss


>  			cpu->go_unidle(time);
>  		else
>  			cpu->go_idle(time);
> -- 
> 1.7.10.4



> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop

Re: [Powertop] [PATCH] src/cpu/cpu.cpp: Add suffix `u` to `4294967295` to signify an unsigned number

[Paul Menzel]

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

Dear Sergey,


thank you for your review!


Am Sonntag, den 03.02.2013, 18:07 +0300 schrieb Sergey Senozhatsky:
> On (02/02/13 23:19), Paul Menzel wrote:
> > Date: Sat, 2 Feb 2013 22:27:58 +0100
> > 
> > The C++ compiler
> > 
> >         $ g++ --version
> >         g++-4.7.real (Debian 4.7.2-15) 4.7.2
> >         Copyright (C) 2012 Free Software Foundation, Inc.
> >         This is free software; see the source for copying conditions.  There is NO
> >         warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> > 
> > reports the following warning.
> > 
> >         $ make
> >         […]
> >           CXX    cpu/powertop-cpu.o
> >         cpu/cpu.cpp:776:3: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]
> >         […]
> > 
> > Adding the suffix to the constant gets rid of the warning [1].
> > 
> > [1] http://stackoverflow.com/questions/2347936/cant-get-rid-of-this-decimal-constant-is-unsigned-only-in-iso-c90-warning
> > ---
> >  src/cpu/cpu.cpp |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
> > index f875e45..401bade 100644
> > --- a/src/cpu/cpu.cpp
> > +++ b/src/cpu/cpu.cpp
> > @@ -797,7 +797,7 @@ void perf_power_bundle::handle_trace_point(void *trace, int cpunr, uint64_t time
> >                          exit(-1);
> >                  }
> >  
> > -		if (val == 4294967295)
> > +		if (val == 4294967295u)
> 
> NACK.
> 
> cpu_idle tracepoint defined as
> 
> 	 TP_PROTO(unsigned int state, unsigned int cpu_id)
> 
> please use `== (unsigned int)-1', which is, by the way, more clear than 4294967295.

Could you please elaborate your answer as I do not understand it. I
could not find that `TP_PROTO` definition in the PowerTOP source using
`git grep`.

Also beforehand `val` is defined as follows.

	unsigned long long val;


Thanks and sorry for not understanding it,

Paul

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[Powertop] [PATCH] src/cpu/cpu.cpp: Add suffix `u` to `4294967295` to signify an unsigned number

[Paul Menzel]

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

Date: Sat, 2 Feb 2013 22:27:58 +0100

The C++ compiler

        $ g++ --version
        g++-4.7.real (Debian 4.7.2-15) 4.7.2
        Copyright (C) 2012 Free Software Foundation, Inc.
        This is free software; see the source for copying conditions.  There is NO
        warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

reports the following warning.

        $ make
        […]
          CXX    cpu/powertop-cpu.o
        cpu/cpu.cpp:776:3: warning: this decimal constant is unsigned only in ISO C90 [enabled by default]
        […]

Adding the suffix to the constant gets rid of the warning [1].

[1] http://stackoverflow.com/questions/2347936/cant-get-rid-of-this-decimal-constant-is-unsigned-only-in-iso-c90-warning
---
 src/cpu/cpu.cpp |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cpu/cpu.cpp b/src/cpu/cpu.cpp
index f875e45..401bade 100644
--- a/src/cpu/cpu.cpp
+++ b/src/cpu/cpu.cpp
@@ -797,7 +797,7 @@ void perf_power_bundle::handle_trace_point(void *trace, int cpunr, uint64_t time
                         exit(-1);
                 }
 
-		if (val == 4294967295)
+		if (val == 4294967295u)
 			cpu->go_unidle(time);
 		else
 			cpu->go_idle(time);
-- 
1.7.10.4

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]