Hey,

I think we can improve how we handle length and skip in both hexdump and
od.  While we're at it we can improve our documentation a bit and bring
od(1) further into alignment with POSIX.

skip is an off_t, so unless I'm misunderstanding sys/_types.h it's
always 64-bit so we should always snag it with strtoll(3).  Right
now we do that for hexdump but not for od.

I have done the strtol -> strtoll in od for the XSI offset thing, but
I'm unsure how to proceed.  What should we do if the offset overflows?
Error out?  Set skip to zero and return?

skip is used by both programs, so move its definition into hexdump.c.

For both programs, check that (a) multiplying skip by its suffix doesn't
overflow and (b) that we don't have an invalid suffix or more than one
character for the suffix.

Add finer-grained error messages when parsing both offset and length in
both programs.  The variable name is "skip" but the documentation calls
it "offset," so I'm using the latter in the error messages.

For od, POSIX.1-2008 (and earlier specs, too, it seems) says length can
be octal or hexidecimal, so snag it with strtol, not atoi, and note this
possibility in od.1.

Unless I'm misunderstanding sys/limits.h, bumping length from a long int to a
long long int gives 32-bit platforms a larger range for length without needing
to change any other code, so do that: snag it with strtoll in both programs.

In the documentation, I think the use of "otherwise" is awkward when
introducing the possibility of an octal input because it signals a sort of
default and we've already said that lengths are decimal by default.  It's
cleaner to introduce octal inputs in a new sentence.

Thoughts?

--
Scott Cheloha

Index: usr.bin/hexdump/hexdump.1
===================================================================
RCS file: /cvs/src/usr.bin/hexdump/hexdump.1,v
retrieving revision 1.26
diff -u -p -r1.26 hexdump.1
--- usr.bin/hexdump/hexdump.1   12 Sep 2015 11:59:38 -0000      1.26
+++ usr.bin/hexdump/hexdump.1   9 Mar 2018 18:36:52 -0000
@@ -96,8 +96,8 @@ With a leading
 or
 .Cm 0X ,
 .Ar length
-is interpreted as a hexadecimal number,
-otherwise, with a leading
+is interpreted as a hexadecimal number.
+With a leading
 .Cm 0 ,
 .Ar length
 is interpreted as an octal number.
@@ -118,8 +118,8 @@ With a leading
 or
 .Cm 0X ,
 .Ar offset
-is interpreted as a hexadecimal number,
-otherwise, with a leading
+is interpreted as a hexadecimal number.
+With a leading
 .Cm 0 ,
 .Ar offset
 is interpreted as an octal number.
Index: usr.bin/hexdump/hexdump.c
===================================================================
RCS file: /cvs/src/usr.bin/hexdump/hexdump.c,v
retrieving revision 1.20
diff -u -p -r1.20 hexdump.c
--- usr.bin/hexdump/hexdump.c   8 Feb 2016 21:05:51 -0000       1.20
+++ usr.bin/hexdump/hexdump.c   9 Mar 2018 18:36:52 -0000
@@ -42,9 +42,10 @@
 FS *fshead;                            /* head of format strings */
 int blocksize;                         /* data block size */
 int exitval;                           /* final exit value */
-long length = -1;                      /* max bytes to read */
+long long length = -1;                 /* max bytes to read */
 char *iobuf;                           /* stdio I/O buffer */
 size_t iobufsiz;                       /* size of stdio I/O buffer */
+off_t skip;                            /* bytes to skip */
 
 int
 main(int argc, char *argv[])
Index: usr.bin/hexdump/hexdump.h
===================================================================
RCS file: /cvs/src/usr.bin/hexdump/hexdump.h,v
retrieving revision 1.12
diff -u -p -r1.12 hexdump.h
--- usr.bin/hexdump/hexdump.h   15 Mar 2016 04:19:13 -0000      1.12
+++ usr.bin/hexdump/hexdump.h   9 Mar 2018 18:36:52 -0000
@@ -76,7 +76,7 @@ extern int odmode;                    /* od compatibility
 extern FU *endfu;                      /* format at end-of-data */
 extern int exitval;                    /* final exit value */
 extern FS *fshead;                     /* head of format strings list */
