Max Reitz <mre...@redhat.com> writes: > On 2014-11-13 at 11:17, Markus Armbruster wrote: >> try_seek_hole() doesn't really seek to a hole, it tries to find out >> whether its argument is in a hole or not, and where the hole or >> non-hole ends. Rename to find_allocation() and add a proper function >> comment. >> >> Using arguments passed by reference like local variables is a bad >> habit. Only assign to them right before return. >> >> Avoid nesting of conditionals. >> >> When find_allocation() fails, don't make up a range that'll get mapped >> to nb_sectors, simply set *pnum = nb_sectors directly. >> >> Don't repeat BDRV_BLOCK_OFFSET_VALID | start. >> >> Drop a pointless assertion, add some meaningful ones. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> block/raw-posix.c | 62 >> +++++++++++++++++++++++++++++++++---------------------- >> 1 file changed, 37 insertions(+), 25 deletions(-) >> >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index 2a12a50..ea5b3b8 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -1478,28 +1478,43 @@ out: >> return result; >> } >> -static int try_seek_hole(BlockDriverState *bs, off_t start, off_t >> *data, >> - off_t *hole) >> +/* >> + * Find allocation range in @bs around offset @start. >> + * If @start is in a hole, store @start in @hole and the end of the >> + * hole in @data, and return 0. >> + * If @start is in a data, store @start to @data, and the end of the > > "is in a data" sounds funny enough I'd even like to keep it. Probably > should be "data extent" or something similar.
Okay. >> + * data to @hole, and return 0. > > Here, too. > > With or without that changed: > > Reviewed-by: Max Reitz <mre...@redhat.com> Thanks!
