Re: [PATCH] Fix issues identified by Coverity run from Aug 11 2014

On 08/13/2014 09:46 AM, William Cohen wrote:
> On 08/12/2014 04:09 PM, Maynard Johnson wrote:
>> Fix issues identified by Coverity run from Aug 11 2014
>>
>> Running Coverity against the oprofile source tree resulted
>> in 8 problems being identified. One issue was dead code
>> that was originally created for special handling of the
>> IBM Cell BE processor.  Other issues were fairly mundane,
>> from uninitialized variables to ignoring return values of
>> functions, etc.
> 
> Hi Maynard,
> 
> I attempted to build an fedora 20 rpm with the patch and it looks like need to remove an unused variable in profile_container::add(...) other wise get:
> 
> g++ -DHAVE_CONFIG_H -I. -I..  -I ../libop -I ../libutil -I ../libdb -I ../libopt++ -I ../libutil++ -I ../libop++ -I ../libregex   -W -Wall -fno-common -ftemplate-depth-50 -Werror -g -c -o profile_container.o profile_container.cpp
> profile_container.cpp: In member function 'void profile_container::add(const profile_t&, const op_bfd&, const string&, size_t)':
> profile_container.cpp:78:13: error: variable 'header' set but not used [-Werror=unused-but-set-variable]
>   opd_header header = profile.get_header();
>              ^
> cc1plus: all warnings being treated as errors
> make[2]: *** [profile_container.o] Error 1
> 
> After fixing that I ran it through the version of coverity available at Red Hat.  There are still a few minor coverity errors being reported, but they look harmless.  I don't see how the CHECKED_RETURN is possible because the read return value is being checked.
> 
> 
> Error: CHECKED_RETURN (CWE-252): [#def1]
> oprofile-1.0.0git/libperf_events/operf_counter.cpp:66: check_return: "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
> 
> Error: DEADCODE (CWE-561): [#def2]
> oprofile-1.0.0git/libpp/image_errors.cpp:68: dead_error_condition: The switch value "error" cannot be "0".
> oprofile-1.0.0git/libpp/image_errors.cpp:68: dead_error_begin: Execution cannot reach this statement "case 0:".
> 
> Error: STREAM_FORMAT_STATE: [#def3]
> oprofile-1.0.0git/pp/opreport.cpp:251: format_changed: "setf" changes the format state of "std::cout" for category adjustfield.
> 
> I looked through the patch and it looked okay.
Thanks for the review, Will.  I committed the patch, including the removal of
	opd_header header = profile.get_header();
in libpp/profile_container.cpp.

-Maynard
> 
> -Will Cohen
> 
>>
>> This patch fixes all issues found by Coverity. One particular
>> change of note, however, is that the sample data format has
>> changed with the removal of Cell SPU-related fields.  This
>> change required the bumping of the OPD_VERSION number in the
>> sample file header, so that sample files created with earlier
>> oprofile builds will no longer be readable by oprofile. The
>> error reported by oprofile post-profiling tools would be:
>>    oprofpp: samples files version mismatch
>>
>> Signed-off-by: Maynard Johnson <[email protected]>
>> ---
>>  libop/op_config.h                 |    2 +-
>>  libop/op_cpu_type.c               |   12 +-
>>  libop/op_sample_file.h            |    4 -
>>  libopagent/opagent.c              |   11 ++-
>>  libperf_events/operf_counter.h    |    1 +
>>  libperf_events/operf_mangling.cpp |    8 +-
>>  libpp/Makefile.am                 |    4 +-
>>  libpp/callgraph_container.cpp     |    2 +-
>>  libpp/format_output.cpp           |   26 +----
>>  libpp/populate.cpp                |    7 --
>>  libpp/populate_for_spu.cpp        |  166 --------------------------------
>>  libpp/populate_for_spu.h          |   42 --------
>>  libpp/profile.cpp                 |   14 ---
>>  libpp/profile.h                   |   11 --
>>  libpp/profile_container.cpp       |    9 --
>>  libpp/symbol.h                    |    4 +-
>>  libutil++/Makefile.am             |    4 +-
>>  libutil++/bfd_spu_support.cpp     |  117 -----------------------
>>  libutil++/bfd_support.h           |    3 -
>>  libutil++/op_bfd.cpp              |    7 +-
>>  libutil++/op_bfd.h                |   18 ----
>>  libutil++/op_spu_bfd.cpp          |  188 -------------------------------------
>>  pe_profiling/operf.cpp            |   11 ++-
>>  23 files changed, 43 insertions(+), 628 deletions(-)
>>  delete mode 100644 libpp/populate_for_spu.cpp
>>  delete mode 100644 libpp/populate_for_spu.h
>>  delete mode 100644 libutil++/bfd_spu_support.cpp
>>  delete mode 100644 libutil++/op_spu_bfd.cpp
[snip]


------------------------------------------------------------------------------