On 09/04/2012 10:25 AM, William Cohen wrote:
> On 09/03/2012 10:23 AM, stephane eranian wrote:
>> Will,
>>
>> I have fixed most of the warnings reported by your tool.
>> Please pull and try again.
>>
>>
>> On Tue, Aug 28, 2012 at 9:17 PM, William Cohen <wco...@redhat.com> wrote:
>>> On 08/28/2012 12:03 PM, stephane eranian wrote:
>>>> On Tue, Aug 28, 2012 at 4:45 PM, William Cohen <wco...@redhat.com> wrote:
>>>
>>>>>
>>>>> Would it be possible to do the ldconf separately from the "make install"?
>>>>> The rpm packages are built as normal users with a staged install and the
>>>>> ldconf is done as a post install operation. The ldconf in the makefile
>>>>> needed to be disabled for the libpfm rpm to be built.
>>>>>
>>>> I just updated the libpfm.spec today to convert ldconfig to a nop
>>>> (LDCONFIG=true), so
>>>> it should not perturb the generation of the RPM. At least, it did not
>>>> for me when I tried
>>>> on Fedora 16.
>>>>
>>>
>>> Hi Stephane,
>>>
>>>
>>> Thanks for the libpfm.spec. The fedora rawhide libpfm-4.3.0 doesn't have
>>> any patches in it now:
>>>
>>> http://koji.fedoraproject.org/koji/taskinfo?taskID=4432221
>>>
>>> I ran libpfm through coverity to see whether there were any serious
>>> problems with it. Attached is the output of coverity. All errors are in
>>> the generated swig code and the examples. Probably can't do much about the
>>> swig related error because there are all in generated code. The examples
>>> could be cleaned up a little, but those are not packaged in the libpfm
>>> rpms, so those are not a great problem.
>>>
>>> In perf_setup_list_events() in perf_util.c why not make the for loop
>>> replace the ',' with '\0' the same structure as the earlier while loop
>>> counting the args? That would make the code a bit easier to read and
>>> eliminate one of the coverity errors.
>>> .
>>> -Will
>
> Hi Stephane,
>
> The results are improved. Most all the ones remaining are in the python swig
> wrappers. The one that isn't in the swig wrappers is:
>
> Error: NULL_RETURNS (CWE-476):
> /builddir/build/BUILD/libpfm-4.3.0/examples/showevtinfo.c:659:
> example_assign: Assigning: "p" = return value from "strchr(arg, 44)".
> /builddir/build/BUILD/libpfm-4.3.0/examples/showevtinfo.c:660:
> example_checked: "p" has its value checked in "p".
> /builddir/build/BUILD/libpfm-4.3.0/lib/pfmlib_common.c:476: example_assign:
> Assigning: "p" = return value from "strchr(pfm_cfg.forced_pmu, 44)".
> /builddir/build/BUILD/libpfm-4.3.0/lib/pfmlib_common.c:477: example_checked:
> "p" has its value checked in "p".
> /builddir/build/BUILD/libpfm-4.3.0/lib/pfmlib_common.c:970: example_assign:
> Assigning: "p" = return value from "strchr(s, 44)".
> /builddir/build/BUILD/libpfm-4.3.0/lib/pfmlib_common.c:971: example_checked:
> "p" has its value checked in "p".
> /builddir/build/BUILD/libpfm-4.3.0/perf_examples/perf_util.c:127:
> example_checked: "strchr(q, 44)" has its value checked in "p = strchr(q, 44)".
> /builddir/build/BUILD/libpfm-4.3.0/perf_examples/perf_util.c:141:
> returned_null: Function "strchr" returns null (checked 4 out of 5 times).
> /builddir/build/BUILD/libpfm-4.3.0/perf_examples/perf_util.c:141:
> var_assigned: Assigning: "p" = null return value from "strchr".
> /builddir/build/BUILD/libpfm-4.3.0/perf_examples/perf_util.c:142:
> dereference: Dereferencing a null pointer "p".
>
>
> Earlier in the code have:
>
> q = events;
> while((p = strchr(q, ','))) {
> num++;
> q = p + 1;
> }
> num++;
> num++; /* term
>
> Why not code the following for loop:
>
>
> for(i=0, q = events; i < num-2; i++, q = p + 1) {
> p = strchr(q, ',');
> *p = '\0';
> argv[i] = q;
> }
> argv[i++] = q;
> argv[i] = NULL;
>
> to mirror the earlier loop so the strchr value is checked? So make the for
> loop into something like:
>
> q = events;
> i = 0;
> while((p = strchr(q, ','))) {
> *p = '\0';
> i++;
> q = p + 1;
> }
> argv[i++] = q;
> argv[i] =NULL; /* terminator */
>
> -Will
>
The previous suggested code is not correct (missing the actual assignment to
argv[i]). Attached is a patch that attempts to simplify the code and remove
the coverity error. Before the patch have the following errors:
grep ^E libpfm-4.3.0-2.fc17cov2/run0/libpfm-4.3.0-2.fc17cov2.err |sort |uniq -c
1 Error: DEADCODE (CWE-561):
1 Error: NULL_RETURNS (CWE-476):
4 Error: RESOURCE_LEAK (CWE-404):
4 Error: UNUSED_VALUE (CWE-563):
After the patch have the following errors:
$ grep ^E libpfm-4.3.0-2.fc17cov2/run1/libpfm-4.3.0-2.fc17cov2.err |sort |uniq
-c
1 Error: DEADCODE (CWE-561):
4 Error: RESOURCE_LEAK (CWE-404):
4 Error: UNUSED_VALUE (CWE-563):
-Will
>From c0a4cfdb32ec8d4a2cdffcb0f6d2e4931ec06222 Mon Sep 17 00:00:00 2001
From: William Cohen <wco...@redhat.com>
Date: Tue, 4 Sep 2012 15:04:15 -0400
Subject: [PATCH] Simplify perf_util.c loop
There were two separate loops in perf_setup_list_events() that were
iterating pretty much the same way over a string. To make it a bit
easier to follow the second loop, a for-loop, was rewritten to be as
similar as possible to the earlier while loop. As a bonus, it keeps
Coverity from complaining about the possible NULL access using the
result of strchr.
Signed-off-by: William Cohen <wco...@redhat.com>
---
perf_examples/perf_util.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/perf_examples/perf_util.c b/perf_examples/perf_util.c
index f3cc919..a2c922c 100644
--- a/perf_examples/perf_util.c
+++ b/perf_examples/perf_util.c
@@ -137,13 +137,17 @@ perf_setup_list_events(const char *ev, perf_event_desc_t **fd, int *num_fds)
return -1;
}
- for(i=0, q = events; i < num-2; i++, q = p + 1) {
- p = strchr(q, ',');
+ i = 0;
+ q = events;
+ while((p = strchr(q, ','))) {
*p = '\0';
argv[i] = q;
+ i++;
+ q = p + 1;
}
argv[i++] = q;
argv[i] = NULL;
+
ret = perf_setup_argv_events(argv, fd, num_fds);
free(argv);
free(events); /* strdup in perf_setup_argv_events() */
--
1.7.11.4
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
