[PATCH 0/4] Few config related refactors

[PATCH 0/4] Few config related refactors

[Mariusz Tkaczyk]

Hi Jes,
These patches remove multiple inlines across code and replace them
by defines or functions. No functional changes intended. The goal
is to make this some code reusable for both config and cmdline
(mdadm.c). I next patchset I will start optimizing names verification
(extended v2 of previous patchset).

Mariusz Tkaczyk (4):
  mdadm: define DEV_MD_DIR
  mdadm: define DEV_NUM_PREF
  mdadm: define is_devname_ignore()
  mdadm: numbered names verification

 Create.c      |  7 +++----
 Detail.c      |  9 ++++-----
 Incremental.c | 10 ++++------
 Monitor.c     | 34 +++++++++++++++++++---------------
 config.c      | 43 +++++++++++++++++++++----------------------
 lib.c         |  4 ++--
 mapfile.c     | 12 ++++++------
 mdadm.c       |  5 ++---
 mdadm.h       | 21 ++++++++++++++++++++-
 mdopen.c      | 16 ++++++++--------
 super-ddf.c   |  2 +-
 super-intel.c |  2 +-
 super1.c      |  3 +--
 sysfs.c       |  2 +-
 util.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
 15 files changed, 137 insertions(+), 77 deletions(-)

-- 
2.26.2

[PATCH 1/4] mdadm: define DEV_MD_DIR

[Mariusz Tkaczyk]

It is used many times. Additionally define _LEN to avoid repeated
strlen() calls when length is needed.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 Create.c      |  7 +++----
 Detail.c      |  9 ++++-----
 Incremental.c |  4 ++--
 Monitor.c     | 32 ++++++++++++++++++--------------
 config.c      | 10 +++++-----
 lib.c         |  2 +-
 mapfile.c     | 12 ++++++------
 mdadm.h       |  8 ++++++++
 mdopen.c      |  6 +++---
 super-ddf.c   |  2 +-
 super-intel.c |  2 +-
 super1.c      |  3 +--
 sysfs.c       |  2 +-
 13 files changed, 54 insertions(+), 45 deletions(-)

