From 553ac4cc6da5c6cbc1e6c6f87dc3cdfd935c0880 Mon Sep 17 00:00:00 2001 From: "Kaleb S. KEITHLEY" Date: Fri, 18 Aug 2017 11:24:44 -0400 Subject: cluster/ec: coverity, fix for BAD_SHIFT This is how I would like to see this fixed. passes (eliminates the warning in) coverity. The use of uintptr_t as a bitmask is a problem IMO, especially on 32-bit clients. Change-Id: I86e15d12939c610c99f5f96c551bb870df20f4b4 Signed-off-by: Kaleb S. KEITHLEY Reviewed-on: https://review.gluster.org/18067 Smoke: Gluster Build System CentOS-regression: Gluster Build System Reviewed-by: Amar Tumballi Reviewed-by: Jeff Darcy --- xlators/cluster/ec/src/ec-common.c | 25 ++++++++++++++----------- xlators/cluster/ec/src/ec-common.h | 2 +- xlators/cluster/ec/src/ec-types.h | 2 +- 3 files changed, 16 insertions(+), 13 deletions(-) (limited to 'xlators') diff --git a/xlators/cluster/ec/src/ec-common.c b/xlators/cluster/ec/src/ec-common.c index 732d422517d..a03bbce367b 100644 --- a/xlators/cluster/ec/src/ec-common.c +++ b/xlators/cluster/ec/src/ec-common.c @@ -42,10 +42,11 @@ ec_select_first_by_read_policy (ec_t *ec, ec_fop_data_t *fop) int32_t ec_child_valid(ec_t * ec, ec_fop_data_t * fop, int32_t idx) { - return (idx < ec->nodes) && (((fop->remaining >> idx) & 1) == 1); + return (idx >= 0 && idx < ec->nodes) && + (((fop->remaining >> idx) & 1) == 1); } -int32_t ec_child_next(ec_t * ec, ec_fop_data_t * fop, int32_t idx) +int32_t ec_child_next(ec_t * ec, ec_fop_data_t * fop, uint32_t idx) { while (!ec_child_valid(ec, fop, idx)) { @@ -512,15 +513,17 @@ int32_t ec_child_select(ec_fop_data_t * fop) return 1; } -int32_t ec_dispatch_next(ec_fop_data_t * fop, int32_t idx) +void ec_dispatch_next(ec_fop_data_t * fop, uint32_t idx) { + int32_t i = -1; ec_t * ec = fop->xl->private; LOCK(&fop->lock); - idx = ec_child_next(ec, fop, idx); - if (idx >= 0) - { + i = ec_child_next(ec, fop, idx); + if (i >= 0 && i < 64) { + idx = i; + fop->remaining ^= 1ULL << idx; ec_trace("EXECUTE", fop, "idx=%d", idx); @@ -531,12 +534,10 @@ int32_t ec_dispatch_next(ec_fop_data_t * fop, int32_t idx) UNLOCK(&fop->lock); - if (idx >= 0) + if (i >= 0 && i < 64) { fop->wind(ec, fop, idx); } - - return idx; } void ec_dispatch_mask(ec_fop_data_t * fop, uintptr_t mask) @@ -646,7 +647,8 @@ void ec_dispatch_min(ec_fop_data_t * fop) { ec_t * ec = fop->xl->private; uintptr_t mask; - int32_t idx, count; + int32_t idx; + int count; ec_dispatch_start(fop); @@ -659,7 +661,8 @@ void ec_dispatch_min(ec_fop_data_t * fop) while (count-- > 0) { idx = ec_child_next(ec, fop, idx + 1); - mask |= 1ULL << idx; + if (idx >= 0 && idx < 64) + mask |= 1ULL << idx; } ec_dispatch_mask(fop, mask); diff --git a/xlators/cluster/ec/src/ec-common.h b/xlators/cluster/ec/src/ec-common.h index a03a590402a..0e8be35dbf9 100644 --- a/xlators/cluster/ec/src/ec-common.h +++ b/xlators/cluster/ec/src/ec-common.h @@ -75,7 +75,7 @@ typedef enum { #define EC_STATE_HEAL_DISPATCH 218 gf_boolean_t ec_dispatch_one_retry (ec_fop_data_t *fop, ec_cbk_data_t **cbk); -int32_t ec_dispatch_next(ec_fop_data_t * fop, int32_t idx); +void ec_dispatch_next(ec_fop_data_t * fop, uint32_t idx); void ec_complete(ec_fop_data_t *fop); diff --git a/xlators/cluster/ec/src/ec-types.h b/xlators/cluster/ec/src/ec-types.h index 3e93a1a32cc..907a3084952 100644 --- a/xlators/cluster/ec/src/ec-types.h +++ b/xlators/cluster/ec/src/ec-types.h @@ -328,7 +328,7 @@ struct _ec_cbk_data { struct list_head answer_list; /* item in the list of answers */ ec_fop_data_t *fop; ec_cbk_data_t *next; /* next answer in the same group */ - int32_t idx; + uint32_t idx; int32_t op_ret; int32_t op_errno; int32_t count; -- cgit