[PATCH] acpi: s3, s3power, s4: replace cleanup free with explicit frees

Colin King colin.king at canonical.com
Wed Dec 16 14:57:45 UTC 2015


From: Colin Ian King <colin.king at canonical.com>

The fwts idiom is to follow the kernel style of error handling by
jumping to a common return point that free's resources.  Use this
explicit free'ing error handling idiom rather than using GCC
cleanup attributes to follow the fwts code style and so that we
don't get a lot of false positives from various static code
analysis tools.

Signed-off-by: Colin Ian King <colin.king at canonical.com>
---
 src/acpi/s3/s3.c                 | 41 +++++++++++++++++++++++-----------
 src/acpi/s3power/s3power.c       | 16 +++++++++-----
 src/acpi/s4/s4.c                 | 34 +++++++++++++++++++---------
 src/lib/include/fwts_pm_method.h | 48 +++++++++++-----------------------------
 4 files changed, 75 insertions(+), 64 deletions(-)

diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c
index 8e224cf..3742eec 100644
--- a/src/acpi/s3/s3.c
+++ b/src/acpi/s3/s3.c
@@ -153,9 +153,10 @@ static int s3_do_suspend_resume(fwts_framework *fw,
 	int status;
 	int duration;
 	int differences;
-	_cleanup_free_ char *command = NULL;
-	_cleanup_free_ char *quirks = NULL;
-	_cleanup_free_pm_vars_ fwts_pm_method_vars * fwts_settings = NULL;
+	int rc = FWTS_OK;
+	char *command = NULL;
+	char *quirks = NULL;
+	fwts_pm_method_vars *fwts_settings = NULL;
 
 	int (*do_suspend)(fwts_pm_method_vars *, const int, int*, const char*);
 
@@ -176,7 +177,8 @@ static int s3_do_suspend_resume(fwts_framework *fw,
 			fwts_log_info(fw, "Using logind as the default power method.");
 			if (fwts_logind_init_proxy(fwts_settings) != 0) {
 				fwts_log_error(fw, "Failure to connect to Logind.");
-				return FWTS_ERROR;
+				rc = FWTS_ERROR;
+				goto tidy;
 			}
 			do_suspend = &wrap_logind_do_suspend;
 			break;
@@ -202,22 +204,30 @@ static int s3_do_suspend_resume(fwts_framework *fw,
 	/* Format up pm-suspend command with optional quirking arguments */
 	if (fw->pm_method == FWTS_PM_PMUTILS) {
 		if (s3_hybrid) {
-			if ((command = fwts_realloc_strcat(NULL, PM_SUSPEND_HYBRID_PMUTILS)) == NULL)
-				return FWTS_OUT_OF_MEMORY;
+			if ((command = fwts_realloc_strcat(NULL, PM_SUSPEND_HYBRID_PMUTILS)) == NULL) {
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
+			}
 		} else {
-			if ((command = fwts_realloc_strcat(NULL, PM_SUSPEND_PMUTILS)) == NULL)
-				return FWTS_OUT_OF_MEMORY;
+			if ((command = fwts_realloc_strcat(NULL, PM_SUSPEND_PMUTILS)) == NULL) {
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
+			}
 		}
 
 		/* For now we only support quirks with pm-utils */
 		if (s3_quirks) {
-			if ((command = fwts_realloc_strcat(command, " ")) == NULL)
-				return FWTS_OUT_OF_MEMORY;
+			if ((command = fwts_realloc_strcat(command, " ")) == NULL) {
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
+			}
 			if ((quirks = fwts_args_comma_list(s3_quirks)) == NULL) {
-				return FWTS_OUT_OF_MEMORY;
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
 			}
 			if ((command = fwts_realloc_strcat(command, quirks)) == NULL) {
-				return FWTS_OUT_OF_MEMORY;
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
 			}
 		}
 	}
@@ -294,7 +304,12 @@ static int s3_do_suspend_resume(fwts_framework *fw,
 			"enter the requested power saving state.");
 	}
 
-	return FWTS_OK;
+tidy:
+	free(command);
+	free(quirks);
+	free_pm_method_vars(fwts_settings);
+
+	return rc;
 }
 
 static int s3_scan_times(
diff --git a/src/acpi/s3power/s3power.c b/src/acpi/s3power/s3power.c
index ba5f1bb..c9b1c0c 100644
--- a/src/acpi/s3power/s3power.c
+++ b/src/acpi/s3power/s3power.c
@@ -242,6 +242,7 @@ static int s3power_test(fwts_framework *fw)
 {
 	int status;
 	int duration;
+	int rc = FWTS_OK;
 
 	bool offline;
 
@@ -250,7 +251,7 @@ static int s3power_test(fwts_framework *fw)
 	uint32_t capacity_before_mWh;
 	uint32_t capacity_after_mWh;
 
-	_cleanup_free_pm_vars_ fwts_pm_method_vars * fwts_settings = NULL;
+	fwts_pm_method_vars *fwts_settings = NULL;
 
 	int (*do_suspend)(fwts_pm_method_vars *, const int, int*, const char*);
 
@@ -278,7 +279,8 @@ static int s3power_test(fwts_framework *fw)
 			fwts_log_info(fw, "Using logind as the default power method.");
 			if (fwts_logind_init_proxy(fwts_settings) != 0) {
 				fwts_log_error(fw, "Failure to connect to Logind.");
-				return FWTS_ERROR;
+				rc = FWTS_ERROR;
+				goto tidy;
 			}
 			do_suspend = &wrap_logind_do_suspend;
 			break;
@@ -300,11 +302,13 @@ static int s3power_test(fwts_framework *fw)
 
 	if (s3power_wait_for_adapter_offline(fw, &offline) == FWTS_ERROR) {
 		fwts_log_error(fw, "Cannot check if machine is running on battery, aborting test.");
-		return FWTS_ABORTED;
+		rc = FWTS_ABORTED;
+		goto tidy;
 	}
 	if (!offline) {
 		fwts_log_error(fw, "Machine needs to be running on battery to run test, aborting test.");
-		return FWTS_ABORTED;
+		rc = FWTS_ABORTED;
+		goto tidy;
 	}
 
 	s3power_get_remaining_capacity(fw, &capacity_before_mAh, &capacity_before_mWh);
@@ -343,8 +347,10 @@ static int s3power_test(fwts_framework *fw)
 			"pm-action encountered an error and also failed to "
 			"enter the requested power saving state.");
 	}
+tidy:
+	free_pm_method_vars(fwts_settings);
 
-	return FWTS_OK;
+	return rc;
 }
 
 static int s3power_options_check(fwts_framework *fw)
