[dm-devel] [PATCH 00/11] multipath-tools: improvements for pretty-printing code

[dm-devel] [PATCH 00/11] multipath-tools: improvements for pretty-printing code

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Hi Christophe, hi Ben,

this series attempts to improve the robustness of the code by making the lookup
tables used for pretty printing and wildcard hanling static const arrays.
This requires getting rid of the variable "width" field in these arrays.
I could have simply split of "width" into a separate global array variable,
but as the field width are designed to change depending on the data being
printed, it makes more sense to use local variables for it and pass them to
the actual table-formatting code.

A couple of minor cleanups are done along the way.

As always, feedback welcome
Martin

Martin Wilck (11):
  libmultipath: make multipath_data etc. static
  libmultipath: move path_data etc. to print.c
  libmultipath: make pd_lookup() etc. return an index
  libmultipath: use ARRAY_SIZE for iterating over wildcard arrays
  libmultipath: drop padding code in _snprint_pathgroup()
  libmultipath: snprint_foreign_topology(): split out lockless variant
  multipathd: drop unnecessary path layout calls
  libmultipath: add a cleanup function for unsigned char *
  libmultipath: make sprint_path_marginal() static
  libmultipath: introduce width argument for pretty-printing functions
  libmultipath: change wildcard handler tables to static const

 libmultipath/foreign.c            |  72 ++++--
 libmultipath/foreign.h            |  25 ++-
 libmultipath/libmultipath.version |   5 +-
 libmultipath/print.c              | 360 +++++++++++++++++-------------
 libmultipath/print.h              |  60 ++---
 libmultipath/util.c               |   5 +
 libmultipath/util.h               |   1 +
 multipath/main.c                  |   7 +-
 multipathd/cli_handlers.c         |  79 ++++---
 9 files changed, 360 insertions(+), 254 deletions(-)

-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 01/11] libmultipath: make multipath_data etc. static

[mwilck]

From: Martin Wilck <mwilck@suse.com>

These variables are only used in print.c

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index e61349f..938eaad 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -771,7 +771,7 @@ snprint_path_vpd_data(struct strbuf *buff, const struct path * pp)
 	return append_strbuf_str(buff, "[undef]");
 }
 
