From 5a982050c1daddee33bb2b5d63a5936723283765 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 9 Aug 2017 17:25:12 -0400 Subject: [PATCH] crush/builder: fix ENOENT when removing last bucket item We were decrementing size and then breaking out ENOENT condition check. Fix by decrementing size only after we break out of the loop and verify we found the item. Fix a follow-on bug by avoiding realloc when we have 0 items left. This case was never exercised before due to the ENOENT issue; now we return explicitly. It's really not necessary to realloc at all, probably, since these are very small arrays, but in any case leaving a single item allocation there in place of a 0-length allocation is fine. (And the 0-length allocation behvaior on realloc is undefined.. may either return a pointer or NULL.) Signed-off-by: Sage Weil --- src/crush/builder.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/crush/builder.c b/src/crush/builder.c index 3b4ba25b277b4..f492a27d3819b 100644 --- a/src/crush/builder.c +++ b/src/crush/builder.c @@ -1030,12 +1030,11 @@ int crush_remove_straw_bucket_item(struct crush_map *map, for (i = 0; i < bucket->h.size; i++) { if (bucket->h.items[i] == item) { - bucket->h.size--; if (bucket->item_weights[i] < bucket->h.weight) bucket->h.weight -= bucket->item_weights[i]; else bucket->h.weight = 0; - for (j = i; j < bucket->h.size; j++) { + for (j = i; j < bucket->h.size - 1; j++) { bucket->h.items[j] = bucket->h.items[j+1]; bucket->item_weights[j] = bucket->item_weights[j+1]; } @@ -1044,7 +1043,11 @@ int crush_remove_straw_bucket_item(struct crush_map *map, } if (i == bucket->h.size) return -ENOENT; - + bucket->h.size--; + if (bucket->h.size == 0) { + /* don't bother reallocating */ + return 0; + } void *_realloc = NULL; if ((_realloc = realloc(bucket->h.items, sizeof(__s32)*newsize)) == NULL) { @@ -1074,12 +1077,11 @@ int crush_remove_straw2_bucket_item(struct crush_map *map, for (i = 0; i < bucket->h.size; i++) { if (bucket->h.items[i] == item) { - bucket->h.size--; if (bucket->item_weights[i] < bucket->h.weight) bucket->h.weight -= bucket->item_weights[i]; else bucket->h.weight = 0; - for (j = i; j < bucket->h.size; j++) { + for (j = i; j < bucket->h.size - 1; j++) { bucket->h.items[j] = bucket->h.items[j+1]; bucket->item_weights[j] = bucket->item_weights[j+1]; } @@ -1089,6 +1091,12 @@ int crush_remove_straw2_bucket_item(struct crush_map *map, if (i == bucket->h.size) return -ENOENT; + bucket->h.size--; + if (!newsize) { + /* don't bother reallocating a 0-length array. */ + return 0; + } + void *_realloc = NULL; if ((_realloc = realloc(bucket->h.items, sizeof(__s32)*newsize)) == NULL) { -- 2.39.5