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.

+ * data to @hole, and return 0.

Here, too.

With or without that changed:

Reviewed-by: Max Reitz <mre...@redhat.com>

Reply via email to