OProfile

A Coverity scan of the OProfile code showed that op_get_cpu_type() did
not close the file in the case where the function opened the file, but
was unable to read from the file.

Signed-off-by: William Cohen <wcohen@...>
---
 libop/op_cpu_type.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/libop/op_cpu_type.c b/libop/op_cpu_type.c
index 9d11b21..14e26bf 100644
--- a/libop/op_cpu_type.c
+++ b/libop/op_cpu_type.c
@@ -145,7 +145,8 @@ op_cpu op_get_cpu_type(void)
 
 	if (!fgets(str, 99, fp)) {
 		fprintf(stderr, "Could not read cpu type.\n");
-		return CPU_NO_GOOD;
+		fclose(fp);
+		return cpu_type;
 	}
 
 	cpu_type = op_get_cpu_number(str);
-- 
1.7.1

The close(fd) was after the throw. A Coverity scan pointed out that the
close(fd) would be unreachable. Reorder the statements to make sure that
close(fd) occurs before the throw.

Signed-off-by: William Cohen <wcohen@...>
---
 libpp/op_header.cpp |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libpp/op_header.cpp b/libpp/op_header.cpp
index 041a1e8..754015a 100644
--- a/libpp/op_header.cpp
+++ b/libpp/op_header.cpp
@@ -142,10 +142,10 @@ opd_header const read_header(string const & sample_filename)
 	}
 
 	if (memcmp(header.magic, OPD_MAGIC, sizeof(header.magic))) {
+		close(fd);
 		throw op_fatal_error("Invalid sample file, "
 				     "bad magic number: " +
 				     sample_filename);
-		close(fd);
 	}
 
 	close(fd);
-- 
1.7.1

Analysis of the OProfile code by Coverity showed that in some cases the
memory malloced for bitmap would not be freed.  In the case that a
redundant key is found the memory should also be freed.

Signed-off-by: William Cohen <wcohen@...>
---
 libdb/db_debug.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/libdb/db_debug.c b/libdb/db_debug.c
index 0575570..8c01860 100644
--- a/libdb/db_debug.c
+++ b/libdb/db_debug.c
@@ -77,6 +77,7 @@ static int check_redundant_key(odb_data_t const * data, odb_key_t max)
 		if (bitmap[data->node_base[pos].key]) {
 			printf("redundant key found %lld\n",
 			       (unsigned long long)data->node_base[pos].key);
+			free(bitmap);
 			return 1;
 		}
 		bitmap[data->node_base[pos].key] = 1;
-- 
1.7.1

On 05/27/2011 4:32 PM, William Cohen wrote:
> OProfile packaged in Fedora development has been scanned by Coverity. These
> are some patches to address issues uncovered by the Coverity scan.
>
> William Cohen (3):
>    Do the close(fd) before doing the throw
>    Ensure that malloc'ed memory freed in check_redundant_key
>    Ensure that  op_get_cpu_type() closes the file
>
>   libdb/db_debug.c    |    1 +
>   libop/op_cpu_type.c |    3 ++-
>   libpp/op_header.cpp |    2 +-

Will,
All three changes look fine.  Go ahead and commit them.

-Maynard

>   3 files changed, 4 insertions(+), 2 deletions(-)
>
>
> ------------------------------------------------------------------------------
> vRanger cuts backup time in half-while increasing security.
> With the market-leading solution for virtual backup and recovery,
> you get blazing-fast, flexible, and affordable data protection.
> Download your free trial now.
> http://p.sf.net/sfu/quest-d2dcopy1
> _______________________________________________
> oprofile-list mailing list
> oprofile-list@...
> https://lists.sourceforge.net/lists/listinfo/oprofile-list

On 27.05.11 17:32:55, William Cohen wrote:
> The close(fd) was after the throw. A Coverity scan pointed out that the
> close(fd) would be unreachable. Reorder the statements to make sure that
> close(fd) occurs before the throw.
> 
> Signed-off-by: William Cohen <wcohen@...>
> ---
>  libpp/op_header.cpp |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/libpp/op_header.cpp b/libpp/op_header.cpp
> index 041a1e8..754015a 100644
> --- a/libpp/op_header.cpp
> +++ b/libpp/op_header.cpp
> @@ -142,10 +142,10 @@ opd_header const read_header(string const & sample_filename)
>  	}
>  
>  	if (memcmp(header.magic, OPD_MAGIC, sizeof(header.magic))) {
> +		close(fd);

Can't we put this before the memcmp()?

>  		throw op_fatal_error("Invalid sample file, "
>  				     "bad magic number: " +
>  				     sample_filename);
> -		close(fd);
>  	}
>  
>  	close(fd);

This would be obsolete then.

-Robert

> -- 
> 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> vRanger cuts backup time in half-while increasing security.
> With the market-leading solution for virtual backup and recovery, 
> you get blazing-fast, flexible, and affordable data protection.
> Download your free trial now. 
> http://p.sf.net/sfu/quest-d2dcopy1
> _______________________________________________
> oprofile-list mailing list
> oprofile-list@...
> https://lists.sourceforge.net/lists/listinfo/oprofile-list
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

On 27.05.11 17:32:56, William Cohen wrote:
> Analysis of the OProfile code by Coverity showed that in some cases the
> memory malloced for bitmap would not be freed.  In the case that a
> redundant key is found the memory should also be freed.
> 
> Signed-off-by: William Cohen <wcohen@...>
> ---
>  libdb/db_debug.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/libdb/db_debug.c b/libdb/db_debug.c
> index 0575570..8c01860 100644
> --- a/libdb/db_debug.c
> +++ b/libdb/db_debug.c
> @@ -77,6 +77,7 @@ static int check_redundant_key(odb_data_t const * data, odb_key_t max)
>  		if (bitmap[data->node_base[pos].key]) {
>  			printf("redundant key found %lld\n",
>  			       (unsigned long long)data->node_base[pos].key);
> +			free(bitmap);
>  			return 1;

Instead of free/return We could break here ...

>  		}
>  		bitmap[data->node_base[pos].key] = 1;

... and return from function with

	return pos < data->descr->current_size;

We would avoid multiple exit points from the function which are the
root cause of this bug.

-Robert

> -- 
> 1.7.1
> 
> 
> ------------------------------------------------------------------------------
> vRanger cuts backup time in half-while increasing security.
> With the market-leading solution for virtual backup and recovery, 
> you get blazing-fast, flexible, and affordable data protection.
> Download your free trial now. 
> http://p.sf.net/sfu/quest-d2dcopy1
> _______________________________________________
> oprofile-list mailing list
> oprofile-list@...
> https://lists.sourceforge.net/lists/listinfo/oprofile-list
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center