In vector_alloc_slot func, if REALLOC fails, it means new slot
allocation fails. However, it just update v->allocated and then
return the old v->slot without new slot. So, the caller will take
the last old slot as the new allocated slot, and use it by calling
vector_set_slot func. Finally, the data of last slot is lost.
Here, we rewrite vector_alloc_slot as suggested by Martin Wilck:
- increment v->allocated only after successful allocation,
- avoid the "if (v->slot)" conditional by just calling realloc(),
- make sure all newly allocated vector elements are set to NULL,
- change return value to bool.
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx>
Signed-off-by: lixiaokeng <lixiaokeng@xxxxxxxxxx>
---
libmultipath/config.c | 2 +-
libmultipath/foreign.c | 2 +-
libmultipath/foreign/nvme.c | 6 +++---
libmultipath/vector.c | 22 +++++++++-------------
libmultipath/vector.h | 6 ++++--
5 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 49723add..4f5cefda 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -131,7 +131,7 @@ find_hwe (const struct _vector *hwtable,
vector_foreach_slot_backwards (hwtable, tmp, i) {
if (hwe_regmatch(tmp, vendor, product, revision))
continue;
- if (vector_alloc_slot(result) != NULL) {
+ if (vector_alloc_slot(result)) {
vector_set_slot(result, tmp);
n++;
}
diff --git a/libmultipath/foreign.c b/libmultipath/foreign.c
index 26f92672..c2335ed5 100644
--- a/libmultipath/foreign.c
+++ b/libmultipath/foreign.c
@@ -236,7 +236,7 @@ static int _init_foreign(const char *multipath_dir, const char *const enable)
goto dl_err;
}
- if (vector_alloc_slot(foreigns) == NULL) {
+ if (!vector_alloc_slot(foreigns)) {
goto dl_err;
}
diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
index da85e515..88b6aee2 100644
--- a/libmultipath/foreign/nvme.c
+++ b/libmultipath/foreign/nvme.c
@@ -731,12 +731,12 @@ static void _find_controllers(struct context *ctx, struct nvme_map *map)
test_ana_support(map, path->ctl);
path->pg.gen.ops = &nvme_pg_ops;
- if (vector_alloc_slot(&path->pg.pathvec) == NULL) {
+ if (!vector_alloc_slot(&path->pg.pathvec)) {
cleanup_nvme_path(path);
continue;
}
vector_set_slot(&path->pg.pathvec, path);
- if (vector_alloc_slot(&map->pgvec) == NULL) {
+ if (!vector_alloc_slot(&map->pgvec)) {
cleanup_nvme_path(path);
continue;
}
@@ -792,7 +792,7 @@ static int _add_map(struct context *ctx, struct udev_device *ud,
map->subsys = subsys;
map->gen.ops = &nvme_map_ops;
- if (vector_alloc_slot(ctx->mpvec) == NULL) {
+ if (!vector_alloc_slot(ctx->mpvec)) {
cleanup_nvme_map(map);
return FOREIGN_ERR;
}
diff --git a/libmultipath/vector.c b/libmultipath/vector.c
index 501cf4c5..39e2c20f 100644
--- a/libmultipath/vector.c
+++ b/libmultipath/vector.c
@@ -35,26 +35,22 @@ vector_alloc(void)
}
/* allocated one slot */
-void *
+bool
vector_alloc_slot(vector v)
{
void *new_slot = NULL;
if (!v)
- return NULL;
-
- v->allocated += VECTOR_DEFAULT_SIZE;
- if (v->slot)
- new_slot = REALLOC(v->slot, sizeof (void *) * v->allocated);
- else
- new_slot = (void *) MALLOC(sizeof (void *) * v->allocated);
+ return false;
+ new_slot = REALLOC(v->slot, sizeof (void *) * (v->allocated + VECTOR_DEFAULT_SIZE));
if (!new_slot)
- v->allocated -= VECTOR_DEFAULT_SIZE;
- else
- v->slot = new_slot;
+ return false;
- return v->slot;
+ v->slot = new_slot;
+ v->allocated += VECTOR_DEFAULT_SIZE;
+ v->slot[VECTOR_SIZE(v) - 1] = NULL;
+ return true;
}
int
@@ -203,7 +199,7 @@ int vector_find_or_add_slot(vector v, void *value)
if (n >= 0)
return n;
- if (vector_alloc_slot(v) == NULL)
+ if (!vector_alloc_slot(v))
return -1;
vector_set_slot(v, value);
return VECTOR_SIZE(v) - 1;
diff --git a/libmultipath/vector.h b/libmultipath/vector.h
index e16ec461..cb64b7d6 100644
--- a/libmultipath/vector.h
+++ b/libmultipath/vector.h
@@ -23,6 +23,8 @@
#ifndef _VECTOR_H
#define _VECTOR_H
+#include <stdbool.h>
+
/* vector definition */
struct _vector {
int allocated;
@@ -60,7 +62,7 @@ typedef struct _vector *vector;
__t = vector_alloc(); \
if (__t != NULL) { \
vector_foreach_slot(__v, __j, __i) { \
- if (vector_alloc_slot(__t) == NULL) { \
+ if (!vector_alloc_slot(__t)) { \
vector_free(__t); \
__t = NULL; \
break; \
@@ -73,7 +75,7 @@ typedef struct _vector *vector;
/* Prototypes */
extern vector vector_alloc(void);
-extern void *vector_alloc_slot(vector v);
+extern bool vector_alloc_slot(vector v);
vector vector_reset(vector v);
extern void vector_free(vector v);
#define vector_free_const(x) vector_free((vector)(long)(x))
--
2.24.0.windows.2
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
