Il 20/03/2014 08:32, Markus Armbruster ha scritto:
+static void __write(uint8_t *buf, int len)
Will the fact that you used 'int len' instead of 'size_t' bite us on 32-
vs. 64-bit? Same for __read.
Yeah, I copied this from address_space_rw. I'll change to ssize_t to
catch negative values.
Change the real address_space_rw(), or the model's __write()?
__read and __write for now (hard freeze etc. etc.).
+ if (is_write) __write(buf, len); else __read(buf, len);
+
+ return result;
+}
I'm curious: could you give me a rough idea on how modelling
address_space_rw() affects results?
Sure! The problematic code is this one:
if (!memory_access_is_direct(mr, is_write)) {
l = memory_access_size(mr, l, addr1);
/* XXX: could force current_cpu to NULL to avoid
potential bugs */
switch (l) {
case 8:
/* 64 bit write access */
val = ldq_p(buf);
error |= io_mem_write(mr, addr1, val, 8);
break;
Coverity doesn't understand that memory_access_size return a value that
is less than l, and thus thinks that address_space_rw can do an 8-byte
access. So it flags cases where we use it to read into an int or a
similarly small char[].
It's actually fairly common, it occurs ~20 times.
+static int get_keysym(const name2keysym_t *table,
+ const char *name)
Curious again: is this just insurance, or did you observe scanning
improvements?
It fixes exactly one error. All of the "tainted value" can be
considered false positives, but I wanted to have an example on how to
shut them up.
This claims g_malloc(0) returns a non-null pointer to a block of size 1.
Could we say it returns a non-null pointer to a block of size 0?
Not sure of the semantics of __coverity_alloc__(0). Leave it to further
future improvements?
if (success) {
void* tmp = __coverity_alloc__(size);
if (tmp) __coverity_mark_as_uninitialized_buffer__(tmp);
__coverity_mark_as_afm_allocated__(tmp, AFM_free);
return tmp;
} else {
__coverity_panic__ ();
}
Is the "if" needed at all? The "else" path is killed altogether by
__coverity_panic__().
Paolo
