[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 5/5] qcow2: Catch more unaligned write_zero i
Re: [Qemu-devel] [PATCH v3 5/5] qcow2: Catch more unaligned write_zero into zero cluster
Thu, 2 Jun 2016 06:33:48 -0600
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0
On 06/02/2016 04:14 AM, Kevin Wolf wrote:
>> If you prefer, I could have written '-tail % s->cluster_sectors', but as
>> % on a negative signed integer gives different results than what you get
>> for an unsigned number, I felt that & was nicer than % for making it
>> more obvious that I'm grabbing particular bits.
>> If you can think of any cleaner expression that represents the number of
>> sectors occurring after the tail until the next cluster boundary, I'm
>> game; the hardest part is that when tail is 0, we want the number passed
>> to is_zero_sectors() to also be 0, not s->cluster_sectors (so the naive
>> 's->cluster_sectors - tail' is wrong).
> The obvious one would be translating your English into C:
> tail ? s->cluster_sectors - tail : 0
Would gcc optimize this into a bit operation rather than a branch? If
not, that's a missed optimization bug that we should probably report
(that is, if gcc has enough information elsewhere that
s->cluster_sectors is a power of 2, since the bit operation optimization
depends on that fact).
> Another option which doesn't require an unsigned type would be
> (s->cluster_sectors - tail) % s->cluster_sectors.
That's the same thing as '-tail % s->cluster_sectors', since the
distributive rule applies to modulo arithmetic, and since 'a % a' is 0
for non-zero 'a'. So you can argue I was just saving typing :)
> I'm okay with merging the "more interesting" version, though I must
> admit that I had to read it twice.
That's certainly your call as maintainer :)
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Description: OpenPGP digital signature