diff --git a/src/acpi/s4/s4.c b/src/acpi/s4/s4.c
index 198fec1..71d5a25 100644
--- a/src/acpi/s4/s4.c
+++ b/src/acpi/s4/s4.c
@@ -178,9 +178,10 @@ static int s4_hibernate(fwts_framework *fw,
 	int status;
 	int duration;
 	int differences;
-	_cleanup_free_ char *command = NULL;
-	_cleanup_free_ char *quirks = NULL;
-	_cleanup_free_pm_vars_ fwts_pm_method_vars * fwts_settings = NULL;
+	int rc = FWTS_OK;
+	char *command = NULL;
+	char *quirks = NULL;
+	fwts_pm_method_vars *fwts_settings = NULL;
 
 	int (*do_s4)(fwts_pm_method_vars *, const int, int*, const char*);
 
@@ -201,7 +202,8 @@ static int s4_hibernate(fwts_framework *fw,
 			fwts_log_info(fw, "Using logind as the default power method.");
 			if (fwts_logind_init_proxy(fwts_settings) != 0) {
 				fwts_log_error(fw, "Failure to connect to Logind.");
-				return FWTS_ERROR;
+				rc = FWTS_ERROR;
+				goto tidy;
 			}
 			do_s4 = &wrap_logind_do_s4;
 			break;
@@ -226,18 +228,24 @@ static int s4_hibernate(fwts_framework *fw,
 
 	if (fw->pm_method == FWTS_PM_PMUTILS) {
 		/* Format up pm-hibernate command with optional quirking arguments */
-		if ((command = fwts_realloc_strcat(NULL, PM_HIBERNATE)) == NULL)
-			return FWTS_OUT_OF_MEMORY;
+		if ((command = fwts_realloc_strcat(NULL, PM_HIBERNATE)) == NULL) {
+			rc = FWTS_OUT_OF_MEMORY;
+			goto tidy;
+		}
 
 		/* For now we only support quirks with pm-utils */
 		if (s4_quirks) {
-			if ((command = fwts_realloc_strcat(command, " ")) == NULL)
-				return FWTS_OUT_OF_MEMORY;
+			if ((command = fwts_realloc_strcat(command, " ")) == NULL) {
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
+			}
 			if ((quirks = fwts_args_comma_list(s4_quirks)) == NULL) {
-				return FWTS_OUT_OF_MEMORY;
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
 			}
 			if ((command = fwts_realloc_strcat(command, quirks)) == NULL) {
-				return FWTS_OUT_OF_MEMORY;
+				rc = FWTS_OUT_OF_MEMORY;
+				goto tidy;
 			}
 		}
 	}
@@ -336,8 +344,12 @@ static int s4_hibernate(fwts_framework *fw,
 	fwts_klog_free(klog_pre);
 	fwts_klog_free(klog_post);
 	fwts_list_free(klog_diff, NULL);
+tidy:
+	free(command);
+	free(quirks);
+	free_pm_method_vars(fwts_settings);
 
-	return FWTS_OK;
+	return rc;
 }
 
 static int s4_test_multiple(fwts_framework *fw)
diff --git a/src/lib/include/fwts_pm_method.h b/src/lib/include/fwts_pm_method.h
index 97041cc..fc8a638 100644
--- a/src/lib/include/fwts_pm_method.h
+++ b/src/lib/include/fwts_pm_method.h
@@ -44,52 +44,30 @@ typedef struct
 	int  min_delay;
 } fwts_pm_method_vars;
 
-static inline void free_pm_method_vars(void *);
-static inline void freep(void *);
-
-#define _cleanup_free_pm_vars_ __attribute__((cleanup(free_pm_method_vars)))
-#define _cleanup_free_ __attribute__((cleanup(freep)))
-
 #define PM_SUSPEND_LOGIND		"Suspend"
 #define PM_SUSPEND_HYBRID_LOGIND	"HybridSleep"
 #define PM_HIBERNATE_LOGIND		"Hibernate"
 
 #define FWTS_SUSPEND		"FWTS_SUSPEND"
 #define FWTS_RESUME		"FWTS_RESUME"
-#define FWTS_HIBERNATE	"FWTS_HIBERNATE"
-#define FWTS_RESUME	"FWTS_RESUME"
+#define FWTS_HIBERNATE		"FWTS_HIBERNATE"
+#define FWTS_RESUME		"FWTS_RESUME"
 
-static inline void free_pm_method_vars(void *vars)
+static inline void free_pm_method_vars(fwts_pm_method_vars *var)
 {
-	fwts_pm_method_vars *var = *(void**)vars;
-
+	if (!var)
+		return;
 #if FWTS_ENABLE_LOGIND
-	if (var) {
-		if (var->logind_proxy) {
-			g_object_unref(var->logind_proxy);
-			var->logind_proxy = NULL;
-		}
-		if (var->logind_connection) {
-			g_object_unref(var->logind_connection);
-			var->logind_connection = NULL;
-		}
-		if (var->gmainloop) {
-			g_main_loop_unref(var->gmainloop);
-			var->gmainloop = NULL;
-		}
-		if (var->action) {
-			free(var->action);
-			var->action = NULL;
-		}
-	}
+	if (var->logind_proxy)
+		g_object_unref(var->logind_proxy);
+	if (var->logind_connection)
+		g_object_unref(var->logind_connection);
+	if (var->gmainloop)
+		g_main_loop_unref(var->gmainloop);
+	if (var->action)
+		free(var->action);
 #endif
 	free(var);
-	var = NULL;
-}
-
-static inline void freep(void *p)
-{
-	free(*(void**) p);
 }
 
 int fwts_logind_init_proxy(fwts_pm_method_vars *fwts_settings);
-- 
2.6.4




More information about the fwts-devel mailing list