On Sat, 16 Nov 2002, Bruce Evans wrote:

> On Sat, 16 Nov 2002, Julian Elischer wrote:
> 
> > On Fri, 15 Nov 2002, Nate Lawson wrote:
> > > In the overflow case, strtoul returns ULONG_MAX.  Or if you're interested
> > > in catching invalid characters, use endptr.
> >
> > I'm not that interested in catching those cases.. they were not caught
> > before and I'm looking for the minimal diff..
> >
> > I've just removed the clause. See new patch attached..
> 
> Removing clauses gives maximal diffs and loses even some of the sub-minimal
> bounds checking (values less than 0 were just errors in some cases, but
> they are now converted to large unsigned values and not checked at all).

but that make sthem the same as the "almost as large" bad values that
were not checked before either.. The onlything that the code removed
might serve would be to restrict times (in mSecs) to something like
"less than 10 seconds" or something, but for exaple on some forms of
floppy tape, it took almost that long to change "tracks" so it is
hard to try predict what a legal value is.. (probably less than 1 day
:-)

removing the clause for testing < 0 (as it can no longer happen)
seems easiest. A stupid value in the disklabel will stand out but will
probably be ignored anyhow.

> 
> > A sensible thing might be to compare against a "sensible" value
> > but who knows what that value should be?
> 
> Values should only be restricted to avoid ones that can't possibly work.
> This is mostly already done.


