From b0b5392d1a403cb482e9632e0f773924e337dcf6 Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Thu, 21 Apr 2016 11:50:05 +0800 Subject: [PATCH] crush: fix mishandle result of get_bucket() method Get_bucket() is supposed to return a pointer to a specific bucket on success. However, it never returns a null pointer if error occurs. It returns -ENOENT instead. That's why most caller is misjudge the result code of get_bucket() method. Signed-off-by: xie xingguo --- src/crush/CrushWrapper.cc | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc index 3450b48fb8f..dc6280ee850 100644 --- a/src/crush/CrushWrapper.cc +++ b/src/crush/CrushWrapper.cc @@ -202,7 +202,7 @@ bool CrushWrapper::subtree_contains(int root, int item) const return false; // root is a leaf const crush_bucket *b = get_bucket(root); - if (!b) + if (IS_ERR(b)) return false; for (unsigned j=0; jsize; j++) { @@ -243,7 +243,12 @@ int CrushWrapper::remove_item(CephContext *cct, int item, bool unlink_only) if (item < 0 && !unlink_only) { crush_bucket *t = get_bucket(item); - if (t && t->size) { + if (IS_ERR(t)) { + ldout(cct, 1) << "remove_item bucket " << item << " does not exist" << dendl; + return -ENOENT; + } + + if (t->size) { ldout(cct, 1) << "remove_item bucket " << item << " has " << t->size << " items, not empty" << dendl; return -ENOTEMPTY; @@ -352,7 +357,13 @@ int CrushWrapper::remove_item_under(CephContext *cct, int item, int ancestor, bo if (item < 0 && !unlink_only) { crush_bucket *t = get_bucket(item); - if (t && t->size) { + if (IS_ERR(t)) { + ldout(cct, 1) << "remove_item_under bucket " << item + << " does not exist" << dendl; + return -ENOENT; + } + + if (t->size) { ldout(cct, 1) << "remove_item_under bucket " << item << " has " << t->size << " items, not empty" << dendl; return -ENOTEMPTY; @@ -555,7 +566,7 @@ int CrushWrapper::get_children(int id, list *children) } crush_bucket *b = get_bucket(id); - if (!b) { + if (IS_ERR(b)) { return -ENOENT; } -- 2.47.3