-struct multipath_data mpd[] = {
+static struct multipath_data mpd[] = {
 	{'n', "name",          0, snprint_name},
 	{'w', "uuid",          0, snprint_multipath_uuid},
 	{'d', "sysfs",         0, snprint_sysfs},
@@ -799,7 +799,7 @@ struct multipath_data mpd[] = {
 	{0, NULL, 0 , NULL}
 };
 
-struct path_data pd[] = {
+static struct path_data pd[] = {
 	{'w', "uuid",          0, snprint_path_uuid},
 	{'i', "hcil",          0, snprint_hcil},
 	{'d', "dev",           0, snprint_dev},
@@ -828,7 +828,7 @@ struct path_data pd[] = {
 	{0, NULL, 0 , NULL}
 };
 
-struct pathgroup_data pgd[] = {
+static struct pathgroup_data pgd[] = {
 	{'s', "selector",      0, snprint_pg_selector},
 	{'p', "pri",           0, snprint_pg_pri},
 	{'t', "dm_st",         0, snprint_pg_state},
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 02/11] libmultipath: move path_data etc. to print.c

[mwilck]

From: Martin Wilck <mwilck@suse.com>

These structs are only used in print.c

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c | 21 +++++++++++++++++++++
 libmultipath/print.h | 21 ---------------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 938eaad..4992cc8 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -100,6 +100,27 @@
 
 #define PROGRESS_LEN  10
 
+struct path_data {
+	char wildcard;
+	char * header;
+	unsigned int width;
+	int (*snprint)(struct strbuf *, const struct path * pp);
+};
+
+struct multipath_data {
+	char wildcard;
+	char * header;
+	unsigned int width;
+	int (*snprint)(struct strbuf *, const struct multipath * mpp);
+};
+
+struct pathgroup_data {
+	char wildcard;
+	char * header;
+	unsigned int width;
+	int (*snprint)(struct strbuf *, const struct pathgroup * pgp);
+};
+
 #define MAX(x,y) (((x) > (y)) ? (x) : (y))
 #define MIN(x,y) (((x) > (y)) ? (y) : (x))
 /*
diff --git a/libmultipath/print.h b/libmultipath/print.h
index b149275..4322f6e 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -9,27 +9,6 @@
 
 struct strbuf;
 
-struct path_data {
-	char wildcard;
-	char * header;
-	unsigned int width;
-	int (*snprint)(struct strbuf *, const struct path * pp);
-};
-
-struct multipath_data {
-	char wildcard;
-	char * header;
-	unsigned int width;
-	int (*snprint)(struct strbuf *, const struct multipath * mpp);
-};
-
-struct pathgroup_data {
-	char wildcard;
-	char * header;
-	unsigned int width;
-	int (*snprint)(struct strbuf *, const struct pathgroup * pgp);
-};
-
 enum layout_reset {
 	LAYOUT_RESET_NOT,
 	LAYOUT_RESET_ZERO,
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 03/11] libmultipath: make pd_lookup() etc. return an index

[mwilck]

From: Martin Wilck <mwilck@suse.com>

This will allow us to remove the mutable width field from path_data.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c | 64 ++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 4992cc8..37222bf 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -976,73 +976,70 @@ _get_multipath_layout (const struct _vector *gmvec,
 	}
 }
 
-static struct multipath_data *
-mpd_lookup(char wildcard)
+static int mpd_lookup(char wildcard)
 {
 	int i;
 
 	for (i = 0; mpd[i].header; i++)
 		if (mpd[i].wildcard == wildcard)
-			return &mpd[i];
+			return i;
 
-	return NULL;
+	return -1;
 }
 
 int snprint_multipath_attr(const struct gen_multipath* gm,
 			   struct strbuf *buf, char wildcard)
 {
 	const struct multipath *mpp = gen_multipath_to_dm(gm);
-	struct multipath_data *mpd = mpd_lookup(wildcard);
+	int i = mpd_lookup(wildcard);
 
-	if (mpd == NULL)
+	if (i == -1)
 		return 0;
-	return mpd->snprint(buf, mpp);
+	return mpd[i].snprint(buf, mpp);
 }
 
-static struct path_data *
-pd_lookup(char wildcard)
+static int pd_lookup(char wildcard)
 {
 	int i;
 
 	for (i = 0; pd[i].header; i++)
 		if (pd[i].wildcard == wildcard)
-			return &pd[i];
+			return i;
 
-	return NULL;
+	return -1;
 }
 
 int snprint_path_attr(const struct gen_path* gp,
 		      struct strbuf *buf, char wildcard)
 {
 	const struct path *pp = gen_path_to_dm(gp);
-	struct path_data *pd = pd_lookup(wildcard);
+	int i = pd_lookup(wildcard);
 
-	if (pd == NULL)
+	if (i == -1)
 		return 0;
-	return pd->snprint(buf, pp);
+	return pd[i].snprint(buf, pp);
 }
 
-static struct pathgroup_data *
-pgd_lookup(char wildcard)
+static int pgd_lookup(char wildcard)
 {
 	int i;
 
 	for (i = 0; pgd[i].header; i++)
 		if (pgd[i].wildcard == wildcard)
-			return &pgd[i];
+			return i;
 
-	return NULL;
+	return -1;
 }
 
 int snprint_pathgroup_attr(const struct gen_pathgroup* gpg,
 			   struct strbuf *buf, char wildcard)
 {
 	const struct pathgroup *pg = gen_pathgroup_to_dm(gpg);
-	struct pathgroup_data *pdg = pgd_lookup(wildcard);
+	int i = pgd_lookup(wildcard);
 
-	if (pdg == NULL)
+	if (i == -1)
 		return 0;
-	return pdg->snprint(buf, pg);
+	return pgd[i].snprint(buf, pg);
 }
 
 int snprint_multipath_header(struct strbuf *line, const char *format)
@@ -1053,12 +1050,15 @@ int snprint_multipath_header(struct strbuf *line, const char *format)
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
+		int iwc;
+
 		if ((rc = __append_strbuf_str(line, format, f - format)) < 0)
 			return rc;
 
 		format = f + 1;
-		if (!(data = mpd_lookup(*format)))
+		if ((iwc = mpd_lookup(*format)) == -1)
 			continue; /* unknown wildcard */
+		data = &mpd[iwc];
 
 		if ((rc = append_strbuf_str(line, data->header)) < 0)
 			return rc;
@@ -1081,12 +1081,15 @@ int _snprint_multipath(const struct gen_multipath *gmp,
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
+		int iwc;
+
 		if ((rc = __append_strbuf_str(line, format, f - format)) < 0)
 			return rc;
 
 		format = f + 1;
-		if (!(data = mpd_lookup(*format)))
+		if ((iwc = mpd_lookup(*format)) == -1)
 			continue; /* unknown wildcard */
+		data = &mpd[iwc];
 
 		if ((rc = gmp->ops->snprint(gmp, line, *format)) < 0)
 			return rc;
@@ -1108,12 +1111,15 @@ int snprint_path_header(struct strbuf *line, const char *format)
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
+		int iwc;
+
 		if ((rc = __append_strbuf_str(line, format, f - format)) < 0)
 			return rc;
 
 		format = f + 1;
-		if (!(data = pd_lookup(*format)))
+		if ((iwc = pd_lookup(*format)) == -1)
 			continue; /* unknown wildcard */
+		data = &pd[iwc];
 
 		if ((rc = append_strbuf_str(line, data->header)) < 0)
 			return rc;
@@ -1136,12 +1142,15 @@ int _snprint_path(const struct gen_path *gp, struct strbuf *line,
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
+		int iwc;
+
 		if ((rc = __append_strbuf_str(line, format, f - format)) < 0)
 			return rc;
 
 		format = f + 1;
-		if (!(data = pd_lookup(*format)))
+		if ((iwc = pd_lookup(*format)) == -1)
 			continue; /* unknown wildcard */
+		data = &pd[iwc];
 
 		if ((rc = gp->ops->snprint(gp, line, *format)) < 0)
 			return rc;
@@ -1164,12 +1173,15 @@ int _snprint_pathgroup(const struct gen_pathgroup *ggp, struct strbuf *line,
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
+		int iwc;
+
 		if ((rc = __append_strbuf_str(line, format, f - format)) < 0)
 			return rc;
 
 		format = f + 1;
-		if (!(data = pgd_lookup(*format)))
+		if ((iwc = pgd_lookup(*format)) == -1)
 			continue; /* unknown wildcard */
+		data = &pgd[iwc];
 
 		if ((rc = ggp->ops->snprint(ggp, line, *format)) < 0)
 			return rc;
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 04/11] libmultipath: use ARRAY_SIZE for iterating over wildcard arrays

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Avoid the extra empty element at the end of the array, and use the
ARRAY_SIZE paradigm that we've adopted elsewhere in multipath-tools.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 37222bf..fcbaa5a 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -817,7 +817,6 @@ static struct multipath_data mpd[] = {
 	{'e', "rev",           0, snprint_multipath_rev},
 	{'G', "foreign",       0, snprint_multipath_foreign},
 	{'g', "vpd page data", 0, snprint_multipath_vpd_data},
-	{0, NULL, 0 , NULL}
 };
 
 static struct path_data pd[] = {
@@ -846,7 +845,6 @@ static struct path_data pd[] = {
 	{'0', "failures",      0, snprint_path_failures},
 	{'P', "protocol",      0, snprint_path_protocol},
 	{'I', "init_st",       0, snprint_initialized},
-	{0, NULL, 0 , NULL}
 };
 
 static struct pathgroup_data pgd[] = {
@@ -854,31 +852,31 @@ static struct pathgroup_data pgd[] = {
 	{'p', "pri",           0, snprint_pg_pri},
 	{'t', "dm_st",         0, snprint_pg_state},
 	{'M', "marginal_st",   0, snprint_pg_marginal},
-	{0, NULL, 0 , NULL}
 };
 
 int snprint_wildcards(struct strbuf *buff)
 {
 	int initial_len = get_strbuf_len(buff);
-	int i, rc;
+	unsigned int i;
+	int rc;
 
 	if ((rc = append_strbuf_str(buff, "multipath format wildcards:\n")) < 0)
 		return rc;
-	for (i = 0; mpd[i].header; i++)
+	for (i = 0; i < ARRAY_SIZE(mpd); i++)
 		if ((rc = print_strbuf(buff, "%%%c  %s\n",
 				       mpd[i].wildcard, mpd[i].header)) < 0)
 			return rc;
 
 	if ((rc = append_strbuf_str(buff, "\npath format wildcards:\n")) < 0)
 		return rc;
-	for (i = 0; pd[i].header; i++)
+	for (i = 0; i < ARRAY_SIZE(pd); i++)
 		if ((rc = print_strbuf(buff, "%%%c  %s\n",
 				       pd[i].wildcard, pd[i].header)) < 0)
 			return rc;
 
 	if ((rc = append_strbuf_str(buff, "\npathgroup format wildcards:\n")) < 0)
 		return rc;
-	for (i = 0; pgd[i].header; i++)
+	for (i = 0; i < ARRAY_SIZE(pgd); i++)
 		if ((rc = print_strbuf(buff, "%%%c  %s\n",
 				       pgd[i].wildcard, pgd[i].header)) < 0)
 			return rc;
@@ -915,10 +913,10 @@ reset_width(unsigned int *width, enum layout_reset reset, const char *header)
 void
 _get_path_layout (const struct _vector *gpvec, enum layout_reset reset)
 {
-	int i, j;
+	unsigned int i, j;
 	const struct gen_path *gp;
 
-	for (j = 0; pd[j].header; j++) {
+        for (j = 0; j < ARRAY_SIZE(pd); j++) {
 		STRBUF_ON_STACK(buff);
 
 		reset_width(&pd[j].width, reset, pd[j].header);
@@ -937,9 +935,9 @@ _get_path_layout (const struct _vector *gpvec, enum layout_reset reset)
 static void
 reset_multipath_layout (void)
 {
-	int i;
+	unsigned int i;
 
-	for (i = 0; mpd[i].header; i++)
+	for (i = 0; i < ARRAY_SIZE(mpd); i++)
 		mpd[i].width = 0;
 }
 
@@ -956,10 +954,10 @@ void
 _get_multipath_layout (const struct _vector *gmvec,
 			    enum layout_reset reset)
 {
-	int i, j;
+	unsigned int i, j;
 	const struct gen_multipath * gm;
 
-	for (j = 0; mpd[j].header; j++) {
+	for (j = 0; j < ARRAY_SIZE(mpd); j++) {
 		STRBUF_ON_STACK(buff);
 
 		reset_width(&mpd[j].width, reset, mpd[j].header);
@@ -978,9 +976,9 @@ _get_multipath_layout (const struct _vector *gmvec,
 
 static int mpd_lookup(char wildcard)
 {
-	int i;
+	unsigned int i;
 
-	for (i = 0; mpd[i].header; i++)
+	for (i = 0; i < ARRAY_SIZE(mpd); i++)
 		if (mpd[i].wildcard == wildcard)
 			return i;
 
@@ -1000,9 +998,9 @@ int snprint_multipath_attr(const struct gen_multipath* gm,
 
 static int pd_lookup(char wildcard)
 {
-	int i;
+	unsigned int i;
 
-	for (i = 0; pd[i].header; i++)
+	for (i = 0; i < ARRAY_SIZE(pd); i++)
 		if (pd[i].wildcard == wildcard)
 			return i;
 
@@ -1022,9 +1020,9 @@ int snprint_path_attr(const struct gen_path* gp,
 
 static int pgd_lookup(char wildcard)
 {
-	int i;
+	unsigned int i;
 
-	for (i = 0; pgd[i].header; i++)
+	for (i = 0; i < ARRAY_SIZE(pgd); i++)
 		if (pgd[i].wildcard == wildcard)
 			return i;
 
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 05/11] libmultipath: drop padding code in _snprint_pathgroup()

[mwilck]

From: Martin Wilck <mwilck@suse.com>

We don't use padding for pathgroup fields, field width is always 0.
Remove this dead code.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index fcbaa5a..924bca3 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1167,25 +1167,16 @@ int _snprint_pathgroup(const struct gen_pathgroup *ggp, struct strbuf *line,
 {
 	int initial_len = get_strbuf_len(line);
 	const char *f;
-	struct pathgroup_data *data;
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
-		int iwc;
-
 		if ((rc = __append_strbuf_str(line, format, f - format)) < 0)
 			return rc;
 
 		format = f + 1;
-		if ((iwc = pgd_lookup(*format)) == -1)
-			continue; /* unknown wildcard */
-		data = &pgd[iwc];
 
 		if ((rc = ggp->ops->snprint(ggp, line, *format)) < 0)
 			return rc;
-		else if ((unsigned int)rc < data->width)
-			if ((rc = fill_strbuf(line, ' ', data->width - rc)) < 0)
-				return rc;
 	}
 
 	if ((rc = print_strbuf(line, "%s\n", format)) < 0)
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 06/11] libmultipath: snprint_foreign_topology(): split out lockless variant

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Factor out the lockless part, which we'll call from elsewhere in a
future patch.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/foreign.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/libmultipath/foreign.c b/libmultipath/foreign.c
index e091a1d..cb8765c 100644
--- a/libmultipath/foreign.c
+++ b/libmultipath/foreign.c
@@ -498,19 +498,12 @@ void foreign_multipath_layout(void)
 	pthread_cleanup_pop(1);
 }
 
-int snprint_foreign_topology(struct strbuf *buf, int verbosity)
+static int __snprint_foreign_topology(struct strbuf *buf, int verbosity)
 {
 	struct foreign *fgn;
 	int i;
 	size_t initial_len = get_strbuf_len(buf);
 
-	rdlock_foreigns();
-	if (foreigns == NULL) {
-		unlock_foreigns(NULL);
-		return 0;
-	}
-	pthread_cleanup_push(unlock_foreigns, NULL);
-
 	vector_foreach_slot(foreigns, fgn, i) {
 		const struct _vector *vec;
 		const struct gen_multipath *gm;
@@ -531,10 +524,24 @@ int snprint_foreign_topology(struct strbuf *buf, int verbosity)
 		pthread_cleanup_pop(1);
 	}
 
-	pthread_cleanup_pop(1);
 	return get_strbuf_len(buf) - initial_len;
 }
 
+int snprint_foreign_topology(struct strbuf *buf, int verbosity)
+{
+	int rc;
+
+        rdlock_foreigns();
+	if (foreigns == NULL) {
+		unlock_foreigns(NULL);
+		return 0;
+	}
+	pthread_cleanup_push(unlock_foreigns, NULL);
+	rc = __snprint_foreign_topology(buf, verbosity);
+	pthread_cleanup_pop(1);
+	return rc;
+}
+
 void print_foreign_topology(int verbosity)
 {
 	STRBUF_ON_STACK(buf);
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 07/11] multipathd: drop unnecessary path layout calls

[mwilck]

From: Martin Wilck <mwilck@suse.com>

In cli_list_map_fmt(), we only need field widths for map properties,
not for path properties. In json ouput, we need no field width padding
at all. Likewise in the prstatus related functions.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/cli_handlers.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 1b563e7..db7646b 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -277,8 +277,7 @@ cli_list_map_json (void *v, struct strbuf *reply, void *data)
 	struct vectors * vecs = (struct vectors *)data;
 	char * param = get_keyparam(v, MAP);
 
-	param = convert_dev(param, 0);
-	get_path_layout(vecs->pathvec, 0);
+        param = convert_dev(param, 0);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 
 	if (!mpp)
@@ -400,7 +399,6 @@ cli_list_map_fmt (void *v, struct strbuf *reply, void *data)
 	char * fmt = get_keyparam(v, FMT);
 
 	param = convert_dev(param, 0);
-	get_path_layout(vecs->pathvec, 0);
 	get_multipath_layout(vecs->mpvec, 1);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 	if (!mpp)
@@ -1266,8 +1264,7 @@ cli_getprstatus (void * v, struct strbuf *reply, void * data)
 	struct vectors * vecs = (struct vectors *)data;
 	char * param = get_keyparam(v, MAP);
 
-	param = convert_dev(param, 0);
-	get_path_layout(vecs->pathvec, 0);
+        param = convert_dev(param, 0);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 
 	if (!mpp)
@@ -1291,7 +1288,6 @@ cli_setprstatus(void * v, struct strbuf *reply, void * data)
 	char * param = get_keyparam(v, MAP);
 
 	param = convert_dev(param, 0);
-	get_path_layout(vecs->pathvec, 0);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 
 	if (!mpp)
@@ -1314,7 +1310,6 @@ cli_unsetprstatus(void * v, struct strbuf *reply, void * data)
 	char * param = get_keyparam(v, MAP);
 
 	param = convert_dev(param, 0);
-	get_path_layout(vecs->pathvec, 0);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 
 	if (!mpp)
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 08/11] libmultipath: add a cleanup function for unsigned char *

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/util.c | 5 +++++
 libmultipath/util.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/libmultipath/util.c b/libmultipath/util.c
index b7952a5..ce5ea73 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -466,3 +466,8 @@ void cleanup_charp(char **p)
 {
 	free(*p);
 }
+
+void cleanup_ucharp(unsigned char **p)
+{
+	free(*p);
+}
diff --git a/libmultipath/util.h b/libmultipath/util.h
index 421842a..79d9f32 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -125,4 +125,5 @@ static inline void clear_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
 	})
 
 void cleanup_charp(char **p);
+void cleanup_ucharp(unsigned char **p);
 #endif /* _UTIL_H */
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 09/11] libmultipath: make sprint_path_marginal() static

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 924bca3..e813f63 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -776,7 +776,7 @@ snprint_path_protocol(struct strbuf *buff, const struct path * pp)
 	}
 }
 
-int
+static int
 snprint_path_marginal(struct strbuf *buff, const struct path * pp)
 {
 	if (pp->marginal)
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 10/11] libmultipath: introduce width argument for pretty-printing functions

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Instead of modifying the global variables to store the field width for
pretty-printed table-formatted output, use field-width arrays passed
down from the caller. The calling function needs to allocate the array
using alloc_path_layout() or alloc_multipath_layout(), and initialize the
layout with the data to be printed. This is no different than before,
except that the field width is now stored in local variables of the caller.

The type used for column width, fieldwidth_t, is defined to unsigned char.
This limits the size of fields (padding in table columns) to 255 characters.
That's plenty even for lengthy WWID formats (as WWID_SIZE is 128). The typedef
could be changed in the future if necessary.

In some functions, namely _print_multipath_topology() and
print_foreign_topology(), field widths need to be initialized. This was true
before this patch already, but it wasn't done as the necessity wasn't
apparent. Essentially we were using pre-calculated field widths in these
functions, or just 0 if no field width calculation had been done previously.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/foreign.c            |  53 ++++++++----
 libmultipath/foreign.h            |  25 ++++--
 libmultipath/libmultipath.version |   5 +-
 libmultipath/print.c              | 131 ++++++++++++++++++------------
 libmultipath/print.h              |  39 +++++----
 multipath/main.c                  |   7 +-
 multipathd/cli_handlers.c         |  70 ++++++++++------
 7 files changed, 215 insertions(+), 115 deletions(-)

diff --git a/libmultipath/foreign.c b/libmultipath/foreign.c
index cb8765c..988208e 100644
--- a/libmultipath/foreign.c
+++ b/libmultipath/foreign.c
@@ -437,7 +437,7 @@ void check_foreign(void)
 }
 
 /* Call this after get_path_layout */
-void foreign_path_layout(void)
+void foreign_path_layout(fieldwidth_t *width)
 {
 	struct foreign *fgn;
 	int i;
@@ -457,7 +457,7 @@ void foreign_path_layout(void)
 
 		vec = fgn->get_paths(fgn->context);
 		if (vec != NULL) {
-			_get_path_layout(vec, LAYOUT_RESET_NOT);
+			_get_path_layout(vec, LAYOUT_RESET_NOT, width);
 		}
 		fgn->release_paths(fgn->context, vec);
 
@@ -468,7 +468,7 @@ void foreign_path_layout(void)
 }
 
 /* Call this after get_multipath_layout */
-void foreign_multipath_layout(void)
+void foreign_multipath_layout(fieldwidth_t *width)
 {
 	struct foreign *fgn;
 	int i;
@@ -488,7 +488,7 @@ void foreign_multipath_layout(void)
 
 		vec = fgn->get_multipaths(fgn->context);
 		if (vec != NULL) {
-			_get_multipath_layout(vec, LAYOUT_RESET_NOT);
+			_get_multipath_layout(vec, LAYOUT_RESET_NOT, width);
 		}
 		fgn->release_multipaths(fgn->context, vec);
 
@@ -498,7 +498,8 @@ void foreign_multipath_layout(void)
 	pthread_cleanup_pop(1);
 }
 
-static int __snprint_foreign_topology(struct strbuf *buf, int verbosity)
+static int __snprint_foreign_topology(struct strbuf *buf, int verbosity,
+				      const fieldwidth_t *width)
 {
 	struct foreign *fgn;
 	int i;
@@ -516,7 +517,7 @@ static int __snprint_foreign_topology(struct strbuf *buf, int verbosity)
 		if (vec != NULL) {
 			vector_foreach_slot(vec, gm, j) {
 				if (_snprint_multipath_topology(
-					    gm, buf, verbosity) < 0)
+					    gm, buf, verbosity, width) < 0)
 					break;
 			}
 		}
@@ -527,7 +528,8 @@ static int __snprint_foreign_topology(struct strbuf *buf, int verbosity)
 	return get_strbuf_len(buf) - initial_len;
 }
 
-int snprint_foreign_topology(struct strbuf *buf, int verbosity)
+int snprint_foreign_topology(struct strbuf *buf, int verbosity,
+			     const fieldwidth_t *width)
 {
 	int rc;
 
@@ -537,7 +539,7 @@ int snprint_foreign_topology(struct strbuf *buf, int verbosity)
 		return 0;
 	}
 	pthread_cleanup_push(unlock_foreigns, NULL);
-	rc = __snprint_foreign_topology(buf, verbosity);
+	rc = __snprint_foreign_topology(buf, verbosity, width);
 	pthread_cleanup_pop(1);
 	return rc;
 }
@@ -545,12 +547,35 @@ int snprint_foreign_topology(struct strbuf *buf, int verbosity)
 void print_foreign_topology(int verbosity)
 {
 	STRBUF_ON_STACK(buf);
+	struct foreign *fgn;
+	int i;
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
-	snprint_foreign_topology(&buf, verbosity);
+	if ((width = alloc_path_layout()) == NULL)
+		return;
+        rdlock_foreigns();
+	if (foreigns == NULL) {
+		unlock_foreigns(NULL);
+		return;
+	}
+	pthread_cleanup_push(unlock_foreigns, NULL);
+	vector_foreach_slot(foreigns, fgn, i) {
+		const struct _vector *vec;
+
+		fgn->lock(fgn->context);
+		pthread_cleanup_push(fgn->unlock, fgn->context);
+		vec = fgn->get_paths(fgn->context);
+		_get_multipath_layout(vec, LAYOUT_RESET_NOT, width);
+		fgn->release_paths(fgn->context, vec);
+		pthread_cleanup_pop(1);
+	}
+	__snprint_foreign_topology(&buf, verbosity, width);
+	pthread_cleanup_pop(1);
 	printf("%s", get_strbuf_str(&buf));
 }
 
-int snprint_foreign_paths(struct strbuf *buf, const char *style, int pretty)
+int snprint_foreign_paths(struct strbuf *buf, const char *style,
+			  const fieldwidth_t *width)
 {
 	struct foreign *fgn;
 	int i;
@@ -574,7 +599,7 @@ int snprint_foreign_paths(struct strbuf *buf, const char *style, int pretty)
 		vec = fgn->get_paths(fgn->context);
 		if (vec != NULL) {
 			vector_foreach_slot(vec, gp, j) {
-				ret = _snprint_path(gp, buf, style, pretty);
+				ret = _snprint_path(gp, buf, style, width);
 				if (ret < 0)
 					break;
 			}
@@ -589,8 +614,8 @@ int snprint_foreign_paths(struct strbuf *buf, const char *style, int pretty)
 	return get_strbuf_len(buf) - initial_len;
 }
 
-int snprint_foreign_multipaths(struct strbuf *buf,
-			       const char *style, int pretty)
+int snprint_foreign_multipaths(struct strbuf *buf, const char *style,
+			       const fieldwidth_t *width)
 {
 	struct foreign *fgn;
 	int i;
@@ -615,7 +640,7 @@ int snprint_foreign_multipaths(struct strbuf *buf,
 		if (vec != NULL) {
 			vector_foreach_slot(vec, gm, j) {
 				ret = _snprint_multipath(gm, buf,
-							 style, pretty);
+							 style, width);
 				if (ret < 0)
 					break;
 			}
diff --git a/libmultipath/foreign.h b/libmultipath/foreign.h
index 77fc485..b5e1bc6 100644
--- a/libmultipath/foreign.h
+++ b/libmultipath/foreign.h
@@ -18,10 +18,12 @@
 #define _FOREIGN_H
 #include <stdbool.h>
 #include <libudev.h>
-#define LIBMP_FOREIGN_API ((1 << 8) | 1)
+#define LIBMP_FOREIGN_API ((1 << 8) | 2)
 
 struct strbuf;
 struct context;
+/* This must match the definition in print.h */
+typedef unsigned char fieldwidth_t;
 
 /* return codes of functions below returning "int" */
 enum foreign_retcode {
@@ -252,15 +254,17 @@ void check_foreign(void);
  * foreign_path_layout()
  * call this before printing paths, after get_path_layout(), to determine
  * output field width.
+ * @param width: an array allocated by alloc_path_layout()
  */
-void foreign_path_layout(void);
+void foreign_path_layout(fieldwidth_t *width);
 
 /**
  * foreign_multipath_layout()
  * call this before printing maps, after get_multipath_layout(), to determine
  * output field width.
+ * @param width: an array allocated by alloc_multipath_layout()
  */
-void foreign_multipath_layout(void);
+void foreign_multipath_layout(fieldwidth_t *width);
 
 /**
  * snprint_foreign_topology(buf, len, verbosity);
@@ -268,9 +272,11 @@ void foreign_multipath_layout(void);
  * '\0' - terminated.
  * @param buf: output buffer
  * @param verbosity: verbosity level
+ * @param width: an array of field widths, initialized by _get_path_layout()
  * @returns: number of printed characters excluding trailing '\0'.
  */
-int snprint_foreign_topology(struct strbuf *buf, int verbosity);
+int snprint_foreign_topology(struct strbuf *buf, int verbosity,
+			     const fieldwidth_t *width);
 
 /**
  * snprint_foreign_paths(buf, len, style, pad);
@@ -278,10 +284,11 @@ int snprint_foreign_topology(struct strbuf *buf, int verbosity);
  * '\0' - terminated.
  * @param buf: output buffer
  * @param style: format string
- * @param pad: whether to pad field width
+ * @param width: array initialized with get_path_layout(), or NULL for no padding
  * @returns: number of printed characters excluding trailing '\0'.
  */
-int snprint_foreign_paths(struct strbuf *buf, const char *style, int pad);
+int snprint_foreign_paths(struct strbuf *buf, const char *style,
+			  const fieldwidth_t *width);
 
 /**
  * snprint_foreign_multipaths(buf, len, style, pad);
@@ -289,11 +296,11 @@ int snprint_foreign_paths(struct strbuf *buf, const char *style, int pad);
  * '\0' - terminated.
  * @param buf: output buffer
  * @param style: format string
- * @param pad: whether to pad field width
+ * @param width: array initialized with get_path_layout(), or NULL for no padding
  * @returns: number of printed characters excluding trailing '\0'.
  */
-int snprint_foreign_multipaths(struct strbuf *buf,
-			       const char *style, int pretty);
+int snprint_foreign_multipaths(struct strbuf *buf, const char *style,
+			       const fieldwidth_t *width);
 
 /**
  * print_foreign_topology(v)
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index 4c22995..0c5417d 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -31,14 +31,16 @@
  *   The new version inherits the previous ones.
  */
 
-LIBMULTIPATH_12.0.0 {
+LIBMULTIPATH_13.0.0 {
 global:
 	/* symbols referenced by multipath and multipathd */
 	add_foreign;
 	add_map_with_path;
 	adopt_paths;
 	alloc_multipath;
+	alloc_multipath_layout;
 	alloc_path;
+	alloc_path_layout;
 	alloc_path_with_pathinfo;
 	alloc_strvec;
 	change_foreign;
@@ -270,6 +272,7 @@ global:
 
 	/* added in 7.0.0 */
 	cleanup_charp;
+	cleanup_ucharp;
 
 	/* added in 8.1.0 */
 	reset_strbuf;
diff --git a/libmultipath/print.c b/libmultipath/print.c
index e813f63..b41796e 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -884,18 +884,22 @@ int snprint_wildcards(struct strbuf *buff)
 	return get_strbuf_len(buff) - initial_len;
 }
 
-void
-get_path_layout(vector pathvec, int header)
+fieldwidth_t *alloc_path_layout(void) {
+	return calloc(ARRAY_SIZE(pd), sizeof(fieldwidth_t));
+}
+
+void get_path_layout(vector pathvec, int header, fieldwidth_t *width)
 {
 	vector gpvec = vector_convert(NULL, pathvec, struct path,
 				      dm_path_to_gen);
 	_get_path_layout(gpvec,
-			 header ? LAYOUT_RESET_HEADER : LAYOUT_RESET_ZERO);
+			 header ? LAYOUT_RESET_HEADER : LAYOUT_RESET_ZERO,
+			 width);
 	vector_free(gpvec);
 }
 
 static void
-reset_width(unsigned int *width, enum layout_reset reset, const char *header)
+reset_width(fieldwidth_t *width, enum layout_reset reset, const char *header)
 {
 	switch (reset) {
 	case LAYOUT_RESET_HEADER:
@@ -910,67 +914,70 @@ reset_width(unsigned int *width, enum layout_reset reset, const char *header)
 	}
 }
 
-void
-_get_path_layout (const struct _vector *gpvec, enum layout_reset reset)
+void _get_path_layout (const struct _vector *gpvec, enum layout_reset reset,
+		       fieldwidth_t *width)
 {
 	unsigned int i, j;
 	const struct gen_path *gp;
 
+	if (width == NULL)
+		return;
+
         for (j = 0; j < ARRAY_SIZE(pd); j++) {
 		STRBUF_ON_STACK(buff);
 
-		reset_width(&pd[j].width, reset, pd[j].header);
+		reset_width(&width[j], reset, pd[j].header);
 
 		if (gpvec == NULL)
 			continue;
 
 		vector_foreach_slot (gpvec, gp, i) {
 			gp->ops->snprint(gp, &buff, pd[j].wildcard);
-			pd[j].width = MAX(pd[j].width, get_strbuf_len(&buff));
+			width[j] = MAX(width[j],
+				       MIN(get_strbuf_len(&buff), MAX_FIELD_WIDTH));
 			truncate_strbuf(&buff, 0);
 		}
 	}
 }
 
-static void
-reset_multipath_layout (void)
-{
-	unsigned int i;
+fieldwidth_t *alloc_multipath_layout(void) {
 
-	for (i = 0; i < ARRAY_SIZE(mpd); i++)
-		mpd[i].width = 0;
+	return calloc(ARRAY_SIZE(mpd), sizeof(fieldwidth_t));
 }
 
-void
-get_multipath_layout (vector mpvec, int header) {
+void get_multipath_layout (vector mpvec, int header, fieldwidth_t *width) {
 	vector gmvec = vector_convert(NULL, mpvec, struct multipath,
 				      dm_multipath_to_gen);
 	_get_multipath_layout(gmvec,
-			 header ? LAYOUT_RESET_HEADER : LAYOUT_RESET_ZERO);
-	vector_free(gmvec);
+			      header ? LAYOUT_RESET_HEADER : LAYOUT_RESET_ZERO,
+			      width);
+        vector_free(gmvec);
 }
 
 void
-_get_multipath_layout (const struct _vector *gmvec,
-			    enum layout_reset reset)
+_get_multipath_layout (const struct _vector *gmvec, enum layout_reset reset,
+		       fieldwidth_t *width)
 {
 	unsigned int i, j;
 	const struct gen_multipath * gm;
 
+	if (width == NULL)
+		return;
 	for (j = 0; j < ARRAY_SIZE(mpd); j++) {
 		STRBUF_ON_STACK(buff);
 
-		reset_width(&mpd[j].width, reset, mpd[j].header);
+		reset_width(&width[j], reset, mpd[j].header);
 
 		if (gmvec == NULL)
 			continue;
 
 		vector_foreach_slot (gmvec, gm, i) {
 			gm->ops->snprint(gm, &buff, mpd[j].wildcard);
-			mpd[j].width = MAX(mpd[j].width, get_strbuf_len(&buff));
+			width[j] = MAX(width[j],
+				       MIN(get_strbuf_len(&buff), MAX_FIELD_WIDTH));
 			truncate_strbuf(&buff, 0);
 		}
-		condlog(4, "%s: width %d", mpd[j].header, mpd[j].width);
+		condlog(4, "%s: width %d", mpd[j].header, width[j]);
 	}
 }
 
@@ -1040,7 +1047,8 @@ int snprint_pathgroup_attr(const struct gen_pathgroup* gpg,
 	return pgd[i].snprint(buf, pg);
 }
 
-int snprint_multipath_header(struct strbuf *line, const char *format)
+int snprint_multipath_header(struct strbuf *line, const char *format,
+			     const fieldwidth_t *width)
 {
 	int initial_len = get_strbuf_len(line);
 	const char *f;
@@ -1060,8 +1068,8 @@ int snprint_multipath_header(struct strbuf *line, const char *format)
 
 		if ((rc = append_strbuf_str(line, data->header)) < 0)
 			return rc;
-		else if ((unsigned int)rc < data->width)
-			if ((rc = fill_strbuf(line, ' ', data->width - rc)) < 0)
+		else if ((unsigned int)rc < width[iwc])
+			if ((rc = fill_strbuf(line, ' ', width[iwc] - rc)) < 0)
 				return rc;
 	}
 
@@ -1071,11 +1079,11 @@ int snprint_multipath_header(struct strbuf *line, const char *format)
 }
 
 int _snprint_multipath(const struct gen_multipath *gmp,
-		       struct strbuf *line, const char *format, int pad)
+		       struct strbuf *line, const char *format,
+		       const fieldwidth_t *width)
 {
 	int initial_len = get_strbuf_len(line);
 	const char *f;
-	struct multipath_data * data;
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
@@ -1087,12 +1095,11 @@ int _snprint_multipath(const struct gen_multipath *gmp,
 		format = f + 1;
 		if ((iwc = mpd_lookup(*format)) == -1)
 			continue; /* unknown wildcard */
-		data = &mpd[iwc];
 
 		if ((rc = gmp->ops->snprint(gmp, line, *format)) < 0)
 			return rc;
-		else if (pad && (unsigned int)rc < data->width)
-			if ((rc = fill_strbuf(line, ' ', data->width - rc)) < 0)
+		else if (width != NULL && (unsigned int)rc < width[iwc])
+			if ((rc = fill_strbuf(line, ' ', width[iwc] - rc)) < 0)
 				return rc;
 	}
 
@@ -1101,7 +1108,8 @@ int _snprint_multipath(const struct gen_multipath *gmp,
 	return get_strbuf_len(line) - initial_len;
 }
 
-int snprint_path_header(struct strbuf *line, const char *format)
+int snprint_path_header(struct strbuf *line, const char *format,
+			const fieldwidth_t *width)
 {
 	int initial_len = get_strbuf_len(line);
 	const char *f;
@@ -1121,8 +1129,8 @@ int snprint_path_header(struct strbuf *line, const char *format)
 
 		if ((rc = append_strbuf_str(line, data->header)) < 0)
 			return rc;
-		else if ((unsigned int)rc < data->width)
-			if ((rc = fill_strbuf(line, ' ', data->width - rc)) < 0)
+		else if ((unsigned int)rc < width[iwc])
+			if ((rc = fill_strbuf(line, ' ', width[iwc] - rc)) < 0)
 				return rc;
 	}
 
@@ -1132,11 +1140,10 @@ int snprint_path_header(struct strbuf *line, const char *format)
 }
 
 int _snprint_path(const struct gen_path *gp, struct strbuf *line,
-		  const char *format, int pad)
+		  const char *format, const fieldwidth_t *width)
 {
 	int initial_len = get_strbuf_len(line);
 	const char *f;
-	struct path_data * data;
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
@@ -1148,12 +1155,11 @@ int _snprint_path(const struct gen_path *gp, struct strbuf *line,
 		format = f + 1;
 		if ((iwc = pd_lookup(*format)) == -1)
 			continue; /* unknown wildcard */
-		data = &pd[iwc];
 
 		if ((rc = gp->ops->snprint(gp, line, *format)) < 0)
 			return rc;
-		else if (pad && (unsigned int)rc < data->width)
-			if ((rc = fill_strbuf(line, ' ', data->width - rc)) < 0)
+		else if (width != NULL && (unsigned int)rc < width[iwc])
+			if ((rc = fill_strbuf(line, ' ', width[iwc] - rc)) < 0)
 				return rc;
 	}
 
@@ -1190,8 +1196,26 @@ int _snprint_pathgroup(const struct gen_pathgroup *ggp, struct strbuf *line,
 void _print_multipath_topology(const struct gen_multipath *gmp, int verbosity)
 {
 	STRBUF_ON_STACK(buff);
+	fieldwidth_t *p_width __attribute__((cleanup(cleanup_ucharp))) = NULL;
+	const struct gen_pathgroup *gpg;
+	const struct _vector *pgvec, *pathvec;
+	int j;
 
-	_snprint_multipath_topology(gmp, &buff, verbosity);
+	p_width = alloc_path_layout();
+        pgvec = gmp->ops->get_pathgroups(gmp);
+
+	if (pgvec != NULL) {
+                vector_foreach_slot (pgvec, gpg, j) {
+			pathvec = gpg->ops->get_paths(gpg);
+			if (pathvec == NULL)
+				continue;
+			_get_path_layout(pathvec, LAYOUT_RESET_NOT, p_width);
+			gpg->ops->rel_paths(gpg, pathvec);
+		}
+		gmp->ops->rel_pathgroups(gmp, pgvec);
+	}
+
+        _snprint_multipath_topology(gmp, &buff, verbosity, p_width);
 	printf("%s", get_strbuf_str(&buff));
 }
 
@@ -1211,21 +1235,24 @@ int snprint_multipath_style(const struct gen_multipath *gmp,
 }
 
 int _snprint_multipath_topology(const struct gen_multipath *gmp,
-				struct strbuf *buff, int verbosity)
+				struct strbuf *buff, int verbosity,
+				const fieldwidth_t *p_width)
 {
 	int j, i, rc;
 	const struct _vector *pgvec;
 	const struct gen_pathgroup *gpg;
 	STRBUF_ON_STACK(style);
 	size_t initial_len = get_strbuf_len(buff);
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
 	if (verbosity <= 0)
 		return 0;
 
-	reset_multipath_layout();
+	if ((width = alloc_multipath_layout()) == NULL)
+		return -ENOMEM;
 
 	if (verbosity == 1)
-		return _snprint_multipath(gmp, buff, "%n", 1);
+		return _snprint_multipath(gmp, buff, "%n", width);
 
 	if(isatty(1) &&
 	   (rc = print_strbuf(&style, "%c[%dm", 0x1B, 1)) < 0) /* bold on */
@@ -1236,8 +1263,8 @@ int _snprint_multipath_topology(const struct gen_multipath *gmp,
 	   (rc = print_strbuf(&style, "%c[%dm", 0x1B, 0)) < 0) /* bold off */
 		return rc;
 
-	if ((rc = _snprint_multipath(gmp, buff, get_strbuf_str(&style), 1)) < 0
-	    || (rc = _snprint_multipath(gmp, buff, PRINT_MAP_PROPS, 1)) < 0)
+	if ((rc = _snprint_multipath(gmp, buff, get_strbuf_str(&style), width)) < 0
+	    || (rc = _snprint_multipath(gmp, buff, PRINT_MAP_PROPS, width)) < 0)
 		return rc;
 
 	pgvec = gmp->ops->get_pathgroups(gmp);
@@ -1264,7 +1291,7 @@ int _snprint_multipath_topology(const struct gen_multipath *gmp,
 					       i + 1 == VECTOR_SIZE(pathvec) ?
 					       '`': '|')) < 0 ||
 			    (rc = _snprint_path(gp, buff,
-						PRINT_PATH_INDENT, 1)) < 0)
+						PRINT_PATH_INDENT, p_width)) < 0)
 				return rc;
 		}
 		gpg->ops->rel_paths(gpg, pathvec);
@@ -1949,6 +1976,7 @@ static void print_all_paths_custo(vector pathvec, int banner, const char *fmt)
 	int i;
 	struct path * pp;
 	STRBUF_ON_STACK(line);
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
 	if (!VECTOR_SIZE(pathvec)) {
 		if (banner)
@@ -1956,14 +1984,17 @@ static void print_all_paths_custo(vector pathvec, int banner, const char *fmt)
 		return;
 	}
 
-	if (banner)
+	if ((width = alloc_path_layout()) == NULL)
+		return;
+	get_path_layout(pathvec, 1, width);
+
+        if (banner)
 		append_strbuf_str(&line, "===== paths list =====\n");
 
-	get_path_layout(pathvec, 1);
-	snprint_path_header(&line, fmt);
+	snprint_path_header(&line, fmt, width);
 
 	vector_foreach_slot (pathvec, pp, i)
-		snprint_path(&line, fmt, pp, 1);
+		snprint_path(&line, fmt, pp, width);
 
 	printf("%s", get_strbuf_str(&line));
 }
diff --git a/libmultipath/print.h b/libmultipath/print.h
index 4322f6e..3f02747 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -15,23 +15,32 @@ enum layout_reset {
 	LAYOUT_RESET_HEADER,
 };
 
-void _get_path_layout (const struct _vector *gpvec, enum layout_reset);
-void get_path_layout (vector pathvec, int header);
-void _get_multipath_layout (const struct _vector *gmvec, enum layout_reset);
-void get_multipath_layout (vector mpvec, int header);
-int snprint_path_header(struct strbuf *, const char *);
-int snprint_multipath_header(struct strbuf *, const char *);
-int _snprint_path (const struct gen_path *, struct strbuf *, const char *, int);
-#define snprint_path(buf, fmt, pp, v) \
-	_snprint_path(dm_path_to_gen(pp), buf, fmt,  v)
+typedef unsigned char fieldwidth_t;
+#define MAX_FIELD_WIDTH UCHAR_MAX
+
+fieldwidth_t *alloc_path_layout(void);
+void _get_path_layout (const struct _vector *gpvec, enum layout_reset,
+		       fieldwidth_t *width);
+void get_path_layout (vector pathvec, int header, fieldwidth_t *width);
+fieldwidth_t *alloc_multipath_layout(void);
+void _get_multipath_layout (const struct _vector *gmvec, enum layout_reset,
+			    fieldwidth_t *width);
+void get_multipath_layout (vector mpvec, int header, fieldwidth_t *width);
+int snprint_path_header(struct strbuf *, const char *, const fieldwidth_t *);
+int snprint_multipath_header(struct strbuf *, const char *,
+			     const fieldwidth_t *);
+int _snprint_path (const struct gen_path *, struct strbuf *, const char *,
+		   const fieldwidth_t *);
+#define snprint_path(buf, fmt, pp, w)		\
+	_snprint_path(dm_path_to_gen(pp), buf, fmt, w)
 int _snprint_multipath (const struct gen_multipath *, struct strbuf *,
-			const char *, int);
-#define snprint_multipath(buf, fmt, mp, v)				\
-	_snprint_multipath(dm_multipath_to_gen(mp), buf, fmt,  v)
+			const char *, const fieldwidth_t *);
+#define snprint_multipath(buf, fmt, mp, w)				\
+	_snprint_multipath(dm_multipath_to_gen(mp), buf, fmt, w)
 int _snprint_multipath_topology (const struct gen_multipath *, struct strbuf *,
-				 int verbosity);
-#define snprint_multipath_topology(buf, mpp, v) \
-	_snprint_multipath_topology (dm_multipath_to_gen(mpp), buf, v)
+				 int verbosity, const fieldwidth_t *);
+#define snprint_multipath_topology(buf, mpp, v, w)			\
+	_snprint_multipath_topology (dm_multipath_to_gen(mpp), buf, v, w)
 int snprint_multipath_topology_json(struct strbuf *, const struct vectors *vecs);
 int __snprint_config(const struct config *conf, struct strbuf *buff,
 		     const struct _vector *hwtable, const struct _vector *mpvec);
diff --git a/multipath/main.c b/multipath/main.c
index 5711acf..0a9377e 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -460,6 +460,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
 	int di_flag = 0;
 	char * refwwid = NULL;
 	char * dev = NULL;
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
 	/*
 	 * allocate core vectors to store paths and multipaths
@@ -546,8 +547,10 @@ configure (struct config *conf, enum mpath_cmds cmd,
 	if (libmp_verbosity > 2)
 		print_all_paths(pathvec, 1);
 
-	get_path_layout(pathvec, 0);
-	foreign_path_layout();
+	if ((width = alloc_path_layout()) == NULL)
+		goto out;
+	get_path_layout(pathvec, 0, width);
+	foreign_path_layout(width);
 
 	if (get_dm_mpvec(cmd, curmp, pathvec, refwwid))
 		goto out;
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index db7646b..e47813b 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -38,18 +38,22 @@ show_paths (struct strbuf *reply, struct vectors *vecs, char *style, int pretty)
 	int i;
 	struct path * pp;
 	int hdr_len = 0;
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
-	get_path_layout(vecs->pathvec, 1);
-	foreign_path_layout();
-
-	if (pretty && (hdr_len = snprint_path_header(reply, style)) < 0)
+	if (pretty) {
+		if ((width = alloc_path_layout()) == NULL)
+			return 1;
+		get_path_layout(vecs->pathvec, 1, width);
+		foreign_path_layout(width);
+	}
+	if (pretty && (hdr_len = snprint_path_header(reply, style, width)) < 0)
 		return 1;
 
 	vector_foreach_slot(vecs->pathvec, pp, i) {
-		if (snprint_path(reply, style, pp, pretty) < 0)
+		if (snprint_path(reply, style, pp, width) < 0)
 			return 1;
 	}
-	if (snprint_foreign_paths(reply, style, pretty) < 0)
+	if (snprint_foreign_paths(reply, style, width) < 0)
 		return 1;
 
 	if (pretty && get_strbuf_len(reply) == (size_t)hdr_len)
@@ -63,7 +67,11 @@ static int
 show_path (struct strbuf *reply, struct vectors *vecs, struct path *pp,
 	   char *style)
 {
-	get_path_layout(vecs->pathvec, 1);
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
+
+        if ((width = alloc_path_layout()) == NULL)
+		return 1;
+	get_path_layout(vecs->pathvec, 1, width);
 	if (snprint_path(reply, style, pp, 0) < 0)
 		return 1;
 	return 0;
@@ -71,12 +79,12 @@ show_path (struct strbuf *reply, struct vectors *vecs, struct path *pp,
 
 static int
 show_map_topology (struct strbuf *reply, struct multipath *mpp,
-		   struct vectors *vecs)
+		   struct vectors *vecs, const fieldwidth_t *width)
 {
 	if (update_multipath(vecs, mpp->alias, 0))
 		return 1;
 
-	if (snprint_multipath_topology(reply, mpp, 2) < 0)
+	if (snprint_multipath_topology(reply, mpp, 2, width) < 0)
 		return 1;
 
 	return 0;
@@ -87,19 +95,22 @@ show_maps_topology (struct strbuf *reply, struct vectors * vecs)
 {
 	int i;
 	struct multipath * mpp;
+	fieldwidth_t *p_width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
-	get_path_layout(vecs->pathvec, 0);
-	foreign_path_layout();
+	if ((p_width = alloc_path_layout()) == NULL)
+		return 1;
+	get_path_layout(vecs->pathvec, 0, p_width);
+	foreign_path_layout(p_width);
 
 	vector_foreach_slot(vecs->mpvec, mpp, i) {
 		if (update_multipath(vecs, mpp->alias, 0)) {
 			i--;
 			continue;
 		}
-		if (snprint_multipath_topology(reply, mpp, 2) < 0)
+		if (snprint_multipath_topology(reply, mpp, 2, p_width) < 0)
 			return 1;
 	}
-	if (snprint_foreign_topology(reply, 2) < 0)
+	if (snprint_foreign_topology(reply, 2, p_width) < 0)
 		return 1;
 
 	return 0;
@@ -247,9 +258,12 @@ cli_list_map_topology (void *v, struct strbuf *reply, void *data)
 	struct multipath * mpp;
 	struct vectors * vecs = (struct vectors *)data;
 	char * param = get_keyparam(v, MAP);
+	fieldwidth_t *p_width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
+	if ((p_width = alloc_path_layout()) == NULL)
+		return 1;
+	get_path_layout(vecs->pathvec, 0, p_width);
 	param = convert_dev(param, 0);
-	get_path_layout(vecs->pathvec, 0);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 
 	if (!mpp)
@@ -257,7 +271,7 @@ cli_list_map_topology (void *v, struct strbuf *reply, void *data)
 
 	condlog(3, "list multipath %s (operator)", param);
 
-	return show_map_topology(reply, mpp, vecs);
+	return show_map_topology(reply, mpp, vecs, p_width);
 }
 
 static int
@@ -328,9 +342,9 @@ show_daemon (struct strbuf *reply)
 
 static int
 show_map (struct strbuf *reply, struct multipath *mpp, char *style,
-	  int pretty)
+	  const fieldwidth_t *width)
 {
-	if (snprint_multipath(reply, style, mpp, pretty) < 0)
+	if (snprint_multipath(reply, style, mpp, width) < 0)
 		return 1;
 
 	return 0;
@@ -343,11 +357,16 @@ show_maps (struct strbuf *reply, struct vectors *vecs, char *style,
 	int i;
 	struct multipath * mpp;
 	int hdr_len = 0;
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
-	get_multipath_layout(vecs->mpvec, 1);
-	foreign_multipath_layout();
+	if (pretty) {
+		if ((width = alloc_multipath_layout()) == NULL)
+			return 1;
+		get_multipath_layout(vecs->mpvec, 1, width);
+		foreign_multipath_layout(width);
+	}
 
-	if (pretty && (hdr_len = snprint_multipath_header(reply, style)) < 0)
+	if (pretty && (hdr_len = snprint_multipath_header(reply, style, width)) < 0)
 		return 1;
 
 	vector_foreach_slot(vecs->mpvec, mpp, i) {
@@ -355,10 +374,10 @@ show_maps (struct strbuf *reply, struct vectors *vecs, char *style,
 			i--;
 			continue;
 		}
-		if (snprint_multipath(reply, style, mpp, pretty) < 0)
+		if (snprint_multipath(reply, style, mpp, width) < 0)
 			return 1;
 	}
-	if (snprint_foreign_multipaths(reply, style, pretty) < 0)
+	if (snprint_foreign_multipaths(reply, style, width) < 0)
 		return 1;
 
 	if (pretty && get_strbuf_len(reply) == (size_t)hdr_len)
@@ -397,16 +416,19 @@ cli_list_map_fmt (void *v, struct strbuf *reply, void *data)
 	struct vectors * vecs = (struct vectors *)data;
 	char * param = get_keyparam(v, MAP);
 	char * fmt = get_keyparam(v, FMT);
+	fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL;
 
+	if ((width = alloc_multipath_layout()) == NULL)
+		return 1;
+	get_multipath_layout(vecs->pathvec, 1, width);
 	param = convert_dev(param, 0);
-	get_multipath_layout(vecs->mpvec, 1);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 	if (!mpp)
 		return 1;
 
 	condlog(3, "list map %s fmt %s (operator)", param, fmt);
 
-	return show_map(reply, mpp, fmt, 1);
+	return show_map(reply, mpp, fmt, width);
 }
 
 static int
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 11/11] libmultipath: change wildcard handler tables to static const

[mwilck]

From: Martin Wilck <mwilck@suse.com>

These arrays are actually constant lookup tables. Now that we use
local variables for field width, we can remove the width field from
these tables and make them static const, as they should be.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c | 119 +++++++++++++++++++++----------------------
 1 file changed, 58 insertions(+), 61 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index b41796e..3fe4402 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -103,21 +103,18 @@
 struct path_data {
 	char wildcard;
 	char * header;
-	unsigned int width;
 	int (*snprint)(struct strbuf *, const struct path * pp);
 };
 
 struct multipath_data {
 	char wildcard;
 	char * header;
-	unsigned int width;
 	int (*snprint)(struct strbuf *, const struct multipath * mpp);
 };
 
 struct pathgroup_data {
 	char wildcard;
 	char * header;
-	unsigned int width;
 	int (*snprint)(struct strbuf *, const struct pathgroup * pgp);
 };
 
@@ -792,66 +789,66 @@ snprint_path_vpd_data(struct strbuf *buff, const struct path * pp)
 	return append_strbuf_str(buff, "[undef]");
 }
 
-static struct multipath_data mpd[] = {
-	{'n', "name",          0, snprint_name},
-	{'w', "uuid",          0, snprint_multipath_uuid},
-	{'d', "sysfs",         0, snprint_sysfs},
-	{'F', "failback",      0, snprint_failback},
-	{'Q', "queueing",      0, snprint_queueing},
-	{'N', "paths",         0, snprint_nb_paths},
-	{'r', "write_prot",    0, snprint_ro},
-	{'t', "dm-st",         0, snprint_dm_map_state},
-	{'S', "size",          0, snprint_multipath_size},
-	{'f', "features",      0, snprint_features},
-	{'x', "failures",      0, snprint_map_failures},
-	{'h', "hwhandler",     0, snprint_hwhandler},
-	{'A', "action",        0, snprint_action},
-	{'0', "path_faults",   0, snprint_path_faults},
-	{'1', "switch_grp",    0, snprint_switch_grp},
-	{'2', "map_loads",     0, snprint_map_loads},
-	{'3', "total_q_time",  0, snprint_total_q_time},
-	{'4', "q_timeouts",    0, snprint_q_timeouts},
-	{'s', "vend/prod/rev", 0, snprint_multipath_vpr},
-	{'v', "vend",          0, snprint_multipath_vend},
-	{'p', "prod",          0, snprint_multipath_prod},
-	{'e', "rev",           0, snprint_multipath_rev},
-	{'G', "foreign",       0, snprint_multipath_foreign},
-	{'g', "vpd page data", 0, snprint_multipath_vpd_data},
+static const struct multipath_data mpd[] = {
+	{'n', "name",          snprint_name},
+	{'w', "uuid",          snprint_multipath_uuid},
+	{'d', "sysfs",         snprint_sysfs},
+	{'F', "failback",      snprint_failback},
+	{'Q', "queueing",      snprint_queueing},
+	{'N', "paths",         snprint_nb_paths},
+	{'r', "write_prot",    snprint_ro},
+	{'t', "dm-st",         snprint_dm_map_state},
+	{'S', "size",          snprint_multipath_size},
+	{'f', "features",      snprint_features},
+	{'x', "failures",      snprint_map_failures},
+	{'h', "hwhandler",     snprint_hwhandler},
+	{'A', "action",        snprint_action},
+	{'0', "path_faults",   snprint_path_faults},
+	{'1', "switch_grp",    snprint_switch_grp},
+	{'2', "map_loads",     snprint_map_loads},
+	{'3', "total_q_time",  snprint_total_q_time},
+	{'4', "q_timeouts",    snprint_q_timeouts},
+	{'s', "vend/prod/rev", snprint_multipath_vpr},
+	{'v', "vend",          snprint_multipath_vend},
+	{'p', "prod",          snprint_multipath_prod},
+	{'e', "rev",           snprint_multipath_rev},
+	{'G', "foreign",       snprint_multipath_foreign},
+	{'g', "vpd page data", snprint_multipath_vpd_data},
 };
 
-static struct path_data pd[] = {
-	{'w', "uuid",          0, snprint_path_uuid},
-	{'i', "hcil",          0, snprint_hcil},
-	{'d', "dev",           0, snprint_dev},
-	{'D', "dev_t",         0, snprint_dev_t},
-	{'t', "dm_st",         0, snprint_dm_path_state},
-	{'o', "dev_st",        0, snprint_offline},
-	{'T', "chk_st",        0, snprint_chk_state},
-	{'s', "vend/prod/rev", 0, snprint_vpr},
-	{'c', "checker",       0, snprint_path_checker},
-	{'C', "next_check",    0, snprint_next_check},
-	{'p', "pri",           0, snprint_pri},
-	{'S', "size",          0, snprint_path_size},
-	{'z', "serial",        0, snprint_path_serial},
-	{'M', "marginal_st",   0, snprint_path_marginal},
-	{'m', "multipath",     0, snprint_path_mpp},
-	{'N', "host WWNN",     0, snprint_host_wwnn},
-	{'n', "target WWNN",   0, snprint_tgt_wwnn},
-	{'R', "host WWPN",     0, snprint_host_wwpn},
-	{'r', "target WWPN",   0, snprint_tgt_wwpn},
-	{'a', "host adapter",  0, snprint_host_adapter},
-	{'G', "foreign",       0, snprint_path_foreign},
-	{'g', "vpd page data", 0, snprint_path_vpd_data},
-	{'0', "failures",      0, snprint_path_failures},
-	{'P', "protocol",      0, snprint_path_protocol},
-	{'I', "init_st",       0, snprint_initialized},
+static const struct path_data pd[] = {
+	{'w', "uuid",          snprint_path_uuid},
+	{'i', "hcil",          snprint_hcil},
+	{'d', "dev",           snprint_dev},
+	{'D', "dev_t",         snprint_dev_t},
+	{'t', "dm_st",         snprint_dm_path_state},
+	{'o', "dev_st",        snprint_offline},
+	{'T', "chk_st",        snprint_chk_state},
+	{'s', "vend/prod/rev", snprint_vpr},
+	{'c', "checker",       snprint_path_checker},
+	{'C', "next_check",    snprint_next_check},
+	{'p', "pri",           snprint_pri},
+	{'S', "size",          snprint_path_size},
+	{'z', "serial",        snprint_path_serial},
+	{'M', "marginal_st",   snprint_path_marginal},
+	{'m', "multipath",     snprint_path_mpp},
+	{'N', "host WWNN",     snprint_host_wwnn},
+	{'n', "target WWNN",   snprint_tgt_wwnn},
+	{'R', "host WWPN",     snprint_host_wwpn},
+	{'r', "target WWPN",   snprint_tgt_wwpn},
+	{'a', "host adapter",  snprint_host_adapter},
+	{'G', "foreign",       snprint_path_foreign},
+	{'g', "vpd page data", snprint_path_vpd_data},
+	{'0', "failures",      snprint_path_failures},
+	{'P', "protocol",      snprint_path_protocol},
+	{'I', "init_st",       snprint_initialized},
 };
 
-static struct pathgroup_data pgd[] = {
-	{'s', "selector",      0, snprint_pg_selector},
-	{'p', "pri",           0, snprint_pg_pri},
-	{'t', "dm_st",         0, snprint_pg_state},
-	{'M', "marginal_st",   0, snprint_pg_marginal},
+static const struct pathgroup_data pgd[] = {
+	{'s', "selector",      snprint_pg_selector},
+	{'p', "pri",           snprint_pg_pri},
+	{'t', "dm_st",         snprint_pg_state},
+	{'M', "marginal_st",   snprint_pg_marginal},
 };
 
 int snprint_wildcards(struct strbuf *buff)
@@ -1052,7 +1049,7 @@ int snprint_multipath_header(struct strbuf *line, const char *format,
 {
 	int initial_len = get_strbuf_len(line);
 	const char *f;
-	struct multipath_data * data;
+	const struct multipath_data * data;
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
@@ -1113,7 +1110,7 @@ int snprint_path_header(struct strbuf *line, const char *format,
 {
 	int initial_len = get_strbuf_len(line);
 	const char *f;
-	struct path_data *data;
+	const struct path_data *data;
 	int rc;
 
 	for (f = strchr(format, '%'); f; f = strchr(++format, '%')) {
-- 
2.33.1


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel