On Mon 08 Apr 2019 08:22:19 PM CEST, Andrey Shinkevich wrote: > * Return true if (a prefix of) the given range is allocated in any image > - * between BASE and TOP (inclusive). BASE can be NULL to check if the given > + * between BASE and TOP (TOP included). To check the BASE image, set the > + * 'include_base' to 'true'. The BASE can be NULL to check if the given > * offset is allocated in any image of the chain. Return false otherwise, > * or negative errno on failure.
I'm not a native speaker but that sounds a bit odd to me. Alternative: * Return true if (a prefix of) the given range is allocated in any image * between BASE and TOP (BASE is only included if include_base is set). * BASE can be NULL to check if the given offset is allocated in any * image of the chain. Return false otherwise, or negative errno on * failure. > - while (intermediate && intermediate != base) { > + while (include_base || intermediate != base) { > int64_t pnum_inter; > int64_t size_inter; > > @@ -2360,6 +2364,10 @@ int bdrv_is_allocated_above(BlockDriverState *top, > n = pnum_inter; > } > > + if (intermediate == base) { > + break; > + } > + > intermediate = backing_bs(intermediate); I find that the new condition + the break make things a bit less readable. I think it would be simpler with something like this: BlockDriverState *stop_at = include_base ? backing_bs(base) : base; while (intermediate != stop_at) { ... } (stop_at is a terrible name, but I can't think of anything better at the moment) Berto