On 11/18/2016 11:41 AM, Olaf Hering wrote:
> On Fri, Nov 18, Eric Blake wrote:
>
>> On 11/18/2016 04:24 AM, Olaf Hering wrote:
>>> + /* Overflowing byte limit? */
>>> + if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >>
>>> BDRV_SECTOR_BITS)) {
>> This is undefined. INT64_MAX + anything non-negative overflows int64,
>
> The expanded value used to be stored into a uint64_t before it was used
> here. A "cleanup" introduced this error. Thanks for spotting.
>
>> If you are trying to detect guests that make a request that would cover
>> more than INT64_MAX bytes, you can simplify. Besides, for as much
>> storage as there is out there, I seriously doubt ANYONE will ever have
>> 2^63 bytes addressable through a single device. Why not just write it as:
>>
>> if ((INT64_MAX >> BDRV_SECTOR_BITS) - sec_count < sec_start) {
>
> That would always be false I think. I will resubmit with this:
> if ((sec_start + sec_count) > (INT64_MAX >> BDRV_SECTOR_BITS)) {You're testing whether something overflows, but you don't want to cause overflow as part of the test. So use the commutative law to rewrite it to avoid sec_start+sec_count from overflowing, and you get: if (sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) but that's exactly the expression I wrote above. > > Regarding the cast for ->req, it has type blkif_request_t, but the > pointer needs to be assigned to type blkif_request_discard_t. Then why is the cast to (void*) instead of (blkif_request_discard_t*) ? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
