Patchwork lib, acpi, hotkey: remove redundant null checks before free()

login
register
mail settings
Submitter Colin King
Date June 16, 2013, 12:02 p.m.
Message ID <1371384138-3061-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/251678/
State Accepted
Headers show

Comments

Colin King - June 16, 2013, 12:02 p.m.
From: Colin Ian King <colin.king@canonical.com>

Since free() on a null is a no-op, the null checks before free()
are redundant and a waste of code.  So remove the null checks.
Cleans up a bunch of smatch warnings too.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/crsdump/crsdump.c   |  4 +---
 src/acpi/method/method.c     |  3 +--
 src/acpi/pcc/pcc.c           |  4 +---
 src/acpi/wmi/wmi.c           |  4 +---
 src/hotkey/hotkey/hotkey.c   |  6 ++----
 src/lib/src/fwts_framework.c |  3 +--
 src/lib/src/fwts_klog.c      |  3 +--
 src/lib/src/fwts_log.c       | 10 +++-------
 8 files changed, 11 insertions(+), 26 deletions(-)
Alex Hung - June 17, 2013, 3:06 a.m.
On 06/16/2013 08:02 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Since free() on a null is a no-op, the null checks before free()
> are redundant and a waste of code.  So remove the null checks.
> Cleans up a bunch of smatch warnings too.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/crsdump/crsdump.c   |  4 +---
>   src/acpi/method/method.c     |  3 +--
>   src/acpi/pcc/pcc.c           |  4 +---
>   src/acpi/wmi/wmi.c           |  4 +---
>   src/hotkey/hotkey/hotkey.c   |  6 ++----
>   src/lib/src/fwts_framework.c |  3 +--
>   src/lib/src/fwts_klog.c      |  3 +--
>   src/lib/src/fwts_log.c       | 10 +++-------
>   8 files changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/src/acpi/crsdump/crsdump.c b/src/acpi/crsdump/crsdump.c
> index d6dd6df..94fbf0c 100644
> --- a/src/acpi/crsdump/crsdump.c
> +++ b/src/acpi/crsdump/crsdump.c
> @@ -869,9 +869,7 @@ static int crsdump_test1(fwts_framework *fw)
>   				else
>   					crsdump_small_resource_items(fw, name, data, obj->Buffer.Length);
>   			}
> -
> -			if (buf.Length && buf.Pointer)
> -				free(buf.Pointer);
> +			free(buf.Pointer);
>   		}
>   	}
>   	return FWTS_OK;
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 906ebc0..6afa864 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -439,8 +439,7 @@ static void method_evaluate_found_method(
>   			check_func(fw, name, &buf, obj, private);
>   		}
>   	}
> -	if (buf.Length && buf.Pointer)
> -		free(buf.Pointer);
> +	free(buf.Pointer);
>
>   	fwts_acpica_sem_count_get(&sem_acquired, &sem_released);
>   	if (sem_acquired != sem_released) {
> diff --git a/src/acpi/pcc/pcc.c b/src/acpi/pcc/pcc.c
> index 36eb654..4e4dd8a 100644
> --- a/src/acpi/pcc/pcc.c
> +++ b/src/acpi/pcc/pcc.c
> @@ -433,9 +433,7 @@ static int pcc_test1(fwts_framework *fw)
>   				if (ACPI_FAILURE(ret) == AE_OK) {
>   					pcc_check_buffer(fw, pcc_name, &buf);
>   					count++;
> -
> -					if (buf.Length && buf.Pointer)
> -	        				free(buf.Pointer);
> +	        			free(buf.Pointer);
>   				}
>   			}
>   		}
> diff --git a/src/acpi/wmi/wmi.c b/src/acpi/wmi/wmi.c
> index 2425be1..510e9d0 100644
> --- a/src/acpi/wmi/wmi.c
> +++ b/src/acpi/wmi/wmi.c
> @@ -380,9 +380,7 @@ static int wmi_test1(fwts_framework *fw)
>   					(uint8_t*)obj->Buffer.Pointer);
>   				wdg_found = true;
>   			}
> -
> -			if (buf.Length && buf.Pointer)
> -				free(buf.Pointer);
> +			free(buf.Pointer);
>   		}
>   	}
>
> diff --git a/src/hotkey/hotkey/hotkey.c b/src/hotkey/hotkey/hotkey.c
> index 381f029..70fcc78 100644
> --- a/src/hotkey/hotkey/hotkey.c
> +++ b/src/hotkey/hotkey/hotkey.c
> @@ -231,10 +231,8 @@ static int hotkey_deinit(fwts_framework *fw)
>   	FWTS_UNUSED(fw);
>
>   	fwts_keymap_free(hotkeys);
> -	if (hotkey_dev)
> -		free(hotkey_dev);
> -	if (hotkey_keymap)
> -		free(hotkey_keymap);
> +	free(hotkey_dev);
> +	free(hotkey_keymap);
>   	return FWTS_OK;
>   }
>
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index ff1e3b3..8042cf9 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -788,8 +788,7 @@ static void fwts_framework_strdup(char **ptr, const char *str)
>   	if (ptr == NULL)
>   		return;
>
> -	if (*ptr)
> -		free(*ptr);
> +	free(*ptr);
>   	*ptr = strdup(str);
>   }
>
> diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c
> index cd247e2..0472136 100644
> --- a/src/lib/src/fwts_klog.c
> +++ b/src/lib/src/fwts_klog.c
> @@ -469,8 +469,7 @@ static void fwts_klog_regex_find_callback(fwts_framework *fw, char *line, int re
>   			return;
>
>   		rc = pcre_exec(re, extra, line, strlen(line), 0, 0, vector, 1);
> -		if (extra)
> -			free(extra);
> +		free(extra);
>   		pcre_free(re);
>   		if (rc == 0)
>   			(*match)++;
> diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
> index b4a36ec..d8b7cc9 100644
> --- a/src/lib/src/fwts_log.c
> +++ b/src/lib/src/fwts_log.c
> @@ -482,8 +482,7 @@ int fwts_log_set_owner(fwts_log *log, const char *owner)
>   	if (log && (log->magic == LOG_MAGIC)) {
>   		char *newowner = strdup(owner);
>   		if (newowner) {
> -			if (log->owner)
> -				free(log->owner);
> +			free(log->owner);
>   			log->owner = newowner;
>   			return FWTS_OK;
>   		}
> @@ -580,8 +579,7 @@ char *fwts_log_get_filenames(const char *filename, const fwts_log_type type)
>   		fwts_log_type mask = 1 << i;
>   		if (type & mask) {
>   			if ((tmp = fwts_log_filename(filename, mask)) == NULL) {
> -				if (filenames)
> -					free(filenames);
> +				free(filenames);
>   				return NULL;
>   			}
>
> @@ -726,9 +724,7 @@ int fwts_log_close(fwts_log *log)
>   		/* ..and free log files */
>   		fwts_list_free_items(&log->log_files, free);
>
> -		if (log->owner)
> -			free(log->owner);
> -
> +		free(log->owner);
>   		free(log);
>   	}
>   	return FWTS_OK;
>
Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu - June 20, 2013, 7:46 a.m.
On 06/16/2013 08:02 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Since free() on a null is a no-op, the null checks before free()
> are redundant and a waste of code.  So remove the null checks.
> Cleans up a bunch of smatch warnings too.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/crsdump/crsdump.c   |  4 +---
>   src/acpi/method/method.c     |  3 +--
>   src/acpi/pcc/pcc.c           |  4 +---
>   src/acpi/wmi/wmi.c           |  4 +---
>   src/hotkey/hotkey/hotkey.c   |  6 ++----
>   src/lib/src/fwts_framework.c |  3 +--
>   src/lib/src/fwts_klog.c      |  3 +--
>   src/lib/src/fwts_log.c       | 10 +++-------
>   8 files changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/src/acpi/crsdump/crsdump.c b/src/acpi/crsdump/crsdump.c
> index d6dd6df..94fbf0c 100644
> --- a/src/acpi/crsdump/crsdump.c
> +++ b/src/acpi/crsdump/crsdump.c
> @@ -869,9 +869,7 @@ static int crsdump_test1(fwts_framework *fw)
>   				else
>   					crsdump_small_resource_items(fw, name, data, obj->Buffer.Length);
>   			}
> -
> -			if (buf.Length && buf.Pointer)
> -				free(buf.Pointer);
> +			free(buf.Pointer);
>   		}
>   	}
>   	return FWTS_OK;
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index 906ebc0..6afa864 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -439,8 +439,7 @@ static void method_evaluate_found_method(
>   			check_func(fw, name, &buf, obj, private);
>   		}
>   	}
> -	if (buf.Length && buf.Pointer)
> -		free(buf.Pointer);
> +	free(buf.Pointer);
>
>   	fwts_acpica_sem_count_get(&sem_acquired, &sem_released);
>   	if (sem_acquired != sem_released) {
> diff --git a/src/acpi/pcc/pcc.c b/src/acpi/pcc/pcc.c
> index 36eb654..4e4dd8a 100644
> --- a/src/acpi/pcc/pcc.c
> +++ b/src/acpi/pcc/pcc.c
> @@ -433,9 +433,7 @@ static int pcc_test1(fwts_framework *fw)
>   				if (ACPI_FAILURE(ret) == AE_OK) {
>   					pcc_check_buffer(fw, pcc_name, &buf);
>   					count++;
> -
> -					if (buf.Length && buf.Pointer)
> -	        				free(buf.Pointer);
> +	        			free(buf.Pointer);
>   				}
>   			}
>   		}
> diff --git a/src/acpi/wmi/wmi.c b/src/acpi/wmi/wmi.c
> index 2425be1..510e9d0 100644
> --- a/src/acpi/wmi/wmi.c
> +++ b/src/acpi/wmi/wmi.c
> @@ -380,9 +380,7 @@ static int wmi_test1(fwts_framework *fw)
>   					(uint8_t*)obj->Buffer.Pointer);
>   				wdg_found = true;
>   			}
> -
> -			if (buf.Length && buf.Pointer)
> -				free(buf.Pointer);
> +			free(buf.Pointer);
>   		}
>   	}
>
> diff --git a/src/hotkey/hotkey/hotkey.c b/src/hotkey/hotkey/hotkey.c
> index 381f029..70fcc78 100644
> --- a/src/hotkey/hotkey/hotkey.c
> +++ b/src/hotkey/hotkey/hotkey.c
> @@ -231,10 +231,8 @@ static int hotkey_deinit(fwts_framework *fw)
>   	FWTS_UNUSED(fw);
>
>   	fwts_keymap_free(hotkeys);
> -	if (hotkey_dev)
> -		free(hotkey_dev);
> -	if (hotkey_keymap)
> -		free(hotkey_keymap);
> +	free(hotkey_dev);
> +	free(hotkey_keymap);
>   	return FWTS_OK;
>   }
>
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index ff1e3b3..8042cf9 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -788,8 +788,7 @@ static void fwts_framework_strdup(char **ptr, const char *str)
>   	if (ptr == NULL)
>   		return;
>
> -	if (*ptr)
> -		free(*ptr);
> +	free(*ptr);
>   	*ptr = strdup(str);
>   }
>
> diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c
> index cd247e2..0472136 100644
> --- a/src/lib/src/fwts_klog.c
> +++ b/src/lib/src/fwts_klog.c
> @@ -469,8 +469,7 @@ static void fwts_klog_regex_find_callback(fwts_framework *fw, char *line, int re
>   			return;
>
>   		rc = pcre_exec(re, extra, line, strlen(line), 0, 0, vector, 1);
> -		if (extra)
> -			free(extra);
> +		free(extra);
>   		pcre_free(re);
>   		if (rc == 0)
>   			(*match)++;
> diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
> index b4a36ec..d8b7cc9 100644
> --- a/src/lib/src/fwts_log.c
> +++ b/src/lib/src/fwts_log.c
> @@ -482,8 +482,7 @@ int fwts_log_set_owner(fwts_log *log, const char *owner)
>   	if (log && (log->magic == LOG_MAGIC)) {
>   		char *newowner = strdup(owner);
>   		if (newowner) {
> -			if (log->owner)
> -				free(log->owner);
> +			free(log->owner);
>   			log->owner = newowner;
>   			return FWTS_OK;
>   		}
> @@ -580,8 +579,7 @@ char *fwts_log_get_filenames(const char *filename, const fwts_log_type type)
>   		fwts_log_type mask = 1 << i;
>   		if (type & mask) {
>   			if ((tmp = fwts_log_filename(filename, mask)) == NULL) {
> -				if (filenames)
> -					free(filenames);
> +				free(filenames);
>   				return NULL;
>   			}
>
> @@ -726,9 +724,7 @@ int fwts_log_close(fwts_log *log)
>   		/* ..and free log files */
>   		fwts_list_free_items(&log->log_files, free);
>
> -		if (log->owner)
> -			free(log->owner);
> -
> +		free(log->owner);
>   		free(log);
>   	}
>   	return FWTS_OK;
>
Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/acpi/crsdump/crsdump.c b/src/acpi/crsdump/crsdump.c
index d6dd6df..94fbf0c 100644
--- a/src/acpi/crsdump/crsdump.c
+++ b/src/acpi/crsdump/crsdump.c
@@ -869,9 +869,7 @@  static int crsdump_test1(fwts_framework *fw)
 				else
 					crsdump_small_resource_items(fw, name, data, obj->Buffer.Length);
 			}
-
-			if (buf.Length && buf.Pointer)
-				free(buf.Pointer);
+			free(buf.Pointer);
 		}
 	}
 	return FWTS_OK;
diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
index 906ebc0..6afa864 100644
--- a/src/acpi/method/method.c
+++ b/src/acpi/method/method.c
@@ -439,8 +439,7 @@  static void method_evaluate_found_method(
 			check_func(fw, name, &buf, obj, private);
 		}
 	}
-	if (buf.Length && buf.Pointer)
-		free(buf.Pointer);
+	free(buf.Pointer);
 
 	fwts_acpica_sem_count_get(&sem_acquired, &sem_released);
 	if (sem_acquired != sem_released) {
diff --git a/src/acpi/pcc/pcc.c b/src/acpi/pcc/pcc.c
index 36eb654..4e4dd8a 100644
--- a/src/acpi/pcc/pcc.c
+++ b/src/acpi/pcc/pcc.c
@@ -433,9 +433,7 @@  static int pcc_test1(fwts_framework *fw)
 				if (ACPI_FAILURE(ret) == AE_OK) {
 					pcc_check_buffer(fw, pcc_name, &buf);
 					count++;
-
-					if (buf.Length && buf.Pointer)
-	        				free(buf.Pointer);
+	        			free(buf.Pointer);
 				}
 			}
 		}
diff --git a/src/acpi/wmi/wmi.c b/src/acpi/wmi/wmi.c
index 2425be1..510e9d0 100644
--- a/src/acpi/wmi/wmi.c
+++ b/src/acpi/wmi/wmi.c
@@ -380,9 +380,7 @@  static int wmi_test1(fwts_framework *fw)
 					(uint8_t*)obj->Buffer.Pointer);
 				wdg_found = true;
 			}
