Re: [libvirt] [PATCH 1/3] storage: Resolve Coverity FORWARD_NULL

On Wed, May 13, 2015 at 12:32:20 -0400, John Ferlan wrote: > Coverity points out it's possible for one of the virCommand{Output|Error}* > API's to have not allocated 'output' and/or 'error' in which case the > strstr comparison will cause a NULL deref > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/storage/storage_backend_disk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c > index 6394dac..5c2c49a 100644 > --- a/src/storage/storage_backend_disk.c > +++ b/src/storage/storage_backend_disk.c > @@ -412,7 +412,7 @@ virStorageBackendDiskFindLabel(const char* device) > > /* if parted succeeds we have a valid partition table */ > ret = virCommandRun(cmd, NULL); > - if (ret < 0) { > + if (ret < 0 && output && error) { > if (strstr(output, "unrecognised disk label") || > strstr(error, "unrecognised disk label")) { > ret = 1; This doesn't seem to be correct if either output or error is NULL and the other one is non-NULL. I'm too lazy to check if it's possible or not, but I think we should change this code in the following way and be safe: if (ret < 0) { if ((output && strstr(output, "unrecognised disk label")) || (error && strstr(error, "unrecognised disk label"))) { ret = 1; Jirka