On Tue, Feb 05, 2013 at 02:36:08PM -0700, Eric Blake wrote:
> On 02/05/2013 11:54 AM, Stefan Hajnoczi wrote:
> > @@ -963,6 +965,17 @@ static int check_refcounts_l2(BlockDriverState *bs,
> > BdrvCheckResult *res,
> > l2_entry &= s->cluster_offset_mask;
> > inc_refcounts(bs, res, refcount_table, refcount_table_size,
> > l2_entry & ~511, nb_csectors * 512);
> > +
> > + if (flags & CHECK_FRAG_INFO) {
> > + res->bfi.allocated_clusters++;
> > + if (next_contiguous_offset &&
> > + (l2_entry & ~511) != next_contiguous_offset) {
> > + res->bfi.fragmented_clusters++;
> > + }
> > + /* Round down again, see nb_sectors above */
> > + next_contiguous_offset = (l2_entry & ~511) +
>
> That's a lot of repetition of (l2_entry & ~511), which feels somewhat
> magic; is it worth a dedicated temporary variable better named to
> explain why l2_entry is being rounded down to a 512-byte boundary?
You are right. The rounding down is refers to nb_csectors, not
l2_entry. I have added a comment to explain the reason.
Stefan
