]> git.ipfire.org Git - thirdparty/mdadm.git/commitdiff
Refactor parse_num and use it to parse optarg.
authorMateusz Grzonka <mateusz.grzonka@intel.com>
Thu, 2 Sep 2021 09:48:12 +0000 (11:48 +0200)
committerJes Sorensen <jsorensen@fb.com>
Fri, 8 Oct 2021 11:09:11 +0000 (07:09 -0400)
Use parse_num instead of atoi to parse optarg. Replace atoi by strtol.
Move inst to int conversion into manage_new. Add better error handling.

Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com>
Signed-off-by: Jes Sorensen <jsorensen@fb.com>
lib.c
managemon.c
mdadm.c
mdadm.h
super-ddf.c
super-intel.c
util.c

diff --git a/lib.c b/lib.c
index 60890b95baf888762e0a1cf9c919e1314abf1285..76d1fbb942c71d8d4193d567d753cfa3c34a4ceb 100644 (file)
--- a/lib.c
+++ b/lib.c
@@ -25,6 +25,7 @@
 #include       "mdadm.h"
 #include       "dlink.h"
 #include       <ctype.h>
+#include       <limits.h>
 
 /* This fill contains various 'library' style function.  They
  * have no dependency on anything outside this file.
@@ -534,3 +535,30 @@ void free_line(char *line)
        }
        dl_free(line);
 }
+
+/**
+ * parse_num() - Parse int from string.
+ * @dest: Pointer to destination.
+ * @num: Pointer to string that is going to be parsed.
+ *
+ * If string contains anything after a number, error code is returned.
+ * The same happens when number is bigger than INT_MAX or smaller than 0.
+ * Writes to destination only if successfully read the number.
+ *
+ * Return: 0 on success, 1 otherwise.
+ */
+int parse_num(int *dest, char *num)
+{
+       char *c = NULL;
+       long temp;
+
+       if (!num)
+               return 1;
+
+       errno = 0;
+       temp = strtol(num, &c, 10);
+       if (temp < 0 || temp > INT_MAX || *c || errno != 0 || num == c)
+               return 1;
+       *dest = temp;
+       return 0;
+}
index 200cf83e3436a9b9890097b052b4fc379fb7e334..bb7334cf1b965635dc29addb239ace5b180d7042 100644 (file)
@@ -661,18 +661,17 @@ static void manage_new(struct mdstat_ent *mdstat,
         * the monitor.
         */
 
-       struct active_array *new;
-       struct mdinfo *mdi, *di;
-       char *inst;
-       int i;
+       struct active_array *new = NULL;
+       struct mdinfo *mdi = NULL, *di;
+       int i, inst;
        int failed = 0;
        char buf[40];
 
        /* check if array is ready to be monitored */
        if (!mdstat->active || !mdstat->level)
                return;
-       if (strcmp(mdstat->level, "raid0") == 0 ||
-           strcmp(mdstat->level, "linear") == 0)
+       if (strncmp(mdstat->level, "raid0", strlen("raid0")) == 0 ||
+           strncmp(mdstat->level, "linear", strlen("linear")) == 0)
                return;
 
        mdi = sysfs_read(-1, mdstat->devnm,
@@ -691,7 +690,8 @@ static void manage_new(struct mdstat_ent *mdstat,
 
        new->container = container;
 
-       inst = to_subarray(mdstat, container->devnm);
+       if (parse_num(&inst, to_subarray(mdstat, container->devnm)) != 0)
+               goto error;
 
        new->info.array = mdi->array;
        new->info.component_size = mdi->component_size;
@@ -724,7 +724,7 @@ static void manage_new(struct mdstat_ent *mdstat,
        new->safe_mode_delay_fd = sysfs_open2(new->info.sys_name, NULL,
                                              "safe_mode_delay");
 
-       dprintf("inst: %s action: %d state: %d\n", inst,
+       dprintf("inst: %d action: %d state: %d\n", inst,
                new->action_fd, new->info.state_fd);
 
        if (mdi->safe_mode_delay >= 50)
@@ -759,15 +759,13 @@ static void manage_new(struct mdstat_ent *mdstat,
        }
 
        sysfs_free(mdi);
+       mdi = NULL;
 
        /* if everything checks out tell the metadata handler we want to
         * manage this instance
         */
        if (!aa_ready(new) || container->ss->open_new(container, new, inst) < 0) {
-               pr_err("failed to monitor %s\n",
-                       mdstat->metadata_version);
-               new->container = NULL;
-               free_aa(new);
+               goto error;
        } else {
                replace_array(container, victim, new);
                if (failed) {
@@ -775,6 +773,16 @@ static void manage_new(struct mdstat_ent *mdstat,
                        manage_member(mdstat, new);
                }
        }
+       return;
+
+error:
+       pr_err("failed to monitor %s\n", mdstat->metadata_version);
+       if (new) {
+               new->container = NULL;
+               free_aa(new);
+       }
+       if (mdi)
+               sysfs_free(mdi);
 }
 
 void manage(struct mdstat_ent *mdstat, struct supertype *container)
diff --git a/mdadm.c b/mdadm.c
index 073b72416858cd5a0daad78adf7dabfbc0182155..a31ab99c0c2f23753c43cf1aef5622eea3d57d77 100644 (file)
--- a/mdadm.c
+++ b/mdadm.c
@@ -610,8 +610,7 @@ int main(int argc, char *argv[])
                                        s.raiddisks, optarg);
                                exit(2);
                        }
-                       s.raiddisks = parse_num(optarg);
-                       if (s.raiddisks <= 0) {
+                       if (parse_num(&s.raiddisks, optarg) != 0 || s.raiddisks <= 0) {
                                pr_err("invalid number of raid devices: %s\n",
                                        optarg);
                                exit(2);
@@ -621,8 +620,7 @@ int main(int argc, char *argv[])
                case O(ASSEMBLE, Nodes):
                case O(GROW, Nodes):
                case O(CREATE, Nodes):
-                       c.nodes = parse_num(optarg);
-                       if (c.nodes < 2) {
+                       if (parse_num(&c.nodes, optarg) != 0 || c.nodes < 2) {
                                pr_err("clustered array needs two nodes at least: %s\n",
                                        optarg);
                                exit(2);
@@ -647,8 +645,7 @@ int main(int argc, char *argv[])
                                        s.level);
                                exit(2);
                        }
-                       s.sparedisks = parse_num(optarg);
-                       if (s.sparedisks < 0) {
+                       if (parse_num(&s.sparedisks, optarg) != 0 || s.sparedisks < 0) {
                                pr_err("invalid number of spare-devices: %s\n",
                                        optarg);
                                exit(2);
@@ -732,12 +729,9 @@ int main(int argc, char *argv[])
                        }
                        if (strcmp(optarg, "dev") == 0)
                                ident.super_minor = -2;
-                       else {
-                               ident.super_minor = parse_num(optarg);
-                               if (ident.super_minor < 0) {
-                                       pr_err("Bad super-minor number: %s.\n", optarg);
-                                       exit(2);
-                               }
+                       else if (parse_num(&ident.super_minor, optarg) != 0 || ident.super_minor < 0) {
+                               pr_err("Bad super-minor number: %s.\n", optarg);
+                               exit(2);
                        }
                        continue;
 
@@ -907,8 +901,8 @@ int main(int argc, char *argv[])
 
                case O(MONITOR,'r'): /* rebuild increments */
                case O(MONITOR,Increment):
-                       increments = atoi(optarg);
-                       if (increments > 99 || increments < 1) {
+                       if (parse_num(&increments, optarg) != 0
+                               || increments > 99 || increments < 1) {
                                pr_err("please specify positive integer between 1 and 99 as rebuild increments.\n");
                                exit(2);
                        }
@@ -919,15 +913,10 @@ int main(int argc, char *argv[])
                case O(BUILD,'d'): /* delay for bitmap updates */
                case O(CREATE,'d'):
                        if (c.delay)
-                               pr_err("only specify delay once. %s ignored.\n",
-                                       optarg);
-                       else {
-                               c.delay = parse_num(optarg);
-                               if (c.delay < 1) {
-                                       pr_err("invalid delay: %s\n",
-                                               optarg);
-                                       exit(2);
-                               }
+                               pr_err("only specify delay once. %s ignored.\n", optarg);
+                       else if (parse_num(&c.delay, optarg) != 0 || c.delay < 1) {
+                               pr_err("invalid delay: %s\n", optarg);
+                               exit(2);
                        }
                        continue;
                case O(MONITOR,'f'): /* daemonise */
@@ -1209,18 +1198,15 @@ int main(int argc, char *argv[])
 
                case O(GROW, WriteBehind):
                case O(BUILD, WriteBehind):
-               case O(CREATE, WriteBehind): /* write-behind mode */
+               case O(CREATE, WriteBehind):
                        s.write_behind = DEFAULT_MAX_WRITE_BEHIND;
-                       if (optarg) {
-                               s.write_behind = parse_num(optarg);
-                               if (s.write_behind < 0 ||
-                                   s.write_behind > 16383) {
-                                       pr_err("Invalid value for maximum outstanding write-behind writes: %s.\n\tMust be between 0 and 16383.\n", optarg);
-                                       exit(2);
-                               }
+                       if (parse_num(&s.write_behind, optarg) != 0 ||
+                       s.write_behind < 0 || s.write_behind > 16383) {
+                               pr_err("Invalid value for maximum outstanding write-behind writes: %s.\n\tMust be between 0 and 16383.\n",
+                                               optarg);
+                               exit(2);
                        }
                        continue;
-
                case O(INCREMENTAL, 'r'):
                case O(INCREMENTAL, RebuildMapOpt):
                        rebuild_map = 1;
diff --git a/mdadm.h b/mdadm.h
index 8f8841d870bc1a3181c1d5709299ec3dcc211e19..53ea0de66afe60f6b685f55a8f75153f31acca77 100644 (file)
--- a/mdadm.h
+++ b/mdadm.h
@@ -1077,7 +1077,7 @@ extern struct superswitch {
 
 /* for mdmon */
        int (*open_new)(struct supertype *c, struct active_array *a,
-                       char *inst);
+                       int inst);
 
        /* Tell the metadata handler the current state of the array.
         * This covers whether it is known to be consistent (no pending writes)
@@ -1488,7 +1488,7 @@ extern int parse_uuid(char *str, int uuid[4]);
 extern int is_near_layout_10(int layout);
 extern int parse_layout_10(char *layout);
 extern int parse_layout_faulty(char *layout);
-extern long parse_num(char *num);
+extern int parse_num(int *dest, 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);
index dc8e512f36d4d247c4495c00ccfdbc32cda23424..afbe3b73827616b5fd7ee7cbdc50aee0c2cf250a 100644 (file)
@@ -4057,20 +4057,19 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst,
  * We need to confirm that the array matches the metadata in 'c' so
  * that we don't corrupt any metadata.
  */
-static int ddf_open_new(struct supertype *c, struct active_array *a, char *inst)
+static int ddf_open_new(struct supertype *c, struct active_array *a, int inst)
 {
        struct ddf_super *ddf = c->sb;
-       int n = atoi(inst);
        struct mdinfo *dev;
        struct dl *dl;
        static const char faulty[] = "faulty";
 
-       if (all_ff(ddf->virt->entries[n].guid)) {
-               pr_err("subarray %d doesn't exist\n", n);
+       if (all_ff(ddf->virt->entries[inst].guid)) {
+               pr_err("subarray %d doesn't exist\n", inst);
                return -ENODEV;
        }
-       dprintf("new subarray %d, GUID: %s\n", n,
-               guid_str(ddf->virt->entries[n].guid));
+       dprintf("new subarray %d, GUID: %s\n", inst,
+               guid_str(ddf->virt->entries[inst].guid));
        for (dev = a->info.devs; dev; dev = dev->next) {
                for (dl = ddf->dlist; dl; dl = dl->next)
                        if (dl->major == dev->disk.major &&
@@ -4078,13 +4077,13 @@ static int ddf_open_new(struct supertype *c, struct active_array *a, char *inst)
                                break;
                if (!dl || dl->pdnum < 0) {
                        pr_err("device %d/%d of subarray %d not found in meta data\n",
-                               dev->disk.major, dev->disk.minor, n);
+                               dev->disk.major, dev->disk.minor, inst);
                        return -1;
                }
                if ((be16_to_cpu(ddf->phys->entries[dl->pdnum].state) &
                        (DDF_Online|DDF_Missing|DDF_Failed)) != DDF_Online) {
                        pr_err("new subarray %d contains broken device %d/%d (%02x)\n",
-                              n, dl->major, dl->minor,
+                              inst, dl->major, dl->minor,
                               be16_to_cpu(ddf->phys->entries[dl->pdnum].state));
                        if (write(dev->state_fd, faulty, sizeof(faulty)-1) !=
                            sizeof(faulty) - 1)
@@ -4092,7 +4091,7 @@ static int ddf_open_new(struct supertype *c, struct active_array *a, char *inst)
                        dev->curr_state = DS_FAULTY;
                }
        }
-       a->info.container_member = n;
+       a->info.container_member = inst;
        return 0;
 }
 
index 83ddc000e6bb16e982c12b650a06ddd9c6dd3d11..08c644091bf210859bf5426c69986dd303f459ad 100644 (file)
@@ -8263,19 +8263,19 @@ static int imsm_count_failed(struct intel_super *super, struct imsm_dev *dev,
 }
 
 static int imsm_open_new(struct supertype *c, struct active_array *a,
-                        char *inst)
+                        int inst)
 {
        struct intel_super *super = c->sb;
        struct imsm_super *mpb = super->anchor;
        struct imsm_update_prealloc_bb_mem u;
 
-       if (atoi(inst) >= mpb->num_raid_devs) {
-               pr_err("subarry index %d, out of range\n", atoi(inst));
+       if (inst >= mpb->num_raid_devs) {
+               pr_err("subarry index %d, out of range\n", inst);
                return -ENODEV;
        }
 
-       dprintf("imsm: open_new %s\n", inst);
-       a->info.container_member = atoi(inst);
+       dprintf("imsm: open_new %d\n", inst);
+       a->info.container_member = inst;
 
        u.type = update_prealloc_badblocks_mem;
        imsm_update_metadata_locally(c, &u, sizeof(u));
diff --git a/util.c b/util.c
index ea07277cea2e20f895c319030d24818515c52805..3d05d074b80ae0495db830c12a5766e502fa2f88 100644 (file)
--- a/util.c
+++ b/util.c
@@ -422,6 +422,8 @@ int parse_layout_10(char *layout)
 
 int parse_layout_faulty(char *layout)
 {
+       if (!layout)
+               return -1;
        /* Parse the layout string for 'faulty' */
        int ln = strcspn(layout, "0123456789");
        char *m = xstrdup(layout);
@@ -434,17 +436,6 @@ int parse_layout_faulty(char *layout)
        return mode | (atoi(layout+ln)<< ModeShift);
 }
 
-long parse_num(char *num)
-{
-       /* Either return a valid number, or -1 */
-       char *c;
-       long rv = strtol(num, &c, 10);
-       if (rv < 0 || *c || !num[0])
-               return -1;
-       else
-               return rv;
-}
-
 int parse_cluster_confirm_arg(char *input, char **devname, int *slot)
 {
        char *dev;