[src/trunk]: src/sbin/raidctl Several more cleanups:

[kre <kre%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/5a0e734b3ea4
branches:  trunk
changeset: 357694:5a0e734b3ea4
user:      kre <kre%NetBSD.org@localhost>
date:      Wed Nov 22 00:31:31 2017 +0000

description:
Several more cleanups:
1. Don't force use of "for" when "while" works better.
2. No need to check c != '\0' when we also check (c == ' ' || c == '\t')
3. Use the size of the buffer we're using, rather than a different one
   (not really a concern, they're the same size)
4. Don't use fscanf() to read file data, use fgets() & sscanf().
5. After using a pointer as a char *, validate alignment before switching
   to int * (can only fail if kernel #define gets set stupidly)   Or #6...
6. Validate sparemap file name isn't too long for assigned space.
7. recognise that strlen() returns size_t - don't shove it into an int.
8. On out of mem, be more clear which allocation failed in warning msg.

ATF tests all pass.   But I don't think they use sparemap files.

diffstat:

 sbin/raidctl/rf_configure.c |  54 ++++++++++++++++++++++++++++++--------------
 1 files changed, 37 insertions(+), 17 deletions(-)

diffs (144 lines):

diff -r 44d815847d4f -r 5a0e734b3ea4 sbin/raidctl/rf_configure.c
--- a/sbin/raidctl/rf_configure.c       Tue Nov 21 16:31:37 2017 +0000
+++ b/sbin/raidctl/rf_configure.c       Wed Nov 22 00:31:31 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: rf_configure.c,v 1.31 2017/11/21 16:31:37 christos Exp $ */
+/*     $NetBSD: rf_configure.c,v 1.32 2017/11/22 00:31:31 kre Exp $ */
 
 /*
  * Copyright (c) 1995 Carnegie-Mellon University.
@@ -49,7 +49,7 @@
 #include <sys/cdefs.h>
 
 #ifndef lint
-__RCSID("$NetBSD: rf_configure.c,v 1.31 2017/11/21 16:31:37 christos Exp $");
+__RCSID("$NetBSD: rf_configure.c,v 1.32 2017/11/22 00:31:31 kre Exp $");
 #endif
 
 
@@ -384,12 +384,18 @@
         * daemon that's responsible for finding the sparemaps
         */
        if (distSpare) {
-               if (rf_get_next_nonblank_line(smbuf, sizeof(buf), configfp,
+               if (rf_get_next_nonblank_line(smbuf, sizeof(smbuf), configfp,
                    "Can't find sparemap file name in config file")) {
                        fclose(fp);
                        return EINVAL;
                }
                smname = rf_find_non_white(smbuf, 1);
+               if (strlen(smname) >= RF_SPAREMAP_NAME_LEN) {
+                       warnx("sparemap file name '%s' too long (max %d)",
+                           smname, RF_SPAREMAP_NAME_LEN - 1);
+                       fclose(fp);
+                       return EINVAL;
+               }
        } else {
                smbuf[0] = '\0';
                smname = smbuf;
@@ -415,6 +421,13 @@
                *p++ = '\0';
                i++;
        }
+       if ((i & (sizeof(int) - 1)) != 0) {
+               /* panic, unaligned data; RF_SPAREMAP_NAME_LEN invalid */
+               warnx("Program Bug: (RF_SPAREMAP_NAME_LEN(%d) %% %zd) != 0",
+                   RF_SPAREMAP_NAME_LEN, sizeof(int));
+               fclose(fp);
+               return EINVAL;
+       }
 
        /*
         * fill in the buffer with the block design parameters
@@ -454,8 +467,8 @@
 static char *
 rf_find_non_white(char *p, int eatnl)
 {
-       for (; *p != '\0' && (*p == ' ' || *p == '\t'); p++)
-               continue;
+       while (*p == ' ' || *p == '\t')
+               p++;
        if (*p == '\n' && eatnl)
                *p = '\0';
        return p;
@@ -465,8 +478,8 @@
 static char *
 rf_find_white(char *p)
 {
-       for (; *p != '\0' && *p != ' ' && *p != '\t'; p++)
-               continue;
+       while (*p != '\0' && *p != ' ' && *p != '\t')
+               p++;
        return p;
 }
 
@@ -497,17 +510,15 @@
 rf_get_next_nonblank_line(char *buf, int len, FILE *fp, const char *errmsg)
 {
        char *p;
-       int l;
+       size_t l;
 
        while (fgets(buf, len, fp) != NULL) {
                p = rf_find_non_white(buf, 0);
                if (*p == '\n' || *p == '\0' || *p == '#')
                        continue;
-               l = strlen(buf) - 1;
-               while (l >= 0 && (buf[l] == ' ' || buf[l] == '\n')) {
+               l = strlen(buf);
+               while (l > 0 && (buf[--l] == ' ' || buf[l] == '\n'))
                        buf[l] = '\0';
-                       l--;
-               }
                return 0;
        }
        if (errmsg)
@@ -536,6 +547,7 @@
        char buf[BUFSIZ], targString[BUFSIZ], errString[BUFSIZ];
        RF_SpareTableEntry_t **table;
        FILE *fp = NULL;
+       size_t len;
 
        /* allocate and initialize the table */
        table = calloc(req->TablesPerSpareRegion, sizeof(*table));
@@ -546,7 +558,7 @@
        for (i = 0; i < req->TablesPerSpareRegion; i++) {
                table[i] = calloc(req->BlocksPerTable, sizeof(**table));
                if (table[i] == NULL) {
-                       warn("%s: Unable to allocate table", __func__);
+                       warn("%s: Unable to allocate table:%d", __func__, i);
                        goto out;
                }
                for (j = 0; j < req->BlocksPerTable; j++)
@@ -563,7 +575,7 @@
            "Invalid sparemap file:  can't find header line"))
                goto out;
 
-       size_t len = strlen(buf);
+       len = strlen(buf);
        if (len != 0 && buf[len - 1] == '\n')
                buf[len - 1] = '\0';
 
@@ -579,11 +591,19 @@
        /* no more blank lines or comments allowed now */
        linecount = req->TablesPerSpareRegion * req->TableDepthInPUs;
        for (i = 0; i < linecount; i++) {
-               numFound = fscanf(fp, " %d %d %d %d", &tableNum, &tupleNum,
+               char linebuf[BUFSIZ];
+
+               if (fgets(linebuf, BUFSIZ, fp) == NULL) {
+                       warnx("Sparemap file prematurely exhausted after %d "
+                           "of %d lines", i, linecount);
+                       goto out;
+               }
+               numFound = sscanf(linebuf, " %d %d %d %d", &tableNum, &tupleNum,
                    &spareDisk, &spareBlkOffset);
                if (numFound != 4) {
-                       warnx("Sparemap file prematurely exhausted after %d "
-                           "of %d lines", i, linecount);
+                       warnx("Sparemap file format error - "
+                           "line %d of %d lines",
+                           i + 1, linecount);
                        goto out;
                }