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
signature.asc
Description: OpenPGP digital signature