On Wed, May 15, 2013 at 01:47:12PM +0200, Kevin Wolf wrote: > +static int map_is_allocated(int64_t sector_num, int64_t nb_sectors, int64_t > *pnum) > +{ > + int num, num_checked; > + int ret, firstret; > + > + num_checked = MIN(nb_sectors, INT_MAX); > + ret = bdrv_is_allocated(bs, sector_num, num_checked, &num); > + if (ret < 0) { > + return ret; > + } > + > + firstret = ret; > + *pnum = num; > + > + while (nb_sectors > 0 && ret == firstret) { > + sector_num += num; > + nb_sectors -= num; > + > + num_checked = MIN(nb_sectors, INT_MAX); > + ret = bdrv_is_allocated(bs, sector_num, num_checked, &num); > + if (ret == firstret) { > + *pnum += num; > + } else { > + break; > + }
The break makes && ret == firstret redundant above. I suggest just while (nb_sectors > 0) { ... } which is easier to read. Also, if you respin the patch please tweak the commit message. "Coalesce 'map' output" is more specific than "Fix 'map' output" - unless this really fixes a bug which you didn't mention in the commit description.