diff --git a/Create.c b/Create.c
index bbe9e13d..f3a02a2e 100644
--- a/Create.c
+++ b/Create.c
@@ -1029,10 +1029,9 @@ int Create(struct supertype *st, char *mddev,
 	 * it could be in conflict with already existing device
 	 * e.g. container, array
 	 */
-	if (strncmp(chosen_name, "/dev/md/", 8) == 0 &&
-	    map_by_name(&map, chosen_name+8) != NULL) {
-		pr_err("Array name %s is in use already.\n",
-			chosen_name);
+	if (strncmp(chosen_name, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0 &&
+	    map_by_name(&map, chosen_name + DEV_MD_DIR_LEN)) {
+		pr_err("Array name %s is in use already.\n", chosen_name);
 		close(mdfd);
 		map_unlock(&map);
 		udev_unblock();
diff --git a/Detail.c b/Detail.c
index 4ef26460..206d88e3 100644
--- a/Detail.c
+++ b/Detail.c
@@ -254,10 +254,9 @@ int Detail(char *dev, struct context *c)
 			fname_from_uuid(st, info, nbuf, ':');
 			printf("MD_UUID=%s\n", nbuf + 5);
 			mp = map_by_uuid(&map, info->uuid);
-			if (mp && mp->path &&
-			    strncmp(mp->path, "/dev/md/", 8) == 0) {
+			if (mp && mp->path && strncmp(mp->path, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0) {
 				printf("MD_DEVNAME=");
-				print_escape(mp->path + 8);
+				print_escape(mp->path + DEV_MD_DIR_LEN);
 				putchar('\n');
 			}
 
@@ -273,9 +272,9 @@ int Detail(char *dev, struct context *c)
 				printf("MD_UUID=%s\n", nbuf+5);
 			}
 			if (mp && mp->path &&
-			    strncmp(mp->path, "/dev/md/", 8) == 0) {
+			    strncmp(mp->path, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0) {
 				printf("MD_DEVNAME=");
-				print_escape(mp->path+8);
+				print_escape(mp->path + DEV_MD_DIR_LEN);
 				putchar('\n');
 			}
 			map_free(map);
diff --git a/Incremental.c b/Incremental.c
index 09b94b9f..b3dc499a 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -460,8 +460,8 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
 			info.array.working_disks ++;
 
 	}
-	if (strncmp(chosen_name, "/dev/md/", 8) == 0)
-		md_devname = chosen_name+8;
+	if (strncmp(chosen_name, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0)
+		md_devname = chosen_name + DEV_MD_DIR_LEN;
 	else
 		md_devname = chosen_name;
 	if (c->export) {
diff --git a/Monitor.c b/Monitor.c
index 44918184..3273f2fb 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -36,9 +36,18 @@
 #define EVENT_NAME_MAX 32
 #define AUTOREBUILD_PID_PATH MDMON_DIR "/autorebuild.pid"
 
+/**
+ * struct state - external array or container properties.
+ * @devname: has length of %DEV_MD_DIR + device name + terminating byte
+ * @devnm: to sync with mdstat info
+ * @parent_devnm: or subarray, devnm of parent, for others, ""
+ * @subarray: for a container it is a link to first subarray, for a subarray it is a link to next
+ *	      subarray in the same container
+ * @parent: for a subarray it is a link to its container
+ */
 struct state {
-	char devname[MD_NAME_MAX + sizeof("/dev/md/")];	/* length of "/dev/md/" + device name + terminating byte*/
-	char devnm[MD_NAME_MAX];	/* to sync with mdstat info */
+	char devname[MD_NAME_MAX + sizeof(DEV_MD_DIR)];
+	char devnm[MD_NAME_MAX];
 	unsigned int utime;
 	int err;
 	char *spare_group;
@@ -49,15 +58,10 @@ struct state {
 	int devstate[MAX_DISKS];
 	dev_t devid[MAX_DISKS];
 	int percent;
-	char parent_devnm[MD_NAME_MAX]; /* For subarray, devnm of parent.
-					* For others, ""
-					*/
+	char parent_devnm[MD_NAME_MAX];
 	struct supertype *metadata;
-	struct state *subarray;/* for a container it is a link to first subarray
-				* for a subarray it is a link to next subarray
-				* in the same container */
-	struct state *parent;  /* for a subarray it is a link to its container
-				*/
+	struct state *subarray;
+	struct state *parent;
 	struct state *next;
 };
 
@@ -252,8 +256,8 @@ int Monitor(struct mddev_dev *devlist,
 				continue;
 
 			st = xcalloc(1, sizeof *st);
-			snprintf(st->devname, MD_NAME_MAX + sizeof("/dev/md/"),
-				 "/dev/md/%s", basename(mdlist->devname));
+			snprintf(st->devname, MD_NAME_MAX + sizeof(DEV_MD_DIR), DEV_MD_DIR "%s",
+				 basename(mdlist->devname));
 			st->next = statelist;
 			st->devnm[0] = 0;
 			st->percent = RESYNC_UNKNOWN;
@@ -274,7 +278,7 @@ int Monitor(struct mddev_dev *devlist,
 
 			st = xcalloc(1, sizeof *st);
 			mdlist = conf_get_ident(dv->devname);
-			snprintf(st->devname, MD_NAME_MAX + sizeof("/dev/md/"), "%s", dv->devname);
+			snprintf(st->devname, MD_NAME_MAX + sizeof(DEV_MD_DIR), "%s", dv->devname);
 			st->next = statelist;
 			st->devnm[0] = 0;
 			st->percent = RESYNC_UNKNOWN;
@@ -942,7 +946,7 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist)
 				continue;
 			}
 
-			snprintf(st->devname, MD_NAME_MAX + sizeof("/dev/md/"), "%s", name);
+			snprintf(st->devname, MD_NAME_MAX + sizeof(DEV_MD_DIR), "%s", name);
 			if ((fd = open(st->devname, O_RDONLY)) < 0 ||
 			    md_get_array_info(fd, &array) < 0) {
 				/* no such array */
diff --git a/config.c b/config.c
index eeedd0c6..59d5bfb6 100644
--- a/config.c
+++ b/config.c
@@ -405,7 +405,7 @@ void arrayline(char *line)
 			 *  or anything that doesn't start '/' or '<'
 			 */
 			if (strcasecmp(w, "<ignore>") == 0 ||
-			    strncmp(w, "/dev/md/", 8) == 0 ||
+			    strncmp(w, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0 ||
 			    (w[0] != '/' && w[0] != '<') ||
 			    (strncmp(w, "/dev/md", 7) == 0 &&
 			     is_number(w + 7)) ||
@@ -1102,13 +1102,13 @@ int devname_matches(char *name, char *match)
 	 *  mdNN with NN
 	 * then just strcmp
 	 */
-	if (strncmp(name, "/dev/md/", 8) == 0)
-		name += 8;
+	if (strncmp(name, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0)
+		name += DEV_MD_DIR_LEN;
 	else if (strncmp(name, "/dev/", 5) == 0)
 		name += 5;
 
-	if (strncmp(match, "/dev/md/", 8) == 0)
-		match += 8;
+	if (strncmp(match, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0)
+		match += DEV_MD_DIR_LEN;
 	else if (strncmp(match, "/dev/", 5) == 0)
 		match += 5;
 
diff --git a/lib.c b/lib.c
index e395b28d..65ea51e0 100644
--- a/lib.c
+++ b/lib.c
@@ -313,7 +313,7 @@ char *map_dev_preferred(int major, int minor, int create,
 
 	for (p = devlist; p; p = p->next)
 		if (p->major == major && p->minor == minor) {
-			if (strncmp(p->name, "/dev/md/",8) == 0 ||
+			if (strncmp(p->name, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0 ||
 			    (prefer && strstr(p->name, prefer))) {
 				if (preferred == NULL ||
 				    strlen(p->name) < strlen(preferred))
diff --git a/mapfile.c b/mapfile.c
index ac351768..34fea179 100644
--- a/mapfile.c
+++ b/mapfile.c
@@ -320,9 +320,9 @@ struct map_ent *map_by_name(struct map_ent **map, char *name)
 	for (mp = *map ; mp ; mp = mp->next) {
 		if (!mp->path)
 			continue;
-		if (strncmp(mp->path, "/dev/md/", 8) != 0)
+		if (strncmp(mp->path, DEV_MD_DIR, DEV_MD_DIR_LEN) != 0)
 			continue;
-		if (strcmp(mp->path+8, name) != 0)
+		if (strcmp(mp->path + DEV_MD_DIR_LEN, name) != 0)
 			continue;
 		if (!mddev_busy(mp->devnm)) {
 			mp->bad = 1;
@@ -413,7 +413,7 @@ void RebuildMap(void)
 			devid = devnm2devid(md->devnm);
 			path = map_dev(major(devid), minor(devid), 0);
 			if (path == NULL ||
-			    strncmp(path, "/dev/md/", 8) != 0) {
+			    strncmp(path, DEV_MD_DIR, DEV_MD_DIR_LEN) != 0) {
 				/* We would really like a name that provides
 				 * an MD_DEVNAME for udev.
 				 * The name needs to be unique both in /dev/md/
@@ -434,7 +434,7 @@ void RebuildMap(void)
 				if (match && match->devname && match->devname[0] == '/') {
 					path = match->devname;
 					if (path[0] != '/') {
-						strcpy(namebuf, "/dev/md/");
+						strcpy(namebuf, DEV_MD_DIR);
 						strcat(namebuf, path);
 						path = namebuf;
 					}
@@ -478,10 +478,10 @@ void RebuildMap(void)
 
 					while (conflict) {
 						if (unum >= 0)
-							sprintf(namebuf, "/dev/md/%s%s%d",
+							sprintf(namebuf, DEV_MD_DIR "%s%s%d",
 								name, sep, unum);
 						else
-							sprintf(namebuf, "/dev/md/%s",
+							sprintf(namebuf, DEV_MD_DIR "%s",
 								name);
 						unum++;
 						if (lstat(namebuf, &stb) != 0 &&
diff --git a/mdadm.h b/mdadm.h
index b9127f9a..cb644fd8 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -100,6 +100,14 @@ struct dlm_lksb {
 #define DEFAULT_BITMAP_DELAY 5
 #define DEFAULT_MAX_WRITE_BEHIND 256
 
+/* DEV_MD_DIR points to named MD devices directory.
+ * DEV_MD_DIR_LEN is a length with Null byte excluded.
+ */
+#ifndef DEV_MD_DIR
+#define DEV_MD_DIR "/dev/md/"
+#define DEV_MD_DIR_LEN (sizeof(DEV_MD_DIR) - 1)
+#endif /* DEV_MD_DIR */
+
 /* MAP_DIR should be somewhere that persists across the pivotroot
  * from early boot to late boot.
  * /run  seems to have emerged as the best standard.
diff --git a/mdopen.c b/mdopen.c
index d18c9319..ed86df60 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -188,12 +188,12 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 	parts = autof >> 3;
 	autof &= 7;
 
-	strcpy(chosen, "/dev/md/");
+	strcpy(chosen, DEV_MD_DIR);
 	cname = chosen + strlen(chosen);
 
 	if (dev) {
-		if (strncmp(dev, "/dev/md/", 8) == 0) {
-			strcpy(cname, dev+8);
+		if (strncmp(dev, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0) {
+			strcpy(cname, dev + DEV_MD_DIR_LEN);
 		} else if (strncmp(dev, "/dev/", 5) == 0) {
 			char *e = dev + strlen(dev);
 			while (e > dev && isdigit(e[-1]))
diff --git a/super-ddf.c b/super-ddf.c
index b86c6acd..7213284e 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1648,7 +1648,7 @@ static void brief_examine_subarrays_ddf(struct supertype *st, int verbose)
 		fname_from_uuid(st, &info, nbuf1, ':');
 		_ddf_array_name(namebuf, ddf, i);
 		printf("ARRAY%s%s container=%s member=%d UUID=%s\n",
-		       namebuf[0] == '\0' ? "" : " /dev/md/", namebuf,
+		       namebuf[0] == '\0' ? "" : " " DEV_MD_DIR, namebuf,
 		       nbuf+5, i, nbuf1+5);
 	}
 }
diff --git a/super-intel.c b/super-intel.c
index e155a8ae..bbbd4dc2 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2270,7 +2270,7 @@ static void brief_examine_subarrays_imsm(struct supertype *st, int verbose)
 		super->current_vol = i;
 		getinfo_super_imsm(st, &info, NULL);
 		fname_from_uuid(st, &info, nbuf1, ':');
-		printf("ARRAY /dev/md/%.16s container=%s member=%d UUID=%s\n",
+		printf("ARRAY " DEV_MD_DIR "%.16s container=%s member=%d UUID=%s\n",
 		       dev->volume, nbuf + 5, i, nbuf1 + 5);
 	}
 }
diff --git a/super1.c b/super1.c
index f7020320..d0907370 100644
--- a/super1.c
+++ b/super1.c
@@ -642,8 +642,7 @@ static void brief_examine_super1(struct supertype *st, int verbose)
 
 	printf("ARRAY ");
 	if (nm) {
-		printf("/dev/md/");
-		print_escape(nm);
+		printf(DEV_MD_DIR "%s", nm);
 		putchar(' ');
 	}
 	if (verbose && c)
diff --git a/sysfs.c b/sysfs.c
index ca1d888f..94d02f53 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -1114,7 +1114,7 @@ void sysfsline(char *line)
 		if (strncasecmp(w, "name=", 5) == 0) {
 			char *devname = w + 5;
 
-			if (strncmp(devname, "/dev/md/", 8) == 0) {
+			if (strncmp(devname, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0) {
 				if (sr->devname)
 					pr_err("Only give one device per SYSFS line: %s\n",
 						devname);
-- 
2.26.2

[PATCH 2/4] mdadm: define DEV_NUM_PREF

[Mariusz Tkaczyk]

Use define instead of inlines. Add _LEN define.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 config.c |  4 ++--
 mdadm.h  |  8 ++++++++
 mdopen.c | 10 +++++-----
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/config.c b/config.c
index 59d5bfb6..f44cc1d3 100644
--- a/config.c
+++ b/config.c
@@ -407,8 +407,8 @@ void arrayline(char *line)
 			if (strcasecmp(w, "<ignore>") == 0 ||
 			    strncmp(w, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0 ||
 			    (w[0] != '/' && w[0] != '<') ||
-			    (strncmp(w, "/dev/md", 7) == 0 &&
-			     is_number(w + 7)) ||
+			    (strncmp(w, DEV_NUM_PREF, DEV_NUM_PREF_LEN) == 0 &&
+			     is_number(w + DEV_NUM_PREF_LEN)) ||
 			    (strncmp(w, "/dev/md_d", 9) == 0 &&
 			     is_number(w + 9))) {
 				/* This is acceptable */;
diff --git a/mdadm.h b/mdadm.h
index cb644fd8..63ec88a0 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -100,6 +100,14 @@ struct dlm_lksb {
 #define DEFAULT_BITMAP_DELAY 5
 #define DEFAULT_MAX_WRITE_BEHIND 256
 
+/* DEV_NUM_PREF is a subpath to numbered MD devices, e.g. /dev/md1 or directory name.
+ * DEV_NUM_PREF_LEN is a length with Null byte excluded.
+ */
+#ifndef DEV_NUM_PREF
+#define DEV_NUM_PREF "/dev/md"
+#define DEV_NUM_PREF_LEN (sizeof(DEV_NUM_PREF) - 1)
+#endif /* DEV_NUM_PREF */
+
 /* DEV_MD_DIR points to named MD devices directory.
  * DEV_MD_DIR_LEN is a length with Null byte excluded.
  */
diff --git a/mdopen.c b/mdopen.c
index ed86df60..b270fdd5 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -411,11 +411,11 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
 			make_parts(devname, parts);
 
 		if (strcmp(chosen, devname) != 0) {
-			if (mkdir("/dev/md",0700) == 0) {
-				if (chown("/dev/md", ci->uid, ci->gid))
-					perror("chown /dev/md");
-				if (chmod("/dev/md", ci->mode| ((ci->mode>>2) & 0111)))
-					perror("chmod /dev/md");
+			if (mkdir(DEV_NUM_PREF, 0700) == 0) {
+				if (chown(DEV_NUM_PREF, ci->uid, ci->gid))
+					perror("chown " DEV_NUM_PREF);
+				if (chmod(DEV_NUM_PREF, ci->mode | ((ci->mode >> 2) & 0111)))
+					perror("chmod " DEV_NUM_PREF);
 			}
 
 			if (dev && strcmp(chosen, dev) == 0)
-- 
2.26.2

[PATCH 3/4] mdadm: define is_devname_ignore()

[Mariusz Tkaczyk]

Use function instead of direct checks across code.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 Incremental.c |  6 ++----
 Monitor.c     |  2 +-
 config.c      | 16 ++++++++++++++--
 mdadm.c       |  5 ++---
 mdadm.h       |  1 +
 5 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/Incremental.c b/Incremental.c
index b3dc499a..ebbbd4db 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -202,8 +202,7 @@ int Incremental(struct mddev_dev *devlist, struct context *c,
 	if (!match && rv == 2)
 		goto out;
 
-	if (match && match->devname &&
-	    strcasecmp(match->devname, "<ignore>") == 0) {
+	if (match && match->devname && is_devname_ignore(match->devname) == true) {
 		if (c->verbose >= 0)
 			pr_err("array containing %s is explicitly ignored by mdadm.conf\n",
 				devname);
@@ -1564,8 +1563,7 @@ static int Incremental_container(struct supertype *st, char *devname,
 				break;
 			}
 
-			if (match && match->devname &&
-			    strcasecmp(match->devname, "<ignore>") == 0) {
+			if (match && match->devname && is_devname_ignore(match->devname) == true) {
 				if (c->verbose > 0)
 					pr_err("array %s/%s is explicitly ignored by mdadm.conf\n",
 					       match->container, match->member);
diff --git a/Monitor.c b/Monitor.c
index 3273f2fb..ce8d5802 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -250,7 +250,7 @@ int Monitor(struct mddev_dev *devlist,
 
 			if (mdlist->devname == NULL)
 				continue;
-			if (strcasecmp(mdlist->devname, "<ignore>") == 0)
+			if (is_devname_ignore(mdlist->devname) == true)
 				continue;
 			if (!is_mddev(mdlist->devname))
 				continue;
diff --git a/config.c b/config.c
index f44cc1d3..e61c0496 100644
--- a/config.c
+++ b/config.c
@@ -119,6 +119,18 @@ int match_keyword(char *word)
 	return -1;
 }
 
+/**
+ * is_devname_ignore() - check if &devname is a special "<ignore>" keyword.
+ */
+bool is_devname_ignore(char *devname)
+{
+	static const char keyword[] = "<ignore>";
+
+	if (strcasecmp(devname, keyword) == 0)
+		return true;
+	return false;
+}
+
 /**
  * ident_init() - Set defaults.
  * @ident: ident pointer, not NULL.
@@ -404,7 +416,7 @@ void arrayline(char *line)
 			 *  <ignore>
 			 *  or anything that doesn't start '/' or '<'
 			 */
-			if (strcasecmp(w, "<ignore>") == 0 ||
+			if (is_devname_ignore(w) == true ||
 			    strncmp(w, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0 ||
 			    (w[0] != '/' && w[0] != '<') ||
 			    (strncmp(w, DEV_NUM_PREF, DEV_NUM_PREF_LEN) == 0 &&
@@ -571,7 +583,7 @@ void homehostline(char *line)
 	char *w;
 
 	for (w = dl_next(line); w != line; w = dl_next(w)) {
-		if (strcasecmp(w, "<ignore>") == 0)
+		if (is_devname_ignore(w) == true)
 			require_homehost = 0;
 		else if (home_host == NULL) {
 			if (strcasecmp(w, "<none>") == 0)
diff --git a/mdadm.c b/mdadm.c
index 4685ad6b..cae37177 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -154,7 +154,7 @@ int main(int argc, char *argv[])
 			continue;
 
 		case HomeHost:
-			if (strcasecmp(optarg, "<ignore>") == 0)
+			if (is_devname_ignore(optarg) == true)
 				c.require_homehost = 0;
 			else
 				c.homehost = optarg;
@@ -1749,8 +1749,7 @@ static int scan_assemble(struct supertype *ss,
 			int r;
 			if (a->assembled)
 				continue;
-			if (a->devname &&
-			    strcasecmp(a->devname, "<ignore>") == 0)
+			if (a->devname && is_devname_ignore(a->devname) == true)
 				continue;
 
 			r = Assemble(ss, a->devname,
diff --git a/mdadm.h b/mdadm.h
index 63ec88a0..87fd4a57 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1648,6 +1648,7 @@ extern void print_escape(char *str);
 extern int use_udev(void);
 extern unsigned long GCD(unsigned long a, unsigned long b);
 extern int conf_name_is_free(char *name);
+extern bool is_devname_ignore(char *devname);
 extern int conf_verify_devnames(struct mddev_ident *array_list);
 extern int devname_matches(char *name, char *match);
 extern struct mddev_ident *conf_match(struct supertype *st,
-- 
2.26.2

[PATCH 4/4] mdadm: numbered names verification

[Mariusz Tkaczyk]

New functions added to remove literals and make the code reusable.
Use parse_num() instead of is_numer().

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
 config.c | 17 ++---------------
 lib.c    |  2 +-
 mdadm.h  |  4 +++-
 util.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/config.c b/config.c
index e61c0496..450880e3 100644
--- a/config.c
+++ b/config.c
@@ -385,17 +385,6 @@ void devline(char *line)
 struct mddev_ident *mddevlist = NULL;
 struct mddev_ident **mddevlp = &mddevlist;
 
-static int is_number(char *w)
-{
-	/* check if there are 1 or more digits and nothing else */
-	int digits = 0;
-	while (*w && isdigit(*w)) {
-		digits++;
-		w++;
-	}
-	return (digits && ! *w);
-}
-
 void arrayline(char *line)
 {
 	char *w;
@@ -419,10 +408,8 @@ void arrayline(char *line)
 			if (is_devname_ignore(w) == true ||
 			    strncmp(w, DEV_MD_DIR, DEV_MD_DIR_LEN) == 0 ||
 			    (w[0] != '/' && w[0] != '<') ||
-			    (strncmp(w, DEV_NUM_PREF, DEV_NUM_PREF_LEN) == 0 &&
-			     is_number(w + DEV_NUM_PREF_LEN)) ||
-			    (strncmp(w, "/dev/md_d", 9) == 0 &&
-			     is_number(w + 9))) {
+			    is_devname_md_numbered(w) == true ||
+			    is_devname_md_d_numbered(w) == true) {
 				/* This is acceptable */;
 				if (mis.devname)
 					pr_err("only give one device per ARRAY line: %s and %s\n",
diff --git a/lib.c b/lib.c
index 65ea51e0..fe5c8d2c 100644
--- a/lib.c
+++ b/lib.c
@@ -570,7 +570,7 @@ void free_line(char *line)
  *
  * Return: 0 on success, 1 otherwise.
  */
-int parse_num(int *dest, char *num)
+int parse_num(int *dest, const char *num)
 {
 	char *c = NULL;
 	long temp;
diff --git a/mdadm.h b/mdadm.h
index 87fd4a57..b5d58448 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1599,7 +1599,7 @@ int default_layout(struct supertype *st, int level, int verbose);
 extern int is_near_layout_10(int layout);
 extern int parse_layout_10(char *layout);
 extern int parse_layout_faulty(char *layout);
-extern int parse_num(int *dest, char *num);
+extern int parse_num(int *dest, const char *num);
 extern int parse_cluster_confirm_arg(char *inp, char **devname, int *slot);
 extern int check_ext2(int fd, char *name);
 extern int check_reiser(int fd, char *name);
@@ -1649,6 +1649,8 @@ extern int use_udev(void);
 extern unsigned long GCD(unsigned long a, unsigned long b);
 extern int conf_name_is_free(char *name);
 extern bool is_devname_ignore(char *devname);
+extern bool is_devname_md_numbered(const char *devname);
+extern bool is_devname_md_d_numbered(const char *devname);
 extern int conf_verify_devnames(struct mddev_ident *array_list);
 extern int devname_matches(char *name, char *match);
 extern struct mddev_ident *conf_match(struct supertype *st,
diff --git a/util.c b/util.c
index 9f1e1f7c..23372b7e 100644
--- a/util.c
+++ b/util.c
@@ -973,6 +973,50 @@ dev_t devnm2devid(char *devnm)
 	return 0;
 }
 
+/**
+ * is_devname_numbered() - helper for numbered devname verification.
+ * @devname: path or name to check.
+ * @pref: expected devname prefix.
+ * @pref_len: prefix len.
+ */
+static bool is_devname_numbered(const char *devname, const char *pref, const int pref_len)
+{
+	int val;
+
+	assert(devname && pref);
+
+	if (strncmp(devname, pref, pref_len) != 0)
+		return false;
+
+	if (parse_num(&val, devname + pref_len) != 0)
+		return false;
+
+	if (val > 127)
+		return false;
+
+	return true;
+}
+
+/**
+ * is_devname_md_numbered() - check if &devname is numbered MD device (md).
+ * @devname: path or name to check.
+ */
+bool is_devname_md_numbered(const char *devname)
+{
+	return is_devname_numbered(devname, DEV_NUM_PREF, DEV_NUM_PREF_LEN);
+}
+
+/**
+ * is_devname_md_d_numbered() - check if &devname is secondary numbered MD device (md_d).
+ * @devname: path or name to check.
+ */
+bool is_devname_md_d_numbered(const char *devname)
+{
+	static const char d_dev[] = DEV_NUM_PREF "_d";
+
+	return is_devname_numbered(devname, d_dev, sizeof(d_dev) - 1);
+}
+
 /**
  * get_md_name() - Get main dev node of the md device.
  * @devnm: Md device name or path.
-- 
2.26.2

Re: [PATCH 0/4] Few config related refactors

[Xiao Ni]

On Fri, Mar 24, 2023 at 12:50 AM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> wrote:
>
> Hi Jes,
> These patches remove multiple inlines across code and replace them
> by defines or functions. No functional changes intended. The goal
> is to make this some code reusable for both config and cmdline
> (mdadm.c). I next patchset I will start optimizing names verification
> (extended v2 of previous patchset).
>
> Mariusz Tkaczyk (4):
>   mdadm: define DEV_MD_DIR
>   mdadm: define DEV_NUM_PREF
>   mdadm: define is_devname_ignore()
>   mdadm: numbered names verification
>
>  Create.c      |  7 +++----
>  Detail.c      |  9 ++++-----
>  Incremental.c | 10 ++++------
>  Monitor.c     | 34 +++++++++++++++++++---------------
>  config.c      | 43 +++++++++++++++++++++----------------------
>  lib.c         |  4 ++--
>  mapfile.c     | 12 ++++++------
>  mdadm.c       |  5 ++---
>  mdadm.h       | 21 ++++++++++++++++++++-
>  mdopen.c      | 16 ++++++++--------
>  super-ddf.c   |  2 +-
>  super-intel.c |  2 +-
>  super1.c      |  3 +--
>  sysfs.c       |  2 +-
>  util.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  15 files changed, 137 insertions(+), 77 deletions(-)
>
> --
> 2.26.2
>

Acked-by: Xiao Ni <xni@redhat.com>

Re: [PATCH 0/4] Few config related refactors

[Mariusz Tkaczyk]

On Fri, 24 Mar 2023 10:13:04 +0800
Xiao Ni <xni@redhat.com> wrote:

> On Fri, Mar 24, 2023 at 12:50 AM Mariusz Tkaczyk
> <mariusz.tkaczyk@linux.intel.com> wrote:
> >
> > Hi Jes,
> > These patches remove multiple inlines across code and replace them
> > by defines or functions. No functional changes intended. The goal
> > is to make this some code reusable for both config and cmdline
> > (mdadm.c). I next patchset I will start optimizing names verification
> > (extended v2 of previous patchset).
> >
> > Mariusz Tkaczyk (4):
> >   mdadm: define DEV_MD_DIR
> >   mdadm: define DEV_NUM_PREF
> >   mdadm: define is_devname_ignore()
> >   mdadm: numbered names verification
> >
> >  Create.c      |  7 +++----
> >  Detail.c      |  9 ++++-----
> >  Incremental.c | 10 ++++------
> >  Monitor.c     | 34 +++++++++++++++++++---------------
> >  config.c      | 43 +++++++++++++++++++++----------------------
> >  lib.c         |  4 ++--
> >  mapfile.c     | 12 ++++++------
> >  mdadm.c       |  5 ++---
> >  mdadm.h       | 21 ++++++++++++++++++++-
> >  mdopen.c      | 16 ++++++++--------
> >  super-ddf.c   |  2 +-
> >  super-intel.c |  2 +-
> >  super1.c      |  3 +--
> >  sysfs.c       |  2 +-
> >  util.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  15 files changed, 137 insertions(+), 77 deletions(-)
> >
> > --
> > 2.26.2
> >  
> 
> Acked-by: Xiao Ni <xni@redhat.com>
> 

Hi Jes,
Could you please take those patches?
We are working on changes in other areas and the error enum will be useful.

Thanks,
Mariusz

Re: [PATCH 0/4] Few config related refactors

[Jes Sorensen]

On 3/23/23 12:50, Mariusz Tkaczyk wrote:
> Hi Jes,
> These patches remove multiple inlines across code and replace them
> by defines or functions. No functional changes intended. The goal
> is to make this some code reusable for both config and cmdline
> (mdadm.c). I next patchset I will start optimizing names verification
> (extended v2 of previous patchset).

Applied!

I'll push the later, I left my key at home.

Thanks,
Jes