> 
> > I include the new diff for your coments
> 
> I'll wait for a later one with more of the suggested changes incorporated
> (strtoul() should use base 10, and most things shouldn't be changed, etc.).

does it matter? I have had a few times when I'd have liked to be able to
put a number in in hex. To be exactly compatible in NOT handling
the 0x and leading '0' cases I suppose so..

Ok here's  a diff 

> 
> Bruce
> 
> 
> To Unsubscribe: send mail to [EMAIL PROTECTED]
> with "unsubscribe freebsd-current" in the body of the message
> 

Index: disklabel.c
===================================================================
RCS file: /usr/cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.62
diff -u -r1.62 disklabel.c
--- disklabel.c 8 Oct 2002 12:13:19 -0000       1.62
+++ disklabel.c 16 Nov 2002 22:18:27 -0000
@@ -732,7 +732,7 @@
        const struct partition *pp;
 
        fprintf(f, "# %s:\n", specname);
-       if ((unsigned) lp->d_type < DKMAXTYPES)
+       if (lp->d_type < DKMAXTYPES)
                fprintf(f, "type: %s\n", dktypenames[lp->d_type]);
        else
                fprintf(f, "type: %u\n", lp->d_type);
@@ -778,7 +778,7 @@
                if (pp->p_size) {
                        fprintf(f, "  %c: %8lu %8lu  ", 'a' + i,
                           (u_long)pp->p_size, (u_long)pp->p_offset);
-                       if ((unsigned) pp->p_fstype < FSMAXTYPES)
+                       if (pp->p_fstype < FSMAXTYPES)
                                fprintf(f, "%8.8s", fstypenames[pp->p_fstype]);
                        else
                                fprintf(f, "%8d", pp->p_fstype);
@@ -941,9 +941,10 @@
 {
        char *cp;
        const char **cpp;
-       unsigned int part;
+       u_int part;
        char *tp, line[BUFSIZ];
-       int v, lineno = 0, errors = 0;
+       u_int v;
+       int lineno = 0, errors = 0;
        int i;
 
        lp->d_bbsize = BBSIZE;                          /* XXX */
@@ -973,8 +974,8 @@
                                }
                        if (cpp < &dktypenames[DKMAXTYPES])
                                continue;
-                       v = atoi(tp);
-                       if ((unsigned)v >= DKMAXTYPES)
+                       v = strtoul(tp, NULL, 10);
+                       if (v >= DKMAXTYPES)
                                fprintf(stderr, "line %d:%s %d\n", lineno,
                                    "Warning, unknown disk type", v);
                        lp->d_type = v;
@@ -1001,13 +1002,13 @@
                }
                if (streq(cp, "drivedata")) {
                        for (i = 0; (cp = tp) && *cp != '\0' && i < NDDATA;) {
-                               lp->d_drivedata[i++] = atoi(cp);
+                               lp->d_drivedata[i++] = strtoul(cp, NULL, 10);
                                tp = word(cp);
                        }
                        continue;
                }
-               if (sscanf(cp, "%d partitions", &v) == 1) {
-                       if (v == 0 || (unsigned)v > MAXPARTITIONS) {
+               if (sscanf(cp, "%u partitions", &v) == 1) {
+                       if (v == 0 || v > MAXPARTITIONS) {
                                fprintf(stderr,
                                    "line %d: bad # of partitions\n", lineno);
                                lp->d_npartitions = MAXPARTITIONS;
@@ -1027,8 +1028,8 @@
                        continue;
                }
                if (streq(cp, "bytes/sector")) {
-                       v = atoi(tp);
-                       if (v <= 0 || (v % DEV_BSIZE) != 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0 || (v % DEV_BSIZE) != 0) {
                                fprintf(stderr,
                                    "line %d: %s: bad sector size\n",
                                    lineno, tp);
@@ -1038,8 +1039,8 @@
                        continue;
                }
                if (streq(cp, "sectors/track")) {
-                       v = atoi(tp);
-                       if (v <= 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1048,8 +1049,8 @@
                        continue;
                }
                if (streq(cp, "sectors/cylinder")) {
-                       v = atoi(tp);
-                       if (v <= 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1058,8 +1059,8 @@
                        continue;
                }
                if (streq(cp, "tracks/cylinder")) {
-                       v = atoi(tp);
-                       if (v <= 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1068,8 +1069,8 @@
                        continue;
                }
                if (streq(cp, "cylinders")) {
-                       v = atoi(tp);
-                       if (v <= 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1078,8 +1079,8 @@
                        continue;
                }
                if (streq(cp, "sectors/unit")) {
-                       v = atoi(tp);
-                       if (v <= 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1088,8 +1089,8 @@
                        continue;
                }
                if (streq(cp, "rpm")) {
-                       v = atoi(tp);
-                       if (v <= 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0 || v > USHRT_MAX) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1098,8 +1099,8 @@
                        continue;
                }
                if (streq(cp, "interleave")) {
-                       v = atoi(tp);
-                       if (v <= 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v == 0 || v > USHRT_MAX) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1108,8 +1109,8 @@
                        continue;
                }
                if (streq(cp, "trackskew")) {
-                       v = atoi(tp);
-                       if (v < 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v > USHRT_MAX) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1118,8 +1119,8 @@
                        continue;
                }
                if (streq(cp, "cylinderskew")) {
-                       v = atoi(tp);
-                       if (v < 0) {
+                       v = strtoul(tp, NULL, 10);
+                       if (v > USHRT_MAX) {
                                fprintf(stderr, "line %d: %s: bad %s\n",
                                    lineno, tp, cp);
                                errors++;
@@ -1128,23 +1129,13 @@
                        continue;
                }
                if (streq(cp, "headswitch")) {
-                       v = atoi(tp);
-                       if (v < 0) {
-                               fprintf(stderr, "line %d: %s: bad %s\n",
-                                   lineno, tp, cp);
-                               errors++;
-                       } else
-                               lp->d_headswitch = v;
+                       v = strtoul(tp, NULL, 10);
+                       lp->d_headswitch = v;
                        continue;
                }
                if (streq(cp, "track-to-track seek")) {
-                       v = atoi(tp);
-                       if (v < 0) {
-                               fprintf(stderr, "line %d: %s: bad %s\n",
-                                   lineno, tp, cp);
-                               errors++;
-                       } else
-                               lp->d_trkseek = v;
+                       v = strtoul(tp, NULL, 10);
+                       lp->d_trkseek = v;
                        continue;
                }
                /* the ':' was removed above */
@@ -1182,7 +1173,7 @@
                return (1); \
        } else { \
                cp = tp, tp = word(cp); \
-               (n) = atoi(cp); \
+               (n) = strtoul(cp, NULL, 10); \
        } \
 } while (0)
 
@@ -1194,7 +1185,7 @@
        } else { \
                char *tmp; \
                cp = tp, tp = word(cp); \
-               (n) = strtol(cp,&tmp,10); \
+               (n) = strtoul(cp, &tmp, 10); \
                if (tmp) (w) = *tmp; \
        } \
 } while (0)
@@ -1209,26 +1200,26 @@
        struct partition *pp;
        char *cp;
        const char **cpp;
-       int v;
+       u_int v;
 
        pp = &lp->d_partitions[part];
        cp = NULL;
 
        v = 0;
        NXTWORD(part_size_type[part],v);
-       if (v < 0 || (v == 0 && part_size_type[part] != '*')) {
-               fprintf(stderr, "line %d: %s: bad partition size\n", lineno,
-                   cp);
+       if (v == 0 && part_size_type[part] != '*') {
+               fprintf(stderr,
+                   "line %d: %s: bad partition size\n", lineno, cp);
                return (1);
        }
        pp->p_size = v;
 
        v = 0;
        NXTWORD(part_offset_type[part],v);
-       if (v < 0 || (v == 0 && part_offset_type[part] != '*' &&
-           part_offset_type[part] != '\0')) {
-               fprintf(stderr, "line %d: %s: bad partition offset\n", lineno,
-                   cp);
+       if (v == 0 && part_offset_type[part] != '*' &&
+           part_offset_type[part] != '\0') {
+               fprintf(stderr,
+                   "line %d: %s: bad partition offset\n", lineno, cp);
                return (1);
        }
        pp->p_offset = v;
@@ -1240,10 +1231,10 @@
                pp->p_fstype = cpp - fstypenames;
        } else {
                if (isdigit(*cp))
-                       v = atoi(cp);
+                       v = strtoul(cp, NULL, 10);
                else
                        v = FSMAXTYPES;
-               if ((unsigned)v >= FSMAXTYPES) {
+               if (v >= FSMAXTYPES) {
                        fprintf(stderr,
                            "line %d: Warning, unknown file system type %s\n",
                            lineno, cp);
@@ -1314,7 +1305,7 @@
        struct partition *pp;
        int i, errors = 0;
        char part;
-       unsigned long total_size, total_percent, current_offset;
+       u_long total_size, total_percent, current_offset;
        int seen_default_offset;
        int hog_part;
        int j;

Reply via email to