All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhiqiang Liu <liuzhiqiang26@huawei.com>
To: Benjamin Marzinski <bmarzins@redhat.com>,
	Martin Wilck <mwilck@suse.com>,
	christophe.varoqui@opensvc.com, kabelac@redhat.com
Cc: linfeilong <linfeilong@huawei.com>,
	Yanxiaodan <yanxiaodan@huawei.com>,
	dm-devel@redhat.com, lixiaokeng <lixiaokeng@huawei.com>
Subject: [PATCH V2] vector: return false if realloc fails in vector_alloc_slot func
Date: Tue, 11 Aug 2020 11:23:01 +0800	[thread overview]
Message-ID: <7556f86a-0b45-8274-b9e7-0576813ca1fe@huawei.com> (raw)


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@huawei.com>
Signed-off-by: lixiaokeng <lixiaokeng@huawei.com>
---
 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

             reply	other threads:[~2020-08-11  3:23 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-11  3:23 Zhiqiang Liu [this message]
2020-08-11  9:46 ` [PATCH V2] vector: return false if realloc fails in vector_alloc_slot func Martin Wilck
2020-08-12  7:25   ` Zhiqiang Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7556f86a-0b45-8274-b9e7-0576813ca1fe@huawei.com \
    --to=liuzhiqiang26@huawei.com \
    --cc=bmarzins@redhat.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@redhat.com \
    --cc=kabelac@redhat.com \
    --cc=linfeilong@huawei.com \
    --cc=lixiaokeng@huawei.com \
    --cc=mwilck@suse.com \
    --cc=yanxiaodan@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.