9:48 a.m.
*** SOME BLURB HERE ***
Michal Privoznik (2):
virPerfReadEvent: Prefer saferead over read
qemuProcessVerifyGuestCPU: Avoid coverity false positive
src/qemu/qemu_process.c | 2 ++
src/util/virperf.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
--
2.7.3
9:48 a.m.
New subject: [libvirt] [PATCH 1/2] virPerfReadEvent: Prefer saferead over read
Do I really need to explain why?
Well, if read() is interrupted int the middle of reading, we will
never read the rest (even though it's highly unlikely as we are
reading just 8 bytes).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virperf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virperf.c b/src/util/virperf.c
index 9dc4e25..359a9c3 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -247,7 +247,7 @@ virPerfReadEvent(virPerfPtr perf,
if (event == NULL || !event->enabled)
return -1;
- if (read(event->fd, value, sizeof(uint64_t)) < 0) {
+ if (saferead(event->fd, value, sizeof(uint64_t)) < 0) {
virReportSystemError(errno, "%s",
_("Unable to read cache data"));
return -1;
--
2.7.3
10:01 a.m.
New subject: [libvirt] [PATCH 1/2] virPerfReadEvent: Prefer saferead over read
On Thu, Mar 31, 2016 at 04:48:23PM +0200, Michal Privoznik wrote:
Do I really need to explain why?
Do you really need to ask?
Well, if read() is interrupted int the middle of reading, we will
never read the rest (even though it's highly unlikely as we are
reading just 8 bytes).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virperf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK
Jan
9:48 a.m.
New subject: [libvirt] [PATCH 2/2] qemuProcessVerifyGuestCPU: Avoid coverity false positive
We use _LAST items in enums to mark the last position in given
enum. Now, if and enum is passed to switch(), compiler checks
that all the values from enum occur in 'case' enumeration.
Including _LAST. But coverity spots it's a dead code. And it
really is. So to resolve this, we tend to put a comment just
above 'case ..._LAST' notifying coverity that we know this is a
dead code but we want to have it that way.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_process.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 07b9df2..e58bf16 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3953,6 +3953,8 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
virDomainHypervTypeToString(i));
goto cleanup;
break;
+
+ /* coverity[dead_error_begin] */
case VIR_DOMAIN_HYPERV_LAST:
break;
}
--
2.7.3
10:06 a.m.
New subject: [libvirt] [PATCH 2/2] qemuProcessVerifyGuestCPU: Avoid coverity false positive
On Thu, Mar 31, 2016 at 04:48:24PM +0200, Michal Privoznik wrote:
We use _LAST items in enums to mark the last position in given
enum. Now, if and enum is passed to switch(), compiler checks
that all the values from enum occur in 'case' enumeration.
Including _LAST. But coverity spots it's a dead code. And it
really is. So to resolve this, we tend to put a comment just
above 'case ..._LAST' notifying coverity that we know this is a
dead code but we want to have it that way.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_process.c | 2 ++
1 file changed, 2 insertions(+)
ACK.
Would it be possible to somehow store a list of false positives
for Coverity to ignore in libvirt.git?
Like we do with tests/.valgrind.supp.
That way we would not need to clutter up the code.
Jan
10:28 a.m.
New subject: [libvirt] [PATCH 2/2] qemuProcessVerifyGuestCPU: Avoid coverity false positive
On 31.03.2016 17:06, Ján Tomko wrote:
On Thu, Mar 31, 2016 at 04:48:24PM +0200, Michal Privoznik wrote:
> We use _LAST items in enums to mark the last position in given
> enum. Now, if and enum is passed to switch(), compiler checks
> that all the values from enum occur in 'case' enumeration.
> Including _LAST. But coverity spots it's a dead code. And it
> really is. So to resolve this, we tend to put a comment just
> above 'case ..._LAST' notifying coverity that we know this is a
> dead code but we want to have it that way.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_process.c | 2 ++
> 1 file changed, 2 insertions(+)
>
ACK.
Would it be possible to somehow store a list of false positives
for Coverity to ignore in libvirt.git?
Like we do with tests/.valgrind.supp.
That way we would not need to clutter up the code.
Jan
That's what I've been wondering myself too. But frankly, I don't know
the answer. John?
Michal
10:50 a.m.
New subject: [libvirt] [PATCH 2/2] qemuProcessVerifyGuestCPU: Avoid coverity false positive
On 03/31/2016 11:28 AM, Michal Privoznik wrote:
On 31.03.2016 17:06, Ján Tomko wrote:
> On Thu, Mar 31, 2016 at 04:48:24PM +0200, Michal Privoznik wrote:
>> We use _LAST items in enums to mark the last position in given
>> enum. Now, if and enum is passed to switch(), compiler checks
>> that all the values from enum occur in 'case' enumeration.
>> Including _LAST. But coverity spots it's a dead code. And it
>> really is. So to resolve this, we tend to put a comment just
>> above 'case ..._LAST' notifying coverity that we know this is a
>> dead code but we want to have it that way.
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/qemu/qemu_process.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>
> ACK.
>
> Would it be possible to somehow store a list of false positives
> for Coverity to ignore in libvirt.git?
> Like we do with tests/.valgrind.supp.
>
> That way we would not need to clutter up the code.
>
> Jan
>
That's what I've been wondering myself too. But frankly, I don't know
the answer. John?
I'd have to dig on this a bit... It is possible to ignore whole classes
of errors using flags to the cov-analyze command; however, that's not
the same as valgrind where you can disable based on stack patterns using
"fun:" prefixes and elipses (...).
The problem with disabling dead_error_begin warnings could be missing
something that truly is a deadcode as opposed to this one where the
reason why the deadcode shows up is because the typed switch() statement
compiler magic requires the VIR_*_LAST: entry instead of going with the
non typed one where the "default:" case would be used.
John
3188
days inactive
3188
days old
6 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Michal Privoznik