From owner-svn-src-all@FreeBSD.ORG Mon Nov 21 09:30:02 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 551591065672; Mon, 21 Nov 2011 09:30:02 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail01.syd.optusnet.com.au (mail01.syd.optusnet.com.au [211.29.132.182]) by mx1.freebsd.org (Postfix) with ESMTP id D62BB8FC0A; Mon, 21 Nov 2011 09:30:01 +0000 (UTC) Received: from c211-28-227-231.carlnfd1.nsw.optusnet.com.au (c211-28-227-231.carlnfd1.nsw.optusnet.com.au [211.28.227.231]) by mail01.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id pAL9Tvad010353 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 21 Nov 2011 20:29:59 +1100 Date: Mon, 21 Nov 2011 20:29:57 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: mdf@freebsd.org In-Reply-To: Message-ID: <20111121200813.E1158@besplex.bde.org> References: <201111122001.pACK1UML059238@svn.freebsd.org> <20111113220817.GC1677@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="0-825119736-1321867797=:1158" Cc: src-committers@freebsd.org, Pawel Jakub Dawidek , Garrett Cooper , Alexander Motin , svn-src-head@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r227473 - head/sbin/geom/class/multipath X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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: Mon, 21 Nov 2011 09:30:02 -0000 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --0-825119736-1321867797=:1158 Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE On Sun, 13 Nov 2011 mdf@freebsd.org wrote: > On Sun, Nov 13, 2011 at 2:46 PM, Garrett Cooper wrot= e: >> On Sun, Nov 13, 2011 at 2:08 PM, Pawel Jakub Dawidek w= rote: >>> On Sat, Nov 12, 2011 at 12:16:23PM -0800, Garrett Cooper wrote: >>>> On Sat, Nov 12, 2011 at 12:01 PM, Alexander Motin wr= ote: >>>>> Author: mav >>>>> Date: Sat Nov 12 20:01:30 2011 >>>>> New Revision: 227473 >>>>> URL: http://svn.freebsd.org/changeset/base/227473 >>>>> >>>>> Log: >>>>> =A0Fix build on some archs after r227464. >>>>> >>>>> Modified: >>>>> =A0head/sbin/geom/class/multipath/geom_multipath.c >>>>> >>>>> Modified: head/sbin/geom/class/multipath/geom_multipath.c >>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D >>>>> --- head/sbin/geom/class/multipath/geom_multipath.c =A0 =A0 Sat Nov 1= 2 19:55:48 2011 =A0 =A0 =A0 =A0(r227472) >>>>> +++ head/sbin/geom/class/multipath/geom_multipath.c =A0 =A0 Sat Nov 1= 2 20:01:30 2011 =A0 =A0 =A0 =A0(r227473) >>>>> @@ -133,7 +133,8 @@ mp_label(struct gctl_req *req) >>>>> =A0 =A0 =A0 =A0uint8_t *sector, *rsector; >>>>> =A0 =A0 =A0 =A0char *ptr; >>>>> =A0 =A0 =A0 =A0uuid_t uuid; >>>>> - =A0 =A0 =A0 uint32_t secsize =3D 0, ssize, status; >>>>> + =A0 =A0 =A0 ssize_t secsize =3D 0, ssize; >>>>> + =A0 =A0 =A0 uint32_t status; >>>>> =A0 =A0 =A0 =A0const char *name, *name2, *mpname; >>>>> =A0 =A0 =A0 =A0int error, i, nargs, fd; >>>>> >>>>> @@ -161,8 +162,8 @@ mp_label(struct gctl_req *req) >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0disksize =3D msize; >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else { >>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (secsize !=3D ssize= ) { >>>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 gctl_er= ror(req, "%s sector size %u different.", >>>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= name, ssize); >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 gctl_er= ror(req, "%s sector size %ju different.", >>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= name, (intmax_t)ssize); >>>> >>>> Shouldn't that be uintmax_t, not intmax_t ? >>> >>> No, ssize_t is signed. Although the best would be to use %zd for >>> ssize_t. >> >> Thanks... Missed the leading s. > > ... except that the cast and conversion specifier aren't in sync. The > cast is signed; the conversion specifier is unsigned. %zd is still > best, since it's the conversion specifier for the variable's type. > Next best is %jd and cast to intmax_t, since the signedness is > preserved. But ssize_t is still a nonsense type for representing a sector size. ssize_t is for the return type of read(2) and write(2) and not much more. On 64-bit arches it is (bogusly) int64_t, although read() and write() are still limited to INT_MAX for historical reasons (buggy standards may require ssize_t to have this wrong type and SSIZE_MAX to have the corresponding wrong value INT64_MAX (equal to the maximum value representable by this wrong type and not equal to the maximum value that read() and write() actually support). These bugs were missing in V7 where read() and write() took and returned plain int sizes. Portable code must still call them with a size <=3D INT_MAX, else it may break, for example on FreeBSD. When ssize_t is misused for something unrelated to read() and write(), it mainly gives printf format complications since it tends to be larger than necessary. Here it gives a printf format complication of a cast to intmax_t, and a printf format error since intmax_t doesn't match the format %ju, and it gives the nonsense that sectors of size >=3D 2GB are "supported" on 64-bit systems but not on 32-bit systems. Testing this support is difficult even with virtual disks. > But in the end it doesn't matter a lot since whatever is printed in > the error message, it's likely derivable from reading the code what > the actual value used was. Even when the value is truncated, provided the behaviour is defined or benign. Bruce --0-825119736-1321867797=:1158--