summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXavier Hernandez <xhernandez@datalab.es>2014-07-16 13:50:53 +0200
committerVijay Bellur <vbellur@redhat.com>2014-07-21 18:39:46 -0700
commit464ff8f7592e7d6a9239ca1fab1928d4a608f253 (patch)
treeede9747e4b9c79ad0da3598eb43ff922d132e3dc
parentf6ddb4675c096dc81420ed84fb2a6fefa9fea563 (diff)
ec: Fixed coveriry scan issues
CID list: 1226163 Logically dead code 1226166 Missing break in switch 1226167 Missing break in switch 1226168 Missing break in switch 1226169 Missing break in switch 1226170 Missing break in switch 1226171 Missing break in switch 1226172 Missing break in switch 1226173 Missing break in switch 1226174 Missing break in switch 1226175 Missing break in switch 1226176 Missing break in switch 1226177 Missing break in switch 1226178 Data race condition 1226179 Data race condition 1226180 Data race condition 1226181 Thread deadlock 1226182 Uninitialized pointer read 1226183 Uninitialized pointer read 1226184 Read from pointer after free Change-Id: I4d33aa42289371927175c43bb29e018df64fb943 BUG: 789278 Signed-off-by: Xavier Hernandez <xhernandez@datalab.es> Reviewed-on: http://review.gluster.org/8317 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rw-r--r--xlators/cluster/ec/src/ec-combine.c14
-rw-r--r--xlators/cluster/ec/src/ec-dir-read.c2
-rw-r--r--xlators/cluster/ec/src/ec-dir-write.c2
-rw-r--r--xlators/cluster/ec/src/ec-generic.c2
-rw-r--r--xlators/cluster/ec/src/ec-heal.c62
-rw-r--r--xlators/cluster/ec/src/ec-inode-read.c4
-rw-r--r--xlators/cluster/ec/src/ec-inode-write.c4
-rw-r--r--xlators/cluster/ec/src/ec-locks.c6
-rw-r--r--xlators/cluster/ec/src/ec-method.c9
9 files changed, 66 insertions, 39 deletions
diff --git a/xlators/cluster/ec/src/ec-combine.c b/xlators/cluster/ec/src/ec-combine.c
index 07d819a9a3d..3d088d9be4a 100644
--- a/xlators/cluster/ec/src/ec-combine.c
+++ b/xlators/cluster/ec/src/ec-combine.c
@@ -366,18 +366,26 @@ int32_t ec_dict_data_merge(ec_cbk_data_t * cbk, int32_t which, char * key)
return -1;
}
- if (dict_unserialize(data[0]->data, data[0]->len, &lockinfo) != 0)
+ lockinfo = dict_new();
+ if (lockinfo == NULL)
{
return -1;
}
+ if (dict_unserialize(data[0]->data, data[0]->len, &lockinfo) != 0)
+ {
+ goto out;
+ }
+
for (i = 1; i < num; i++)
{
- if (dict_unserialize(data[i]->data, data[i]->len, &tmp) != 0)
+ tmp = dict_new();
+ if (tmp == NULL)
{
goto out;
}
- if (dict_copy(tmp, lockinfo) == NULL)
+ if ((dict_unserialize(data[i]->data, data[i]->len, &tmp) != 0) ||
+ (dict_copy(tmp, lockinfo) == NULL))
{
dict_unref(tmp);
diff --git a/xlators/cluster/ec/src/ec-dir-read.c b/xlators/cluster/ec/src/ec-dir-read.c
index 1e7abc30d91..3a8455101e8 100644
--- a/xlators/cluster/ec/src/ec-dir-read.c
+++ b/xlators/cluster/ec/src/ec-dir-read.c
@@ -363,6 +363,8 @@ int32_t ec_manager_readdir(ec_fop_data_t * fop, int32_t state)
fop->mask &= 1ULL << idx;
}
+ /* Fall through */
+
case EC_STATE_DISPATCH:
ec_dispatch_one(fop);
diff --git a/xlators/cluster/ec/src/ec-dir-write.c b/xlators/cluster/ec/src/ec-dir-write.c
index da89e34ba25..dc1d94e37e8 100644
--- a/xlators/cluster/ec/src/ec-dir-write.c
+++ b/xlators/cluster/ec/src/ec-dir-write.c
@@ -178,6 +178,8 @@ int32_t ec_manager_create(ec_fop_data_t * fop, int32_t state)
fop->int32 &= ~O_ACCMODE;
fop->int32 |= O_RDWR;
+ /* Fall through */
+
case EC_STATE_LOCK:
ec_lock_entry(fop, &fop->loc[0]);
diff --git a/xlators/cluster/ec/src/ec-generic.c b/xlators/cluster/ec/src/ec-generic.c
index 49343388934..dabea16233a 100644
--- a/xlators/cluster/ec/src/ec-generic.c
+++ b/xlators/cluster/ec/src/ec-generic.c
@@ -908,6 +908,8 @@ int32_t ec_manager_lookup(ec_fop_data_t * fop, int32_t state)
return EC_STATE_REPORT;
}
+ /* Fall through */
+
case EC_STATE_DISPATCH:
ec_dispatch_all(fop);
diff --git a/xlators/cluster/ec/src/ec-heal.c b/xlators/cluster/ec/src/ec-heal.c
index 37264f598b9..22d4f89d4dd 100644
--- a/xlators/cluster/ec/src/ec-heal.c
+++ b/xlators/cluster/ec/src/ec-heal.c
@@ -90,6 +90,10 @@ void ec_heal_lookup_resume(ec_fop_data_t * fop)
}
}
+ /* Heal lookups are not executed concurrently with anything else. So, when
+ * a lookup finishes, it's safe to access heal->good and heal->bad without
+ * acquiring any lock.
+ */
heal->good = good;
heal->bad = bad;
@@ -455,43 +459,20 @@ int32_t ec_heal_init(ec_fop_data_t * fop)
return ENODATA;
}
- LOCK(&inode->lock);
-
- ctx = __ec_inode_get(inode, fop->xl);
- if (ctx == NULL)
- {
- error = EIO;
-
- goto out;
- }
-
- if (ctx->heal != NULL)
- {
- error = EEXIST;
-
- goto out;
- }
-
heal = GF_MALLOC(sizeof(ec_heal_t), ec_mt_ec_heal_t);
if (heal == NULL)
{
- error = ENOMEM;
-
- goto out;
+ return ENOMEM;
}
memset(heal, 0, sizeof(ec_heal_t));
- if (loc_copy(&heal->loc, &fop->loc[0]) != 0)
+ if (!ec_loc_from_loc(fop->xl, &heal->loc, &fop->loc[0]))
{
error = ENOMEM;
goto out;
}
- if (uuid_is_null(heal->loc.gfid))
- {
- uuid_copy(heal->loc.gfid, heal->loc.inode->gfid);
- }
LOCK_INIT(&heal->lock);
@@ -500,14 +481,31 @@ int32_t ec_heal_init(ec_fop_data_t * fop)
pool = fop->xl->ctx->iobuf_pool;
heal->size = iobpool_default_pagesize(pool) * ec->fragments;
+ LOCK(&inode->lock);
+
+ ctx = __ec_inode_get(inode, fop->xl);
+ if (ctx == NULL)
+ {
+ error = EIO;
+
+ goto unlock;
+ }
+
+ if (ctx->heal != NULL)
+ {
+ error = EEXIST;
+
+ goto unlock;
+ }
+
fop->data = heal;
ctx->heal = heal;
heal = NULL;
-out:
+unlock:
UNLOCK(&inode->lock);
-
+out:
GF_FREE(heal);
return error;
@@ -942,6 +940,10 @@ int32_t ec_heal_needs_data_rebuild(ec_heal_t * heal)
}
}
+ /* This function can only be called concurrently with entrylk, which do
+ * not modify heal structure, so it's safe to access heal->bad without
+ * acquiring any lock.
+ */
heal->bad = bad;
return (bad != 0);
@@ -1121,7 +1123,7 @@ void ec_heal_dispatch(ec_heal_t * heal)
GF_FREE(heal);
- ec_fop_set_error(heal->fop, error);
+ ec_fop_set_error(fop, error);
}
void ec_wind_heal(ec_t * ec, ec_fop_data_t * fop, int32_t idx)
@@ -1161,6 +1163,8 @@ int32_t ec_manager_heal(ec_fop_data_t * fop, int32_t state)
return EC_STATE_REPORT;
}
+ /* Fall through */
+
case EC_STATE_DISPATCH:
ec_heal_entrylk(fop->data, ENTRYLK_LOCK);
@@ -1221,6 +1225,8 @@ int32_t ec_manager_heal(ec_fop_data_t * fop, int32_t state)
case EC_STATE_HEAL_UNLOCK:
ec_heal_inodelk(heal, F_UNLCK, 0, 0, 0);
+ /* Fall through */
+
case -EC_STATE_HEAL_ENTRY_PREPARE:
case -EC_STATE_HEAL_PRE_INODELK_LOCK:
case -EC_STATE_HEAL_PRE_INODE_LOOKUP:
diff --git a/xlators/cluster/ec/src/ec-inode-read.c b/xlators/cluster/ec/src/ec-inode-read.c
index b1db9c9fbb7..46f904cf321 100644
--- a/xlators/cluster/ec/src/ec-inode-read.c
+++ b/xlators/cluster/ec/src/ec-inode-read.c
@@ -668,6 +668,8 @@ int32_t ec_manager_open(ec_fop_data_t * fop, int32_t state)
fop->int32 |= O_RDWR;
}
+ /* Fall through */
+
case EC_STATE_DISPATCH:
ec_dispatch_all(fop);
@@ -1236,6 +1238,8 @@ int32_t ec_manager_readv(ec_fop_data_t * fop, int32_t state)
fop->size = ec_adjust_size(fop->xl->private, fop->size + fop->head,
1);
+ /* Fall through */
+
case EC_STATE_LOCK:
ec_lock_fd(fop, fop->fd);
diff --git a/xlators/cluster/ec/src/ec-inode-write.c b/xlators/cluster/ec/src/ec-inode-write.c
index a3c4ae5d8f2..c0694531151 100644
--- a/xlators/cluster/ec/src/ec-inode-write.c
+++ b/xlators/cluster/ec/src/ec-inode-write.c
@@ -1363,6 +1363,8 @@ int32_t ec_manager_truncate(ec_fop_data_t * fop, int32_t state)
fop->user_size = fop->offset;
fop->offset = ec_adjust_size(fop->xl->private, fop->offset, 1);
+ /* Fall through */
+
case EC_STATE_LOCK:
ec_lock_inode(fop, &fop->loc[0]);
@@ -2019,6 +2021,8 @@ int32_t ec_manager_writev(ec_fop_data_t * fop, int32_t state)
return EC_STATE_REPORT;
}
+ /* Fall through */
+
case EC_STATE_LOCK:
ec_lock_fd(fop, fop->fd);
diff --git a/xlators/cluster/ec/src/ec-locks.c b/xlators/cluster/ec/src/ec-locks.c
index 96c1e03f8ec..2bcec34b6c3 100644
--- a/xlators/cluster/ec/src/ec-locks.c
+++ b/xlators/cluster/ec/src/ec-locks.c
@@ -215,6 +215,8 @@ int32_t ec_manager_entrylk(ec_fop_data_t * fop, int32_t state)
fop->entrylk_cmd = ENTRYLK_LOCK_NB;
}
+ /* Fall through */
+
case EC_STATE_DISPATCH:
ec_dispatch_all(fop);
@@ -646,6 +648,8 @@ int32_t ec_manager_inodelk(ec_fop_data_t * fop, int32_t state)
fop->int32 = F_SETLK;
}
+ /* Fall through */
+
case EC_STATE_DISPATCH:
ec_dispatch_all(fop);
@@ -1131,6 +1135,8 @@ int32_t ec_manager_lk(ec_fop_data_t * fop, int32_t state)
fop->int32 = F_SETLK;
}
+ /* Fall through */
+
case EC_STATE_DISPATCH:
ec_dispatch_all(fop);
diff --git a/xlators/cluster/ec/src/ec-method.c b/xlators/cluster/ec/src/ec-method.c
index deedf2a21f2..83b603bd14a 100644
--- a/xlators/cluster/ec/src/ec-method.c
+++ b/xlators/cluster/ec/src/ec-method.c
@@ -96,7 +96,7 @@ size_t ec_method_decode(size_t size, uint32_t columns, uint32_t * rows,
uint8_t ** in, uint8_t * out)
{
uint32_t i, j, k;
- uint32_t f, off, mask;
+ uint32_t f, off;
uint8_t inv[EC_METHOD_MAX_FRAGMENTS][EC_METHOD_MAX_FRAGMENTS + 1];
uint8_t mtx[EC_METHOD_MAX_FRAGMENTS][EC_METHOD_MAX_FRAGMENTS];
uint8_t * p[EC_METHOD_MAX_FRAGMENTS];
@@ -105,7 +105,6 @@ size_t ec_method_decode(size_t size, uint32_t columns, uint32_t * rows,
memset(inv, 0, sizeof(inv));
memset(mtx, 0, sizeof(mtx));
- mask = 0;
for (i = 0; i < columns; i++)
{
inv[i][i] = 1;
@@ -114,11 +113,6 @@ size_t ec_method_decode(size_t size, uint32_t columns, uint32_t * rows,
k = 0;
for (i = 0; i < columns; i++)
{
- while ((mask & 1) != 0)
- {
- k++;
- mask >>= 1;
- }
mtx[k][columns - 1] = 1;
for (j = columns - 1; j > 0; j--)
{
@@ -126,7 +120,6 @@ size_t ec_method_decode(size_t size, uint32_t columns, uint32_t * rows,
}
p[k] = in[i];
k++;
- mask >>= 1;
}
for (i = 0; i < columns; i++)