diff mbox

lib: fwts_pipeio: remove gcc cleanup attribute, it confuses cppcheck

Message ID 1410447001-6937-1-git-send-email-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King Sept. 11, 2014, 2:50 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

cppcheck is getting a bit confused by the automatic resource
reaping with the gcc cleanup attribute.  Although this is a
useful gcc extention I think removing it back to some explicit
resource free'ing is a low-risk enough change to be an acceptable
way around the cppcheck issues.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_pipeio.c | 47 ++++++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

\ No newline at end of file

Comments

Keng-Yu Lin Sept. 12, 2014, 5:31 a.m. UTC | #1 
On Thu, Sep 11, 2014 at 10:50 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> cppcheck is getting a bit confused by the automatic resource
> reaping with the gcc cleanup attribute.  Although this is a
> useful gcc extention I think removing it back to some explicit
> resource free'ing is a low-risk enough change to be an acceptable
> way around the cppcheck issues.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_pipeio.c | 47 ++++++++++++++++++++---------------------------
>  1 file changed, 20 insertions(+), 27 deletions(-)
>
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index 162e5cf..06744bd 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -39,23 +39,6 @@
>
>  #include "fwts.h"
>
> -static inline void freep(void *);
> -static inline void fclosep(FILE **);
> -
> -#define _cleanup_free_ __attribute__((cleanup(freep)))
> -#define _cleanup_fclose_ __attribute__((cleanup(fclosep)))
> -
> -static inline void freep(void *p)
> -{
> -       free(*(void**) p);
> -}
> -
> -static inline void fclosep(FILE **file)
> -{
> -       if (*file)
> -               fclose(*file);
> -}
> -
>  /*
>   *  fwts_pipe_open()
>   *     execl a command, return pid in *childpid and
> @@ -248,9 +231,10 @@ int fwts_write_string_file(
>         const char *file_name,
>         const char *str)
>  {
> -       _cleanup_fclose_ FILE *file = NULL;
> -       errno = 0;
> +       FILE *file = NULL;
> +       int ret;
>
> +       errno = 0;
>         file = fopen(file_name, "we");
>         if (!file) {
>                 fwts_log_error(fw,
> @@ -262,7 +246,10 @@ int fwts_write_string_file(
>                 return FWTS_ERROR;
>         }
>
> -       return fwts_write_string_to_file(fw, file, str);
> +       ret = fwts_write_string_to_file(fw, file, str);
> +       fclose(file);
> +
> +       return ret;
>  }
>
>  /*
> @@ -276,7 +263,7 @@ int fwts_read_file_first_line(
>         const char *file_name,
>         char **line)
>  {
> -       _cleanup_fclose_ FILE *file = NULL;
> +       FILE *file = NULL;
>         char buffer[LINE_MAX], *temp;
>         errno = 0;
>
> @@ -292,6 +279,7 @@ int fwts_read_file_first_line(
>
>         if (!fgets(buffer, sizeof(buffer), file)) {
>                 if (ferror(file)) {
> +                       fclose(file);
>                         fwts_log_error(fw,
>                                 "Failed to read first line from %s, error: %d (%s).",
>                                 file_name,
> @@ -304,6 +292,7 @@ int fwts_read_file_first_line(
>
>         temp = strdup(buffer);
>         if (!temp) {
> +               fclose(file);
>                 fwts_log_error(fw,
>                         "Failed to read first line from %s: ran out of memory.",
>                         file_name);
> @@ -312,6 +301,7 @@ int fwts_read_file_first_line(
>
>         fwts_chop_newline(temp);
>         *line = temp;
> +       fclose(file);
>
>         return FWTS_OK;
>  }
> @@ -326,15 +316,18 @@ bool fwts_file_first_line_contains_string(
>         const char *file_name,
>         const char *str)
>  {
> -       _cleanup_free_ char *contents = NULL;
> +       char *contents = NULL;
>         int ret;
> +       bool contains;
>
>         ret = fwts_read_file_first_line(fw, file_name, &contents);
> -
>         if (ret != FWTS_OK) {
>                 fwts_log_error(fw, "Failed to get the contents of %s.", file_name);
> -               return false;
> -       }
> +               contains = false;
> +       } else
> +               contains = (strstr(contents, str) != NULL);
> +
> +       free(contents);
> +       return contains;
> +}
>
> -       return (strstr(contents, str) != NULL);
> -}
> \ No newline at end of file
> --
> 2.1.0
>

Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Alex Hung Sept. 16, 2014, 2:17 a.m. UTC | #2 
On 14-09-11 10:50 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> cppcheck is getting a bit confused by the automatic resource
> reaping with the gcc cleanup attribute.  Although this is a
> useful gcc extention I think removing it back to some explicit
> resource free'ing is a low-risk enough change to be an acceptable
> way around the cppcheck issues.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/lib/src/fwts_pipeio.c | 47 ++++++++++++++++++++---------------------------
>   1 file changed, 20 insertions(+), 27 deletions(-)
>
> diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
> index 162e5cf..06744bd 100644
> --- a/src/lib/src/fwts_pipeio.c
> +++ b/src/lib/src/fwts_pipeio.c
> @@ -39,23 +39,6 @@
>   
>   #include "fwts.h"
>   
> -static inline void freep(void *);
> -static inline void fclosep(FILE **);
> -
> -#define _cleanup_free_ __attribute__((cleanup(freep)))
> -#define _cleanup_fclose_ __attribute__((cleanup(fclosep)))
> -
> -static inline void freep(void *p)
> -{
> -	free(*(void**) p);
> -}
> -
> -static inline void fclosep(FILE **file)
> -{
> -	if (*file)
> -		fclose(*file);
> -}
> -
>   /*
>    *  fwts_pipe_open()
>    *	execl a command, return pid in *childpid and
> @@ -248,9 +231,10 @@ int fwts_write_string_file(
>   	const char *file_name,
>   	const char *str)
>   {
> -	_cleanup_fclose_ FILE *file = NULL;
> -	errno = 0;
> +	FILE *file = NULL;
> +	int ret;
>   
> +	errno = 0;
>   	file = fopen(file_name, "we");
>   	if (!file) {
>   		fwts_log_error(fw,
> @@ -262,7 +246,10 @@ int fwts_write_string_file(
>   		return FWTS_ERROR;
>   	}
>   
> -	return fwts_write_string_to_file(fw, file, str);
> +	ret = fwts_write_string_to_file(fw, file, str);
> +	fclose(file);
> +
> +	return ret;
>   }
>   
>   /*
> @@ -276,7 +263,7 @@ int fwts_read_file_first_line(
>   	const char *file_name,
>   	char **line)
>   {
> -	_cleanup_fclose_ FILE *file = NULL;
> +	FILE *file = NULL;
>   	char buffer[LINE_MAX], *temp;
>   	errno = 0;
>   
> @@ -292,6 +279,7 @@ int fwts_read_file_first_line(
>   
>   	if (!fgets(buffer, sizeof(buffer), file)) {
>   		if (ferror(file)) {
> +			fclose(file);
>   			fwts_log_error(fw,
>   				"Failed to read first line from %s, error: %d (%s).",
>   				file_name,
> @@ -304,6 +292,7 @@ int fwts_read_file_first_line(
>   
>   	temp = strdup(buffer);
>   	if (!temp) {
> +		fclose(file);
>   		fwts_log_error(fw,
>   			"Failed to read first line from %s: ran out of memory.",
>   			file_name);
> @@ -312,6 +301,7 @@ int fwts_read_file_first_line(
>   
>   	fwts_chop_newline(temp);
>   	*line = temp;
> +	fclose(file);
>   
>   	return FWTS_OK;
>   }
> @@ -326,15 +316,18 @@ bool fwts_file_first_line_contains_string(
>   	const char *file_name,
>   	const char *str)
>   {
> -	_cleanup_free_ char *contents = NULL;
> +	char *contents = NULL;
>   	int ret;
> +	bool contains;
>   
>   	ret = fwts_read_file_first_line(fw, file_name, &contents);
> -
>   	if (ret != FWTS_OK) {
>   		fwts_log_error(fw, "Failed to get the contents of %s.", file_name);
> -		return false;
> -	}
> +		contains = false;
> +	} else
> +		contains = (strstr(contents, str) != NULL);
> +
> +	free(contents);
> +	return contains;
> +}
>   
> -	return (strstr(contents, str) != NULL);
> -}
> \ No newline at end of file

Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox

Patch

diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c
index 162e5cf..06744bd 100644
--- a/src/lib/src/fwts_pipeio.c
+++ b/src/lib/src/fwts_pipeio.c
@@ -39,23 +39,6 @@ 
 
 #include "fwts.h"
 
-static inline void freep(void *);
-static inline void fclosep(FILE **);
-
-#define _cleanup_free_ __attribute__((cleanup(freep)))
-#define _cleanup_fclose_ __attribute__((cleanup(fclosep)))
-
-static inline void freep(void *p)
-{
-	free(*(void**) p);
-}
-
-static inline void fclosep(FILE **file)
-{
-	if (*file)
-		fclose(*file);
-}
-
 /*
  *  fwts_pipe_open()
  *	execl a command, return pid in *childpid and
@@ -248,9 +231,10 @@  int fwts_write_string_file(
 	const char *file_name,
 	const char *str)
 {
-	_cleanup_fclose_ FILE *file = NULL;
-	errno = 0;
+	FILE *file = NULL;
+	int ret;
 
+	errno = 0;
 	file = fopen(file_name, "we");
 	if (!file) {
 		fwts_log_error(fw,
@@ -262,7 +246,10 @@  int fwts_write_string_file(
 		return FWTS_ERROR;
 	}
 
-	return fwts_write_string_to_file(fw, file, str);
+	ret = fwts_write_string_to_file(fw, file, str);
+	fclose(file);
+
+	return ret;
 }
 
 /*
@@ -276,7 +263,7 @@  int fwts_read_file_first_line(
 	const char *file_name,
 	char **line)
 {
-	_cleanup_fclose_ FILE *file = NULL;
+	FILE *file = NULL;
 	char buffer[LINE_MAX], *temp;
 	errno = 0;
 
@@ -292,6 +279,7 @@  int fwts_read_file_first_line(
 
 	if (!fgets(buffer, sizeof(buffer), file)) {
 		if (ferror(file)) {
+			fclose(file);
 			fwts_log_error(fw,
 				"Failed to read first line from %s, error: %d (%s).",
 				file_name,
@@ -304,6 +292,7 @@  int fwts_read_file_first_line(
 
 	temp = strdup(buffer);
 	if (!temp) {
+		fclose(file);
 		fwts_log_error(fw,
 			"Failed to read first line from %s: ran out of memory.",
 			file_name);
@@ -312,6 +301,7 @@  int fwts_read_file_first_line(
 
 	fwts_chop_newline(temp);
 	*line = temp;
+	fclose(file);
 
 	return FWTS_OK;
 }
@@ -326,15 +316,18 @@  bool fwts_file_first_line_contains_string(
 	const char *file_name,
 	const char *str)
 {
-	_cleanup_free_ char *contents = NULL;
+	char *contents = NULL;
 	int ret;
+	bool contains;
 
 	ret = fwts_read_file_first_line(fw, file_name, &contents);
-
 	if (ret != FWTS_OK) {
 		fwts_log_error(fw, "Failed to get the contents of %s.", file_name);
-		return false;
-	}
+		contains = false;
+	} else
+		contains = (strstr(contents, str) != NULL);
+
+	free(contents);
+	return contains;
+}
 
-	return (strstr(contents, str) != NULL);
-}