-extern long length;                    /* max bytes to read */
+extern long long length;               /* max bytes to read */
 extern off_t skip;                     /* bytes to skip */
 extern char *iobuf;                    /* stdio I/O buffer */
 extern size_t iobufsiz;                        /* size of stdio I/O buffer */
Index: usr.bin/hexdump/hexsyntax.c
===================================================================
RCS file: /cvs/src/usr.bin/hexdump/hexsyntax.c,v
retrieving revision 1.13
diff -u -p -r1.13 hexsyntax.c
--- usr.bin/hexdump/hexsyntax.c 15 Mar 2016 04:19:13 -0000      1.13
+++ usr.bin/hexdump/hexsyntax.c 9 Mar 2018 18:36:52 -0000
@@ -34,6 +34,7 @@
 
 #include <err.h>
 #include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -41,14 +42,13 @@
 
 #include "hexdump.h"
 
-off_t skip;                            /* bytes to skip */
-
 static __dead void      usage(void);
 
 void
 newsyntax(int argc, char ***argvp)
 {
-       int ch;
+       long long multiplier;
+       int ch, unknown;
        char *p, **argv;
 
        argv = *argvp;
@@ -79,9 +79,13 @@ newsyntax(int argc, char ***argvp)
                        break;
                case 'n':
                        errno = 0;
-                       if ((length = strtol(optarg, NULL, 0)) < 0 ||
-                           errno != 0)
-                               errx(1, "%s: bad length value", optarg);
+                       length = strtoll(optarg, &p, 0);
+                       if (optarg[0] == '\0' || *p != '\0')
+                               errx(1, "length is invalid: %s", optarg);
+                       if (length < 0)
+                               errx(1, "length is too small: %s", optarg);
+                       if (length == LLONG_MAX && errno == ERANGE)
+                               errx(1, "length is too large: %s", optarg);
                        break;
                case 'o':
                        add("\"%07.7_Ax\n\"");
@@ -89,20 +93,35 @@ newsyntax(int argc, char ***argvp)
                        break;
                case 's':
                        errno = 0;
-                       if ((skip = (off_t)strtoll(optarg, &p, 0)) < 0 ||
-                           errno != 0)
-                               errx(1, "%s: bad skip value", optarg);
+                       skip = strtoll(optarg, &p, 0);
+                       if (optarg[0] == '\0' || p == optarg)
+                               errx(1, "offset is invalid 1: %s", optarg);
+                       if (skip < 0)
+                               errx(1, "offset is too small: %s", optarg);
+                       if (skip == LLONG_MAX && errno == ERANGE)
+                               errx(1, "offset is too large: %s", optarg);
+                       unknown = 0;
                        switch(*p) {
+                       case '\0':
+                               multiplier = 1;
+                               break;
                        case 'b':
-                               skip *= 512;
+                               multiplier = 512;
                                break;
                        case 'k':
-                               skip *= 1024;
+                               multiplier = 1024;
                                break;
                        case 'm':
-                               skip *= 1048576;
+                               multiplier = 1048576;
                                break;
+                       default:
+                               unknown = 1;
                        }
+                       if (unknown || (p[0] != '\0' && p[1] != '\0'))
+                               errx(1, "offset has invalid unit: %s", optarg);
+                       if (skip > LLONG_MAX / multiplier)
+                               errx(1, "offset is too large: %s", optarg);
+                       skip *= multiplier;
                        break;
                case 'v':
                        vflag = ALL;
Index: usr.bin/hexdump/od.1
===================================================================
RCS file: /cvs/src/usr.bin/hexdump/od.1,v
retrieving revision 1.31
diff -u -p -r1.31 od.1
--- usr.bin/hexdump/od.1        16 Sep 2015 08:47:26 -0000      1.31
+++ usr.bin/hexdump/od.1        9 Mar 2018 18:36:52 -0000
@@ -138,8 +138,8 @@ With a leading
 or
 .Cm 0X ,
 .Ar offset
-is interpreted as a hexadecimal number,
-otherwise, with a leading
+is interpreted as a hexadecimal number.
+With a leading
 .Cm 0 ,
 .Ar offset
 is interpreted as an octal number.
@@ -181,6 +181,19 @@ Same as
 Interpret only
 .Ar length
 bytes of input.
+By default,
+.Ar length
+is interpreted as a decimal number.
+With a leading
+.Cm 0x
+or
+.Cm 0X ,
+.Ar length
+is interpreted as a hexadecimal number.
+With a leading
+.Cm 0 ,
+.Ar length
+is interpreted as an octal number.
 .It Fl O
 .Em Four-byte octal display .
 Display the input offset in octal, followed by four
Index: usr.bin/hexdump/odsyntax.c
===================================================================
RCS file: /cvs/src/usr.bin/hexdump/odsyntax.c,v
retrieving revision 1.28
diff -u -p -r1.28 odsyntax.c
--- usr.bin/hexdump/odsyntax.c  30 May 2017 05:58:44 -0000      1.28
+++ usr.bin/hexdump/odsyntax.c  9 Mar 2018 18:36:52 -0000
@@ -34,6 +34,8 @@
 
 #include <ctype.h>
 #include <err.h>
+#include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -80,8 +82,9 @@ void
 oldsyntax(int argc, char ***argvp)
 {
        static char empty[] = "", padding[] = PADDING;
-       int ch;
        char *p, **argv;
+       long long multiplier;
+       int ch, unknown;
 
 #define        TYPE_OFFSET     7
        add("\"%07.7_Ao\n\"");
@@ -150,23 +153,46 @@ oldsyntax(int argc, char ***argvp)
                        odadd("8/2 \" %6d \" \"\\n\"");
                        break;
                case 'j':
-                       if ((skip = strtol(optarg, &p, 0)) < 0)
-                               errx(1, "%s: bad skip value", optarg);
+                       errno = 0;
+                       skip = strtoll(optarg, &p, 0);
+                       if (optarg[0] == '\0' || p == optarg)
+                               errx(1, "offset is invalid: %s", optarg);
+                       if (skip < 0)
+                               errx(1, "offset is too small: %s", optarg);
+                       if (skip == LLONG_MAX && errno == ERANGE)
+                               errx(1, "offset is too large: %s", optarg);
+                       unknown = 0;
                        switch(*p) {
+                       case '\0':
+                               multiplier = 1;
+                               break;
                        case 'b':
-                               skip *= 512;
+                               multiplier = 512;
                                break;
                        case 'k':
-                               skip *= 1024;
+                               multiplier = 1024;
                                break;
                        case 'm':
-                               skip *= 1048576;
+                               multiplier = 1048576;
                                break;
+                       default:
+                               unknown = 1;
                        }
+                       if (unknown || (p[0] != '\0' && p[1] != '\0'))
+                               errx(1, "offset has invalid unit: %s", optarg);
+                       if (skip > LLONG_MAX / multiplier)
+                               errx(1, "offset is too large: %s", optarg);
+                       skip *= multiplier;
                        break;
                case 'N':
-                       if ((length = atoi(optarg)) < 0)
-                               errx(1, "%s: bad length value", optarg);
+                       errno = 0;
+                       length = strtoll(optarg, &p, 0);
+                       if (optarg[0] == '\0' || *p != '\0')
+                               errx(1, "length is invalid: %s", optarg);
+                       if (length < 0)
+                               errx(1, "length is too small: %s", optarg);
+                       if (length == LLONG_MAX && errno == ERANGE)
+                               errx(1, "length is too large: %s", optarg);
                        break;
                case 'O':
                        odadd("4/4 \"    %011o \" \"\\n\"");
@@ -363,7 +389,7 @@ odoffset(int argc, char ***argvp)
                base = 10;
        }
 
-       skip = strtol(num, &end, base ? base : 8);
+       skip = strtoll(num, &end, base ? base : 8);
 
        /* if end isn't the same as p, we got a non-octal digit */
        if (end != p) {

Reply via email to