From owner-svn-src-all@freebsd.org Fri Aug 25 17:18:07 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 353B1DDD048; Fri, 25 Aug 2017 17:18:07 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-oi0-f44.google.com (mail-oi0-f44.google.com [209.85.218.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0265C67E4F; Fri, 25 Aug 2017 17:18:06 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-oi0-f44.google.com with SMTP id j144so3917646oib.1; Fri, 25 Aug 2017 10:18:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to:cc; bh=L93maoUCuCS8ZcVHvQ32sc3RpCX2tYz4obaitLTRO0Q=; b=fKDrp1+1VBiHFLHSlHDwbnrLa+2V15JehdAXMKfUrkKWZqP/9F7nGVmlNHXJGWHwpT 2Bv+J1aYfqhzQJ5QwKS1ClpE75+kgW18BDLQVoq0u9n8+JumklPOqvQ5T5p5dH6wxSwV eUVTPm3G552eKDv/f7NaDhN/L8+cpTEb1DStoY56pp5n+gBtDGyRxiOEEKOXU7Pr5oKy HTgbZGNxtZrmWJ6DymlD8T2CvpoRFC5+yhPDHi1auBKRczFhB44XcivPmh1cOVsMagv8 re8S7OdD0breevIsbiXobakvp/mSfosMyWEOe1YaBjQaFMV9GB6KLdJJsoa9hsFXkSCU SAVQ== X-Gm-Message-State: AHYfb5iBkUGIvK8VE+xvnnvOlCHNEYntqhBRcZNflwnbB0xQiuhDDnyv qabIiX0ONtCqAmJKDHA= X-Received: by 10.202.252.199 with SMTP id a190mr14663900oii.268.1503681479301; Fri, 25 Aug 2017 10:17:59 -0700 (PDT) Received: from mail-it0-f47.google.com (mail-it0-f47.google.com. [209.85.214.47]) by smtp.gmail.com with ESMTPSA id e206sm6292062oig.50.2017.08.25.10.17.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 25 Aug 2017 10:17:59 -0700 (PDT) Received: by mail-it0-f47.google.com with SMTP id 77so2194119itj.0; Fri, 25 Aug 2017 10:17:59 -0700 (PDT) X-Received: by 10.36.14.216 with SMTP id 207mr127403ite.151.1503681478707; Fri, 25 Aug 2017 10:17:58 -0700 (PDT) MIME-Version: 1.0 Reply-To: cem@freebsd.org Received: by 10.2.81.131 with HTTP; Fri, 25 Aug 2017 10:17:58 -0700 (PDT) In-Reply-To: <201708251531.v7PFVtoZ038242@repo.freebsd.org> References: <201708251531.v7PFVtoZ038242@repo.freebsd.org> From: Conrad Meyer Date: Fri, 25 Aug 2017 10:17:58 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r322893 - head/bin/dd To: Alan Somers Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Aug 2017 17:18:07 -0000 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 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 > > +#include > #include > #include > #include > @@ -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; >