On 5/20/19 6:17 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Fix the return types of non-predicate bitmap functions to return the
> usual negative error codes instead of the "moveon" boolean.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> include/bitmap.h | 8 +++--
> libfrog/bitmap.c | 86 ++++++++++++++++++++++++++----------------------------
> repair/rmap.c | 18 ++++++++---
> scrub/phase6.c | 18 ++++-------
> 4 files changed, 65 insertions(+), 65 deletions(-)
Sorry for the drama on signs.
Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>
> diff --git a/include/bitmap.h b/include/bitmap.h
> index e29a4335..99a2fb23 100644
> --- a/include/bitmap.h
> +++ b/include/bitmap.h
> @@ -11,11 +11,11 @@ struct bitmap {
> struct avl64tree_desc *bt_tree;
> };
>
> -bool bitmap_init(struct bitmap **bmap);
> +int bitmap_init(struct bitmap **bmap);
> void bitmap_free(struct bitmap **bmap);
> -bool bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> -bool bitmap_iterate(struct bitmap *bmap,
> - bool (*fn)(uint64_t, uint64_t, void *), void *arg);
> +int bitmap_set(struct bitmap *bmap, uint64_t start, uint64_t length);
> +int bitmap_iterate(struct bitmap *bmap, int (*fn)(uint64_t, uint64_t, void *),
> + void *arg);
> bool bitmap_test(struct bitmap *bmap, uint64_t start,
> uint64_t len);
> bool bitmap_empty(struct bitmap *bmap);
> diff --git a/libfrog/bitmap.c b/libfrog/bitmap.c
> index 450ebe0a..4dafc4c9 100644
> --- a/libfrog/bitmap.c
> +++ b/libfrog/bitmap.c
> @@ -66,7 +66,7 @@ static struct avl64ops bitmap_ops = {
> };
>
> /* Initialize a bitmap. */
> -bool
> +int
> bitmap_init(
> struct bitmap **bmapp)
> {
> @@ -74,18 +74,18 @@ bitmap_init(
>
> bmap = calloc(1, sizeof(struct bitmap));
> if (!bmap)
> - return false;
> + return -ENOMEM;
> bmap->bt_tree = malloc(sizeof(struct avl64tree_desc));
> if (!bmap->bt_tree) {
> free(bmap);
> - return false;
> + return -ENOMEM;
> }
>
> pthread_mutex_init(&bmap->bt_lock, NULL);
> avl64_init_tree(bmap->bt_tree, &bitmap_ops);
> *bmapp = bmap;
>
> - return true;
> + return 0;
> }
>
> /* Free a bitmap. */
> @@ -127,8 +127,31 @@ bitmap_node_init(
> return ext;
> }
>
> +/* Create a new bitmap node and insert it. */
> +static inline int
> +__bitmap_insert(
> + struct bitmap *bmap,
> + uint64_t start,
> + uint64_t length)
> +{
> + struct bitmap_node *ext;
> + struct avl64node *node;
> +
> + ext = bitmap_node_init(start, length);
> + if (!ext)
> + return -ENOMEM;
> +
> + node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> + if (node == NULL) {
> + free(ext);
> + return -EEXIST;
> + }
> +
> + return 0;
> +}
> +
> /* Set a region of bits (locked). */
> -static bool
> +static int
> __bitmap_set(
> struct bitmap *bmap,
> uint64_t start,
> @@ -142,28 +165,14 @@ __bitmap_set(
> struct bitmap_node *ext;
> uint64_t new_start;
> uint64_t new_length;
> - struct avl64node *node;
> - bool res = true;
>
> /* Find any existing nodes adjacent or within that range. */
> avl64_findranges(bmap->bt_tree, start - 1, start + length + 1,
> &firstn, &lastn);
>
> /* Nothing, just insert a new extent. */
> - if (firstn == NULL && lastn == NULL) {
> - ext = bitmap_node_init(start, length);
> - if (!ext)
> - return false;
> -
> - node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> - if (node == NULL) {
> - free(ext);
> - errno = EEXIST;
> - return false;
> - }
> -
> - return true;
> - }
> + if (firstn == NULL && lastn == NULL)
> + return __bitmap_insert(bmap, start, length);
>
> assert(firstn != NULL && lastn != NULL);
> new_start = start;
> @@ -175,7 +184,7 @@ __bitmap_set(
> /* Bail if the new extent is contained within an old one. */
> if (ext->btn_start <= start &&
> ext->btn_start + ext->btn_length >= start + length)
> - return res;
> + return 0;
>
> /* Check for overlapping and adjacent extents. */
> if (ext->btn_start + ext->btn_length >= start ||
> @@ -195,28 +204,17 @@ __bitmap_set(
> }
> }
>
> - ext = bitmap_node_init(new_start, new_length);
> - if (!ext)
> - return false;
> -
> - node = avl64_insert(bmap->bt_tree, &ext->btn_node);
> - if (node == NULL) {
> - free(ext);
> - errno = EEXIST;
> - return false;
> - }
> -
> - return res;
> + return __bitmap_insert(bmap, new_start, new_length);
> }
>
> /* Set a region of bits. */
> -bool
> +int
> bitmap_set(
> struct bitmap *bmap,
> uint64_t start,
> uint64_t length)
> {
> - bool res;
> + int res;
>
> pthread_mutex_lock(&bmap->bt_lock);
> res = __bitmap_set(bmap, start, length);
> @@ -308,26 +306,26 @@ bitmap_clear(
>
> #ifdef DEBUG
> /* Iterate the set regions of this bitmap. */
> -bool
> +int
> bitmap_iterate(
> struct bitmap *bmap,
> - bool (*fn)(uint64_t, uint64_t, void *),
> + int (*fn)(uint64_t, uint64_t, void *),
> void *arg)
> {
> struct avl64node *node;
> struct bitmap_node *ext;
> - bool moveon = true;
> + int error = 0;
>
> pthread_mutex_lock(&bmap->bt_lock);
> avl_for_each(bmap->bt_tree, node) {
> ext = container_of(node, struct bitmap_node, btn_node);
> - moveon = fn(ext->btn_start, ext->btn_length, arg);
> - if (!moveon)
> + error = fn(ext->btn_start, ext->btn_length, arg);
> + if (error)
> break;
> }
> pthread_mutex_unlock(&bmap->bt_lock);
>
> - return moveon;
> + return error;
> }
> #endif
>
> @@ -372,14 +370,14 @@ bitmap_empty(
> }
>
> #ifdef DEBUG
> -static bool
> +static int
> bitmap_dump_fn(
> uint64_t startblock,
> uint64_t blockcount,
> void *arg)
> {
> printf("%"PRIu64":%"PRIu64"\n", startblock, blockcount);
> - return true;
> + return 0;
> }
>
> /* Dump bitmap. */
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 19cceca3..47828a06 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -490,16 +490,22 @@ rmap_store_ag_btree_rec(
> error = init_slab_cursor(ag_rmap->ar_raw_rmaps, rmap_compare, &rm_cur);
> if (error)
> goto err;
> - if (!bitmap_init(&own_ag_bitmap)) {
> - error = -ENOMEM;
> + error = -bitmap_init(&own_ag_bitmap);
> + if (error)
> goto err_slab;
> - }
> while ((rm_rec = pop_slab_cursor(rm_cur)) != NULL) {
> if (rm_rec->rm_owner != XFS_RMAP_OWN_AG)
> continue;
> - if (!bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> - rm_rec->rm_blockcount)) {
> - error = EFSCORRUPTED;
> + error = -bitmap_set(own_ag_bitmap, rm_rec->rm_startblock,
> + rm_rec->rm_blockcount);
> + if (error) {
> + /*
> + * If this range is already set, then the incore rmap
> + * records for the AG free space btrees overlap and
> + * we're toast because that is not allowed.
> + */
> + if (error == EEXIST)
> + error = EFSCORRUPTED;
> goto err_slab;
> }
> }
> diff --git a/scrub/phase6.c b/scrub/phase6.c
> index 4b25f3bb..66e6451c 100644
> --- a/scrub/phase6.c
> +++ b/scrub/phase6.c
> @@ -341,7 +341,6 @@ xfs_check_rmap_ioerr(
> struct media_verify_state *vs = arg;
> struct bitmap *tree;
> dev_t dev;
> - bool moveon;
>
> dev = xfs_disk_to_dev(ctx, disk);
>
> @@ -356,8 +355,8 @@ xfs_check_rmap_ioerr(
> else
> tree = NULL;
> if (tree) {
> - moveon = bitmap_set(tree, start, length);
> - if (!moveon)
> + errno = -bitmap_set(tree, start, length);
> + if (errno)
> str_errno(ctx, ctx->mntpoint);
> }
>
> @@ -454,16 +453,16 @@ xfs_scan_blocks(
> struct scrub_ctx *ctx)
> {
> struct media_verify_state vs = { NULL };
> - bool moveon;
> + bool moveon = false;
>
> - moveon = bitmap_init(&vs.d_bad);
> - if (!moveon) {
> + errno = -bitmap_init(&vs.d_bad);
> + if (errno) {
> str_errno(ctx, ctx->mntpoint);
> goto out;
> }
>
> - moveon = bitmap_init(&vs.r_bad);
> - if (!moveon) {
> + errno = -bitmap_init(&vs.r_bad);
> + if (errno) {
> str_errno(ctx, ctx->mntpoint);
> goto out_dbad;
> }
> @@ -472,7 +471,6 @@ xfs_scan_blocks(
> ctx->geo.blocksize, xfs_check_rmap_ioerr,
> scrub_nproc(ctx));
> if (!vs.rvp_data) {
> - moveon = false;
> str_info(ctx, ctx->mntpoint,
> _("Could not create data device media verifier."));
> goto out_rbad;
> @@ -482,7 +480,6 @@ _("Could not create data device media verifier."));
> ctx->geo.blocksize, xfs_check_rmap_ioerr,
> scrub_nproc(ctx));
> if (!vs.rvp_log) {
> - moveon = false;
> str_info(ctx, ctx->mntpoint,
> _("Could not create log device media verifier."));
> goto out_datapool;
> @@ -493,7 +490,6 @@ _("Could not create data device media verifier."));
> ctx->geo.blocksize, xfs_check_rmap_ioerr,
> scrub_nproc(ctx));
> if (!vs.rvp_realtime) {
> - moveon = false;
> str_info(ctx, ctx->mntpoint,
> _("Could not create realtime device media verifier."));
> goto out_logpool;
>