-
-			if (buf.Length && buf.Pointer)
-				free(buf.Pointer);
+			free(buf.Pointer);
 		}
 	}
 
diff --git a/src/hotkey/hotkey/hotkey.c b/src/hotkey/hotkey/hotkey.c
index 381f029..70fcc78 100644
--- a/src/hotkey/hotkey/hotkey.c
+++ b/src/hotkey/hotkey/hotkey.c
@@ -231,10 +231,8 @@  static int hotkey_deinit(fwts_framework *fw)
 	FWTS_UNUSED(fw);
 
 	fwts_keymap_free(hotkeys);
-	if (hotkey_dev)
-		free(hotkey_dev);
-	if (hotkey_keymap)
-		free(hotkey_keymap);
+	free(hotkey_dev);
+	free(hotkey_keymap);
 	return FWTS_OK;
 }
 
diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
index ff1e3b3..8042cf9 100644
--- a/src/lib/src/fwts_framework.c
+++ b/src/lib/src/fwts_framework.c
@@ -788,8 +788,7 @@  static void fwts_framework_strdup(char **ptr, const char *str)
 	if (ptr == NULL)
 		return;
 
-	if (*ptr)
-		free(*ptr);
+	free(*ptr);
 	*ptr = strdup(str);
 }
 
diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c
index cd247e2..0472136 100644
--- a/src/lib/src/fwts_klog.c
+++ b/src/lib/src/fwts_klog.c
@@ -469,8 +469,7 @@  static void fwts_klog_regex_find_callback(fwts_framework *fw, char *line, int re
 			return;
 
 		rc = pcre_exec(re, extra, line, strlen(line), 0, 0, vector, 1);
-		if (extra)
-			free(extra);
+		free(extra);
 		pcre_free(re);
 		if (rc == 0)
 			(*match)++;
diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
index b4a36ec..d8b7cc9 100644
--- a/src/lib/src/fwts_log.c
+++ b/src/lib/src/fwts_log.c
@@ -482,8 +482,7 @@  int fwts_log_set_owner(fwts_log *log, const char *owner)
 	if (log && (log->magic == LOG_MAGIC)) {
 		char *newowner = strdup(owner);
 		if (newowner) {
-			if (log->owner)
-				free(log->owner);
+			free(log->owner);
 			log->owner = newowner;
 			return FWTS_OK;
 		}
@@ -580,8 +579,7 @@  char *fwts_log_get_filenames(const char *filename, const fwts_log_type type)
 		fwts_log_type mask = 1 << i;
 		if (type & mask) {
 			if ((tmp = fwts_log_filename(filename, mask)) == NULL) {
-				if (filenames)
-					free(filenames);
+				free(filenames);
 				return NULL;
 			}
 
@@ -726,9 +724,7 @@  int fwts_log_close(fwts_log *log)
 		/* ..and free log files */
 		fwts_list_free_items(&log->log_files, free);
 
-		if (log->owner)
-			free(log->owner);
-
+		free(log->owner);
 		free(log);
 	}
 	return FWTS_OK;