Re: [libvirt] [PATCH 3/8] virstoragefile: Resolve Coverity DEADCODE

On 09/13/14 15:27, John Ferlan wrote: 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. 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. So you may have this condition theoretically true, while the check above doesn't catch it. Also I've looked on the specs of the qcow2 header format and it seems that we don't do exactly the right thing here. To determine that the qcow2 image doesn't have a backing store we sholuld use the offset rather than size: 8 - 15: backing_file_offset Offset into the image file at which the backing file name is stored (NB: The string is not null terminated). 0 if the image doesn't have a backing file. 16 - 19: backing_file_size Length of the backing file name in bytes. Must not be longer than 1023 bytes. Undefined if the image doesn't have a backing file. Also we probably should make sure that the backing file size is less than 1024 bytes. While I agree that the check is dead code, as we call this function with buf_size with a reasonably low value, we probably should improve the backing file detection code to match the specs. Peter