Date: Fri, 25 Aug 2017 10:17:58 -0700 From: Conrad Meyer <cem@freebsd.org> To: Alan Somers <asomers@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r322893 - head/bin/dd Message-ID: <CAG6CVpU5rsrwU4EEkF3i5gvzSZRX3Q=K%2B8M-LMwk2cGP8hMSxw@mail.gmail.com> In-Reply-To: <201708251531.v7PFVtoZ038242@repo.freebsd.org> References: <201708251531.v7PFVtoZ038242@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This change seems to break buildworld on MIPS: /home/cem/head.svn/bin/dd/args.c: In function 'f_bs': /home/cem/head.svn/bin/dd/args.c:188: warning: format '%zd' expects type 'signed size_t', but argument 3 has type 'long int' /home/cem/head.svn/bin/dd/args.c: In function 'f_cbs': /home/cem/head.svn/bin/dd/args.c:199: warning: format '%zd' expects type 'signed size_t', but argument 3 has type 'long int' /home/cem/head.svn/bin/dd/args.c: In function 'f_ibs': /home/cem/head.svn/bin/dd/args.c:245: warning: format '%zd' expects type 'signed size_t', but argument 3 has type 'long int' /home/cem/head.svn/bin/dd/args.c: In function 'f_obs': /home/cem/head.svn/bin/dd/args.c:266: warning: format '%zd' expects type 'signed size_t', but argument 3 has type 'long int' (Yes, it's odd that the SSIZE_MAX constant has 'long' type.) Best, Conrad On Fri, Aug 25, 2017 at 8:31 AM, Alan Somers <asomers@freebsd.org> wrote: > Author: asomers > Date: Fri Aug 25 15:31:55 2017 > New Revision: 322893 > URL: https://svnweb.freebsd.org/changeset/base/322893 > > Log: > dd(1): Incorrect casting of arguments > > dd(1) casts many of its numeric arguments from uintmax_t to intmax_t and > back again to detect whether or not the original arguments were negative. > This is not correct, and causes problems with boundary cases, for example > when count is SSIZE_MAX-1. > > PR: 191263 > Submitted by: will@worrbase.com > Reviewed by: pi, asomers > MFC after: 3 weeks > > Modified: > head/bin/dd/args.c > head/bin/dd/conv.c > head/bin/dd/dd.c > head/bin/dd/dd.h > head/bin/dd/position.c > > Modified: head/bin/dd/args.c > ============================================================================== > --- head/bin/dd/args.c Fri Aug 25 14:42:11 2017 (r322892) > +++ head/bin/dd/args.c Fri Aug 25 15:31:55 2017 (r322893) > @@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$"); > > #include <sys/types.h> > > +#include <ctype.h> > #include <err.h> > #include <errno.h> > #include <inttypes.h> > @@ -184,7 +185,7 @@ f_bs(char *arg) > > res = get_num(arg); > if (res < 1 || res > SSIZE_MAX) > - errx(1, "bs must be between 1 and %jd", (intmax_t)SSIZE_MAX); > + errx(1, "bs must be between 1 and %zd", SSIZE_MAX); > in.dbsz = out.dbsz = (size_t)res; > } > > @@ -195,22 +196,22 @@ f_cbs(char *arg) > > res = get_num(arg); > if (res < 1 || res > SSIZE_MAX) > - errx(1, "cbs must be between 1 and %jd", (intmax_t)SSIZE_MAX); > + errx(1, "cbs must be between 1 and %zd", SSIZE_MAX); > cbsz = (size_t)res; > } > > static void > f_count(char *arg) > { > - intmax_t res; > + uintmax_t res; > > - res = (intmax_t)get_num(arg); > - if (res < 0) > - errx(1, "count cannot be negative"); > + res = get_num(arg); > + if (res == UINTMAX_MAX) > + errc(1, ERANGE, "%s", oper); > if (res == 0) > - cpy_cnt = (uintmax_t)-1; > + cpy_cnt = UINTMAX_MAX; > else > - cpy_cnt = (uintmax_t)res; > + cpy_cnt = res; > } > > static void > @@ -219,7 +220,7 @@ f_files(char *arg) > > files_cnt = get_num(arg); > if (files_cnt < 1) > - errx(1, "files must be between 1 and %jd", (uintmax_t)-1); > + errx(1, "files must be between 1 and %zu", SIZE_MAX); > } > > static void > @@ -240,8 +241,8 @@ f_ibs(char *arg) > if (!(ddflags & C_BS)) { > res = get_num(arg); > if (res < 1 || res > SSIZE_MAX) > - errx(1, "ibs must be between 1 and %jd", > - (intmax_t)SSIZE_MAX); > + errx(1, "ibs must be between 1 and %zd", > + SSIZE_MAX); > in.dbsz = (size_t)res; > } > } > @@ -261,8 +262,8 @@ f_obs(char *arg) > if (!(ddflags & C_BS)) { > res = get_num(arg); > if (res < 1 || res > SSIZE_MAX) > - errx(1, "obs must be between 1 and %jd", > - (intmax_t)SSIZE_MAX); > + errx(1, "obs must be between 1 and %zd", > + SSIZE_MAX); > out.dbsz = (size_t)res; > } > } > > Modified: head/bin/dd/conv.c > ============================================================================== > --- head/bin/dd/conv.c Fri Aug 25 14:42:11 2017 (r322892) > +++ head/bin/dd/conv.c Fri Aug 25 15:31:55 2017 (r322893) > @@ -133,7 +133,7 @@ block(void) > */ > ch = 0; > for (inp = in.dbp - in.dbcnt, outp = out.dbp; in.dbcnt;) { > - maxlen = MIN(cbsz, in.dbcnt); > + maxlen = MIN(cbsz, (size_t)in.dbcnt); > if ((t = ctab) != NULL) > for (cnt = 0; cnt < maxlen && (ch = *inp++) != '\n'; > ++cnt) > @@ -146,7 +146,7 @@ block(void) > * Check for short record without a newline. Reassemble the > * input block. > */ > - if (ch != '\n' && in.dbcnt < cbsz) { > + if (ch != '\n' && (size_t)in.dbcnt < cbsz) { > (void)memmove(in.db, in.dbp - in.dbcnt, in.dbcnt); > break; > } > @@ -228,7 +228,7 @@ unblock(void) > * translation has to already be done or we might not recognize the > * spaces. > */ > - for (inp = in.db; in.dbcnt >= cbsz; inp += cbsz, in.dbcnt -= cbsz) { > + for (inp = in.db; (size_t)in.dbcnt >= cbsz; inp += cbsz, in.dbcnt -= cbsz) { > for (t = inp + cbsz - 1; t >= inp && *t == ' '; --t) > ; > if (t >= inp) { > > Modified: head/bin/dd/dd.c > ============================================================================== > --- head/bin/dd/dd.c Fri Aug 25 14:42:11 2017 (r322892) > +++ head/bin/dd/dd.c Fri Aug 25 15:31:55 2017 (r322893) > @@ -204,10 +204,10 @@ setup(void) > * record oriented I/O, only need a single buffer. > */ > if (!(ddflags & (C_BLOCK | C_UNBLOCK))) { > - if ((in.db = malloc(out.dbsz + in.dbsz - 1)) == NULL) > + if ((in.db = malloc((size_t)out.dbsz + in.dbsz - 1)) == NULL) > err(1, "input buffer"); > out.db = in.db; > - } else if ((in.db = malloc(MAX(in.dbsz, cbsz) + cbsz)) == NULL || > + } else if ((in.db = malloc(MAX((size_t)in.dbsz, cbsz) + cbsz)) == NULL || > (out.db = malloc(out.dbsz + cbsz)) == NULL) > err(1, "output buffer"); > > @@ -405,7 +405,7 @@ dd_in(void) > ++st.in_full; > > /* Handle full input blocks. */ > - } else if ((size_t)n == in.dbsz) { > + } else if ((size_t)n == (size_t)in.dbsz) { > in.dbcnt += in.dbrcnt = n; > ++st.in_full; > > @@ -562,7 +562,7 @@ dd_out(int force) > outp += nw; > st.bytes += nw; > > - if ((size_t)nw == n && n == out.dbsz) > + if ((size_t)nw == n && n == (size_t)out.dbsz) > ++st.out_full; > else > ++st.out_part; > > Modified: head/bin/dd/dd.h > ============================================================================== > --- head/bin/dd/dd.h Fri Aug 25 14:42:11 2017 (r322892) > +++ head/bin/dd/dd.h Fri Aug 25 15:31:55 2017 (r322893) > @@ -38,10 +38,9 @@ > typedef struct { > u_char *db; /* buffer address */ > u_char *dbp; /* current buffer I/O address */ > - /* XXX ssize_t? */ > - size_t dbcnt; /* current buffer byte count */ > - size_t dbrcnt; /* last read byte count */ > - size_t dbsz; /* block size */ > + ssize_t dbcnt; /* current buffer byte count */ > + ssize_t dbrcnt; /* last read byte count */ > + ssize_t dbsz; /* block size */ > > #define ISCHR 0x01 /* character device (warn on short) */ > #define ISPIPE 0x02 /* pipe-like (see position.c) */ > > Modified: head/bin/dd/position.c > ============================================================================== > --- head/bin/dd/position.c Fri Aug 25 14:42:11 2017 (r322892) > +++ head/bin/dd/position.c Fri Aug 25 15:31:55 2017 (r322893) > @@ -207,7 +207,7 @@ pos_out(void) > n = write(out.fd, out.db, out.dbsz); > if (n == -1) > err(1, "%s", out.name); > - if ((size_t)n != out.dbsz) > + if (n != out.dbsz) > errx(1, "%s: write failure", out.name); > } > break; >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpU5rsrwU4EEkF3i5gvzSZRX3Q=K%2B8M-LMwk2cGP8hMSxw>