Re: [Powertop] Deletion of unnecessary checks before specific function calls

Re: [Powertop] Deletion of unnecessary checks before specific function calls

[SF Markus Elfring]

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

> Thank you for sending your patch, it was added last night.

Thanks for your change acceptance.

https://github.com/fenrus75/powertop/commit/fb6f65a01eca950e2e12a87492268e7d2105aa0b

But I wonder why this software update was put into a single commit after I
offered separate patches. Did the commit message become partly inappropriate
because source code adjustments were squashed together?

Regards,
Markus

Re: [Powertop] Deletion of unnecessary checks before specific function calls

[Alexandra Yates]

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


> Hello,
>
> I suggest to delete redundant checks for null pointers.
> Would you like to integrate the attached update suggestion into your
> source code
> repository?
>
> Regards,
> Markus
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
>

Markus,

Thank you for sending your patch, it was added last night.

Thank you,
Alexandra.

Re: [Powertop] Deletion of unnecessary checks before specific function calls

[Sergey Senozhatsky]

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

On (10/19/14 00:50), SF Markus Elfring wrote:
> Date: Sun, 19 Oct 2014 00:50:29 +0200
> From: SF Markus Elfring <elfring(a)users.sourceforge.net>
> To: powertop(a)lists.01.org
> Subject: [Powertop] Deletion of unnecessary checks before specific function
>  calls
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101
>  Thunderbird/31.2.0
> 
> Hello,
> 
> I suggest to delete redundant checks for null pointers.
> Would you like to integrate the attached update suggestion into your source code
> repository?
> 
> Regards,
> Markus

makes sense.

	-ss

