Re: [Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

Re: [Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

[Lekensteyn]

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

On Thursday 21 June 2012 22:14:41 Sergey Senozhatsky wrote:
> On (06/16/12 18:14), Magnus Fromreide wrote:
> > This undos the last printf->sprintf change from 8fd54ace.
> > 
> > Signed-off-by: Magnus Fromreide <magfr(a)lysator.liu.se>
> > ---
> > 
> >  src/main.cpp |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/main.cpp b/src/main.cpp
> > index 880d745..a2d8700 100644
> > --- a/src/main.cpp
> > +++ b/src/main.cpp
> > @@ -229,7 +229,7 @@ void one_measurement(int seconds, char *workload)
> > 
> >  void out_of_memory()
> >  {
> >  
> >  	reset_display();
> > 
> > -	sprintf("%s...\n",_("PowerTOP is out of memory. PowerTOP is 
Aborting"));
> > +	printf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
> > 
> >  	abort();
> >  
> >  }
> > 
> > @@ -458,7 +458,7 @@ int main(int argc, char **argv)
> > 
> >  #ifndef DISABLE_NCURSES
> >  
> >  	endwin();
> >  
> >  #endif
> > 
> > -	sprintf("%s\n", _("Leaving PowerTOP"));
> > +	printf("%s\n", _("Leaving PowerTOP"));
> > 
> >  	end_process_data();
> >  	clear_process_data();
> 
> Oh, good catch!
> 
> Looks like changes were introduced by commit
> 8fd54ace0a398cd155bf0af620c85d0105778f6a.

That commit contains more abuse of sprintf:
https://github.com/fenrus75/powertop/commit/8fd54ace0a398cd155bf0af620c85d0105778f6a#L1L102
Writing to a constant string is not going to work, eh?

Peter

Re: [Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

[Chris Ferron]

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

patch accepted thank you.

On 06/16/2012 09:14 AM, Magnus Fromreide wrote:
> This undos the last printf->sprintf change from 8fd54ace.
>
> Signed-off-by: Magnus Fromreide<magfr(a)lysator.liu.se>
> ---
>   src/main.cpp |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/main.cpp b/src/main.cpp
> index 880d745..a2d8700 100644
> --- a/src/main.cpp
> +++ b/src/main.cpp
> @@ -229,7 +229,7 @@ void one_measurement(int seconds, char *workload)
>   void out_of_memory()
>   {
>   	reset_display();
> -	sprintf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
> +	printf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
>   	abort();
>   }
>
> @@ -458,7 +458,7 @@ int main(int argc, char **argv)
>   #ifndef DISABLE_NCURSES
>   	endwin();
>   #endif
> -	sprintf("%s\n", _("Leaving PowerTOP"));
> +	printf("%s\n", _("Leaving PowerTOP"));
>
>   	end_process_data();
>   	clear_process_data();

Re: [Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

[Magnus Fromreide]

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

On Fri, 2012-06-22 at 08:45 +0200, Holger Schurig wrote:
> You guys should run powertop via clang's static analyzer :-)

Ok, I did.

It found a few bugs, but not these. On the other hand, the compiler
warned about these.

The warnings from g++ for this particular problem are:

main.cpp: In function ‘void out_of_memory()’:
main.cpp:234:72: warning: deprecated conversion from string constant to ‘char*’ 
[-Wwrite-strings]
main.cpp: In function ‘int main(int, char**)’:
main.cpp:465:39: warning: deprecated conversion from string constant to ‘char*’ 

>From clang++ I get:

main.cpp:234:10: warning: conversion from string literal to 'char *' is deprecated [-Wdeprecated-writable-strings]
        sprintf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
                ^
main.cpp:465:10: warning: conversion from string literal to 'char *' is deprecated [-Wdeprecated-writable-strings]
        sprintf("%s\n", _("Leaving PowerTOP"));
                ^

>From clang-analyze I get nothing.

The most interesting part is actually that the compilers reported this
as a warning and not an error.

/MF

Re: [Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

[Magnus Fromreide]

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

On Thu, 2012-06-21 at 21:42 +0200, Lekensteyn wrote:
> On Thursday 21 June 2012 22:14:41 Sergey Senozhatsky wrote:
> > On (06/16/12 18:14), Magnus Fromreide wrote:
> > > This undos the last printf->sprintf change from 8fd54ace.
> > > 
> > > Signed-off-by: Magnus Fromreide <magfr(a)lysator.liu.se>
> > > ---
> > > 
> > >  src/main.cpp |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/main.cpp b/src/main.cpp
> > > index 880d745..a2d8700 100644
> > > --- a/src/main.cpp
> > > +++ b/src/main.cpp
> > > @@ -229,7 +229,7 @@ void one_measurement(int seconds, char *workload)
> > > 
> > >  void out_of_memory()
> > >  {
> > >  
> > >  	reset_display();
> > > 
> > > -	sprintf("%s...\n",_("PowerTOP is out of memory. PowerTOP is 
> Aborting"));
> > > +	printf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
> > > 
> > >  	abort();
> > >  
> > >  }
> > > 
> > > @@ -458,7 +458,7 @@ int main(int argc, char **argv)
> > > 
> > >  #ifndef DISABLE_NCURSES
> > >  
> > >  	endwin();
> > >  
> > >  #endif
> > > 
> > > -	sprintf("%s\n", _("Leaving PowerTOP"));
> > > +	printf("%s\n", _("Leaving PowerTOP"));
> > > 
> > >  	end_process_data();
> > >  	clear_process_data();
> > 
> > Oh, good catch!
> > 
> > Looks like changes were introduced by commit
> > 8fd54ace0a398cd155bf0af620c85d0105778f6a.
> 
> That commit contains more abuse of sprintf:
> https://github.com/fenrus75/powertop/commit/8fd54ace0a398cd155bf0af620c85d0105778f6a#L1L102
> Writing to a constant string is not going to work, eh?

Yes, lots of sprintf abuse there but most of it is fixed in other
patches. I think these are the only ones left.

/MF

Re: [Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

[Sergey Senozhatsky]

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

On (06/16/12 18:14), Magnus Fromreide wrote:
> This undos the last printf->sprintf change from 8fd54ace.
> 
> Signed-off-by: Magnus Fromreide <magfr(a)lysator.liu.se>
> ---
>  src/main.cpp |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/main.cpp b/src/main.cpp
> index 880d745..a2d8700 100644
> --- a/src/main.cpp
> +++ b/src/main.cpp
> @@ -229,7 +229,7 @@ void one_measurement(int seconds, char *workload)
>  void out_of_memory()
>  {
>  	reset_display();
> -	sprintf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
> +	printf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
>  	abort();
>  }
>  
> @@ -458,7 +458,7 @@ int main(int argc, char **argv)
>  #ifndef DISABLE_NCURSES
>  	endwin();
>  #endif
> -	sprintf("%s\n", _("Leaving PowerTOP"));
> +	printf("%s\n", _("Leaving PowerTOP"));
>  
>  	end_process_data();
>  	clear_process_data();
>

Oh, good catch!

Looks like changes were introduced by commit
8fd54ace0a398cd155bf0af620c85d0105778f6a.


	-ss

Re: [Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

[Magnus Fromreide]

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

Hello!

Could someone please consider the commneted patch?

It stops powertop from crashing on shutdown.

/MF

[Powertop] [PATCH] Use printf, not sprintf, to print messages to stdout.

[Magnus Fromreide]

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

This undos the last printf->sprintf change from 8fd54ace.

Signed-off-by: Magnus Fromreide <magfr(a)lysator.liu.se>
---
 src/main.cpp |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main.cpp b/src/main.cpp
index 880d745..a2d8700 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -229,7 +229,7 @@ void one_measurement(int seconds, char *workload)
 void out_of_memory()
 {
 	reset_display();
-	sprintf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
+	printf("%s...\n",_("PowerTOP is out of memory. PowerTOP is Aborting"));
 	abort();
 }
 
@@ -458,7 +458,7 @@ int main(int argc, char **argv)
 #ifndef DISABLE_NCURSES
 	endwin();
 #endif
-	sprintf("%s\n", _("Leaving PowerTOP"));
+	printf("%s\n", _("Leaving PowerTOP"));
 
 	end_process_data();
 	clear_process_data();
-- 
1.7.10