diff options
| author | Sage Weil <sage@redhat.com> | 2017-08-09 23:25:12 +0200 |
|---|---|---|
| committer | Sage Weil <sage@redhat.com> | 2017-08-12 21:24:04 +0200 |
| commit | 5a982050c1daddee33bb2b5d63a5936723283765 (patch) | |
| tree | fa8e8aeb56158ae6b14104d89cedba6a2a49bbd2 /src/crush/builder.c | |
| parent | qa/workunits/mon/crush_ops.sh: test weight sets vs device classes (diff) | |
| download | ceph-5a982050c1daddee33bb2b5d63a5936723283765.tar.xz ceph-5a982050c1daddee33bb2b5d63a5936723283765.zip | |
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 <sage@redhat.com>
Diffstat (limited to 'src/crush/builder.c')
| -rw-r--r-- | src/crush/builder.c | 18 |
1 files changed, 13 insertions, 5 deletions
diff --git a/src/crush/builder.c b/src/crush/builder.c index 3b4ba25b277..f492a27d381 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) { |