On 01/17/2013 01:03 PM, Peter Krempa wrote:
On 01/17/13 20:17, John Ferlan wrote:
--- src/parallels/parallels_driver.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index ea193af..1cf66ea 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -514,7 +514,10 @@ parallelsGetNetInfo(virDomainNetDefPtr net,
net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; if (virJSONValueObjectHasKey(value, "state")) { - tmp = virJSONValueObjectGetString(value, "state");
Hm calling this without checking should be safe as the virJSONValueObjectHasKey basically guarantees successful return of a value in virJSONValueObjectGetString.
+ if (!(tmp = virJSONValueObjectGetString(value, "state"))) { + parallelsParseError(); + goto error; + } if STREQ(tmp, "disconnected")
Another three possible solutions, both shorter, for silencing Coverity would be: 1. Add in a null check (slight speed penalty, though): if (virJSONValueObjectHasKey(value, "state")) { tmp = virJSONValueObjectGetString(value, "state"); if STREQ_NULLABLE(tmp, "disconnected") 2. use ga_assert() to tell Coverity something that we already know: if (virJSONValueObjectHasKey(value, "state")) { tmp = virJSONValueObjectGetString(value, "state"); ga_assert(tmp); if STREQ(tmp, "disconnected") 3. My favorite - use fewer function calls to begin with: if ((tmp = virJSONValueObjectGetString(value, "state")) && STREQ(tmp, "disconnected"))
I give ACK to this patch, but it shouldn't be a problem to go without it.
I think a v2 would be appropriate with a shorter solution. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org