On 09/15/14 15:23, John Ferlan wrote: > > > On 09/15/2014 04:42 AM, Peter Krempa wrote: >> On 09/13/14 15:27, John Ferlan wrote: >>> Coverity complains that the condition "size + 1 == 0" cannot happen. >>> It's already been determined that offset+size is not larger than >>> buf_size (and buf_size is smaller than UINT_MAX); and also that >> >> buff_size smaller than UINT_MAX isn't guaranteed in this function. >> Although it will probably never happen the size is declared as size_t >> and thus equal size to unsigned long long on 64 bit machines. >> >>> offset+size didn't overflow. >>> >>> So just remove the check >>> >>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >>> --- >>> src/util/virstoragefile.c | 2 -- >>> 1 file changed, 2 deletions(-) >>> >>> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c >>> index 5db9184..1a02b18 100644 >>> --- a/src/util/virstoragefile.c >>> +++ b/src/util/virstoragefile.c >>> @@ -393,8 +393,6 @@ qcowXGetBackingStore(char **res, >>> } >>> if (offset + size > buf_size || offset + size < offset) >> >> If size is UINT_MAX, then when you add it to offset, which is a small >> number, then you get something between UINT_MAX and ULLONG_MAX which is >> still in range of buf_size as it's a size_t. >> >>> return BACKING_STORE_INVALID; >>> - if (size + 1 == 0) >>> - return BACKING_STORE_INVALID; >> >> So you may have this condition theoretically true, while the check above >> doesn't catch it. >> > > My previous change was : > > + if (size == UINT_MAX) > return BACKING_STORE_INVALID; > > But it was pointed out by eblake: > > " > Is this dead code? After all, we just checked that offset+size is not > larger than buf_size (and buf_size is smaller than UINT_MAX); and also I wasn't able to locate the part that guarantees that buf_size is < UINT_MAX. > that offset+size didn't overflow. > > " > > Would it be better to use the original change instead? Just trying to > get past Coverity issue on this... > > > BTW: The remainder of this w/r/t matching spec seems to be outside the > scope of the Coverity DEADCODE, but if you have a patch I'm more than > willing to look at it ;-) This code is shared with qcow1GetBackingStore > and it's not like it's a recent change, so I'm hesitant to change it... I've sent a patch that should fix the parser according to the docs and also should fix the dead code here. > > > John Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list