> From 1f7809ae8d1e4a14ad0f7a812e4f797f65597d1a Mon Sep 17 00:00:00 2001
> From: Markus Elfring <elfring(a)users.sourceforge.net>
> Date: Sun, 19 Oct 2014 00:30:53 +0200
> Subject: [PATCH 1/2] Deletion of unnecessary checks before calls of the
>  function "free"
> 
> The function "free" is documented in the way that no action shall occur for
> a passed null pointer. It is therefore not needed that a function caller
> repeats a corresponding check.
> http://stackoverflow.com/questions/18775608/free-a-null-pointer-anyway-or-check-first
> 
> Signed-off-by: Markus Elfring <elfring(a)users.sourceforge.net>
> ---
>  src/devlist.cpp          | 9 +++------
>  src/perf/perf.cpp        | 6 ++----
>  traceevent/event-parse.c | 3 +--
>  3 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/src/devlist.cpp b/src/devlist.cpp
> index 20fe5da..7f599ac 100644
> --- a/src/devlist.cpp
> +++ b/src/devlist.cpp
> @@ -77,18 +77,15 @@ void clean_open_devices()
>  	unsigned int i=0;
>  
>  	for (i = 0; i < one.size(); i++) {
> -		if(one[i])
> -			free(one[i]);
> +		free(one[i]);
>  	}
>  
>  	for (i = 0; i < two.size(); i++) {
> -		if(two[i])
> -			free(two[i]);
> +		free(two[i]);
>  	}
>  
>  	for (i = 0; i < devpower.size(); i++){
> -		if(devpower[i])
> -			free(devpower[i]);
> +		free(devpower[i]);
>  	}
>  }
>  
> diff --git a/src/perf/perf.cpp b/src/perf/perf.cpp
> index b176256..b1103a7 100644
> --- a/src/perf/perf.cpp
> +++ b/src/perf/perf.cpp
> @@ -149,8 +149,7 @@ void perf_event::create_perf_event(char *eventname, int _cpu)
>  
>  void perf_event::set_event_name(const char *event_name)
>  {
> -	if (name)
> -		free(name);
> +	free(name);
>  	name = strdup(event_name);
>  	if (!name) {
>  		fprintf(stderr, "failed to allocate event name\n");
> @@ -168,8 +167,7 @@ void perf_event::set_event_name(const char *event_name)
>  
>  perf_event::~perf_event(void)
>  {
> -	if (name)
> -		free(name);
> +	free(name);
>  
>  	if (perf_event::pevent->ref_count == 1) {
>  		pevent_free(perf_event::pevent);
> diff --git a/traceevent/event-parse.c b/traceevent/event-parse.c
> index 6195f6f..5a717a0 100644
> --- a/traceevent/event-parse.c
> +++ b/traceevent/event-parse.c
> @@ -1023,8 +1023,7 @@ static enum event_type force_token(const char *str, char **tok)
>  
>  static void free_token(char *tok)
>  {
> -	if (tok)
> -		free(tok);
> +	free(tok);
>  }
>  
>  static enum event_type read_token(char **tok)
> -- 
> 2.1.2
> 

> From 4237084ad74a1370617f60f1146fb9e59221f970 Mon Sep 17 00:00:00 2001
> From: Markus Elfring <elfring(a)users.sourceforge.net>
> Date: Sun, 19 Oct 2014 00:40:20 +0200
> Subject: [PATCH 2/2] Removal of unnecessary checks before a few calls of the
>  C++ delete operator
> 
> The C++ delete operator can also handle passed null pointers on its own
> without unexpected side effects.
> http://www.parashift.com/c++-faq/delete-handles-null.html
> 
> It is therefore not needed to keep additional safety checks directly before
> such function calls.
> 
> Signed-off-by: Markus Elfring <elfring(a)users.sourceforge.net>
> ---
>  src/cpu/abstract_cpu.cpp    | 6 ++----
>  src/report/report-maker.cpp | 6 ++----
>  2 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
> index 3b841bf..a3a9ffa 100644
> --- a/src/cpu/abstract_cpu.cpp
> +++ b/src/cpu/abstract_cpu.cpp
> @@ -34,14 +34,12 @@ abstract_cpu::~abstract_cpu()
>  {
>  	unsigned int i=0;
>  	for (i=0; i < cstates.size(); i++){
> -		if(cstates[i])
> -			delete cstates[i];
> +		delete cstates[i];
>  	}
>  	cstates.clear();
>  
>  	for (i=0; i < pstates.size(); i++){
> -		if(pstates[i])
> -			delete pstates[i];
> +		delete pstates[i];
>  	}
>  	pstates.clear();
>  }
> diff --git a/src/report/report-maker.cpp b/src/report/report-maker.cpp
> index f2fe522..4049a54 100644
> --- a/src/report/report-maker.cpp
> +++ b/src/report/report-maker.cpp
> @@ -47,8 +47,7 @@ report_maker::report_maker(report_type t)
>  
>  report_maker::~report_maker()
>  {
> -	if (formatter)
> -		delete formatter;
> +	delete formatter;
>  }
>  
>  /* ************************************************************************ */
> @@ -98,8 +97,7 @@ report_maker::set_type(report_type t)
>  void
>  report_maker::setup_report_formatter()
>  {
> -	if (formatter)
> -		delete formatter;
> +	delete formatter;
>  
>  	if (type == REPORT_HTML)
>  		formatter = new report_formatter_html();
> -- 
> 2.1.2
> 

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

[Powertop] Deletion of unnecessary checks before specific function calls

[SF Markus Elfring]

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

Hello,

I suggest to delete redundant checks for null pointers.
Would you like to integrate the attached update suggestion into your source code
repository?

Regards,
Markus

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Deletion-of-unnecessary-checks-before-calls-of-the-f.patch --]
[-- Type: text/x-patch, Size: 2268 bytes --]

>From 1f7809ae8d1e4a14ad0f7a812e4f797f65597d1a Mon Sep 17 00:00:00 2001
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 19 Oct 2014 00:30:53 +0200
Subject: [PATCH 1/2] Deletion of unnecessary checks before calls of the
 function "free"

The function "free" is documented in the way that no action shall occur for
a passed null pointer. It is therefore not needed that a function caller
repeats a corresponding check.
http://stackoverflow.com/questions/18775608/free-a-null-pointer-anyway-or-check-first

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 src/devlist.cpp          | 9 +++------
 src/perf/perf.cpp        | 6 ++----
 traceevent/event-parse.c | 3 +--
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/devlist.cpp b/src/devlist.cpp
index 20fe5da..7f599ac 100644
--- a/src/devlist.cpp
+++ b/src/devlist.cpp
@@ -77,18 +77,15 @@ void clean_open_devices()
 	unsigned int i=0;
 
 	for (i = 0; i < one.size(); i++) {
-		if(one[i])
-			free(one[i]);
+		free(one[i]);
 	}
 
 	for (i = 0; i < two.size(); i++) {
-		if(two[i])
-			free(two[i]);
+		free(two[i]);
 	}
 
 	for (i = 0; i < devpower.size(); i++){
-		if(devpower[i])
-			free(devpower[i]);
+		free(devpower[i]);
 	}
 }
 
diff --git a/src/perf/perf.cpp b/src/perf/perf.cpp
index b176256..b1103a7 100644
--- a/src/perf/perf.cpp
+++ b/src/perf/perf.cpp
@@ -149,8 +149,7 @@ void perf_event::create_perf_event(char *eventname, int _cpu)
 
 void perf_event::set_event_name(const char *event_name)
 {
-	if (name)
-		free(name);
+	free(name);
 	name = strdup(event_name);
 	if (!name) {
 		fprintf(stderr, "failed to allocate event name\n");
@@ -168,8 +167,7 @@ void perf_event::set_event_name(const char *event_name)
 
 perf_event::~perf_event(void)
 {
-	if (name)
-		free(name);
+	free(name);
 
 	if (perf_event::pevent->ref_count == 1) {
 		pevent_free(perf_event::pevent);
diff --git a/traceevent/event-parse.c b/traceevent/event-parse.c
index 6195f6f..5a717a0 100644
--- a/traceevent/event-parse.c
+++ b/traceevent/event-parse.c
@@ -1023,8 +1023,7 @@ static enum event_type force_token(const char *str, char **tok)
 
 static void free_token(char *tok)
 {
-	if (tok)
-		free(tok);
+	free(tok);
 }
 
 static enum event_type read_token(char **tok)
-- 
2.1.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Removal-of-unnecessary-checks-before-a-few-calls-of-.patch --]
[-- Type: text/x-patch, Size: 1859 bytes --]

>From 4237084ad74a1370617f60f1146fb9e59221f970 Mon Sep 17 00:00:00 2001
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 19 Oct 2014 00:40:20 +0200
Subject: [PATCH 2/2] Removal of unnecessary checks before a few calls of the
 C++ delete operator

The C++ delete operator can also handle passed null pointers on its own
without unexpected side effects.
http://www.parashift.com/c++-faq/delete-handles-null.html

It is therefore not needed to keep additional safety checks directly before
such function calls.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 src/cpu/abstract_cpu.cpp    | 6 ++----
 src/report/report-maker.cpp | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index 3b841bf..a3a9ffa 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -34,14 +34,12 @@ abstract_cpu::~abstract_cpu()
 {
 	unsigned int i=0;
 	for (i=0; i < cstates.size(); i++){
-		if(cstates[i])
-			delete cstates[i];
+		delete cstates[i];
 	}
 	cstates.clear();
 
 	for (i=0; i < pstates.size(); i++){
-		if(pstates[i])
-			delete pstates[i];
+		delete pstates[i];
 	}
 	pstates.clear();
 }
diff --git a/src/report/report-maker.cpp b/src/report/report-maker.cpp
index f2fe522..4049a54 100644
--- a/src/report/report-maker.cpp
+++ b/src/report/report-maker.cpp
@@ -47,8 +47,7 @@ report_maker::report_maker(report_type t)
 
 report_maker::~report_maker()
 {
-	if (formatter)
-		delete formatter;
+	delete formatter;
 }
 
 /* ************************************************************************ */
@@ -98,8 +97,7 @@ report_maker::set_type(report_type t)
 void
 report_maker::setup_report_formatter()
 {
-	if (formatter)
-		delete formatter;
+	delete formatter;
 
 	if (type == REPORT_HTML)
 		formatter = new report_formatter_html();
-- 
2.1.2