On 4 June 2013 21:23, Alon Levy al...@redhat.com wrote:
Reported by Coverity:
Error: FORWARD_NULL (CWE-476):
qemu-1.5.0/libcacard/vreader.c:267: cond_false: Condition card == NULL,
taking false branch
qemu-1.5.0/libcacard/vreader.c:269: if_end: End of if statement
qemu-1.5.0/libcacard/vreader.c:272: cond_false: Condition apdu == NULL,
taking false branch
qemu-1.5.0/libcacard/vreader.c:275: else_branch: Reached else branch
qemu-1.5.0/libcacard/vreader.c:280: cond_false: Condition response, taking
false branch
qemu-1.5.0/libcacard/vreader.c:284: if_end: End of if statement
qemu-1.5.0/libcacard/vreader.c:280: var_compare_op: Comparing response to
null implies that response might be null.
qemu-1.5.0/libcacard/vreader.c:286: cond_true: Condition card_status ==
VCARD_DONE, taking true branch
qemu-1.5.0/libcacard/vreader.c:287: cond_true: Condition card_status ==
VCARD_DONE, taking true branch
qemu-1.5.0/libcacard/vreader.c:288: var_deref_op: Dereferencing null pointer
response.
Signed-off-by: Alon Levy al...@redhat.com
---
libcacard/vreader.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index 5793d73..60eb43b 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -260,7 +260,7 @@ vreader_xfr_bytes(VReader *reader,
{
VCardAPDU *apdu;
VCardResponse *response = NULL;
-VCardStatus card_status;
+VCardStatus card_status = VCARD_FAIL;
unsigned short status;
VCard *card = vreader_get_card(reader);
This doesn't make any sense to me as a fix for the quoted coverity
issue. It's complaining because the function both checks
that response isn't NULL (line 280) and uses it without
checking (line 288). If your change makes it stop complaining
it's only because you've managed to confuse it.
You either need to:
* assume that vcard_make_response() and vcard_process_apdu()
both guarantee to set response to non-NULL, and drop the
if (response) check
* don't assume it, and handle NULL response consistently
in this function (eg by changing the line 287 check to
if (card_status == VCARD_DONE response)
* take some middle line, eg response is guaranteed not to
be NULL if and only if status is VCARD_DONE and then
consistently check card_status in both places.
Also, this sequence:
assert(card_status == VCARD_DONE);
if (card_status == VCARD_DONE) {
is nonsensical. Either we should assert that the status
is always DONE, or we have code to handle the DONE and not
DONE cases; not both.
thanks
-- PMM
