From owner-svn-src-head@freebsd.org Sat Aug 5 02:26:12 2017 Return-Path: Delivered-To: svn-src-head@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 5D6A8DD74AF; Sat, 5 Aug 2017 02:26:12 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id E59E28D3; Sat, 5 Aug 2017 02:26:11 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 62CF71045ED9; Sat, 5 Aug 2017 12:26:03 +1000 (AEST) Date: Sat, 5 Aug 2017 12:26:03 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov cc: Bruce Evans , Hans Petter Selasky , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r321920 - head/sys/sys In-Reply-To: <20170803120729.GO1700@kib.kiev.ua> Message-ID: <20170805112924.W1055@besplex.bde.org> References: <201708021014.v72AEHEk061037@repo.freebsd.org> <37abc595-c80e-a8da-04a8-815f42c634de@selasky.org> <20170802135455.GG1700@kib.kiev.ua> <20170803122015.Q1093@besplex.bde.org> <20170803075747.GJ1700@kib.kiev.ua> <20170803180419.R2314@besplex.bde.org> <20170803120729.GO1700@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=KeqiiUQD c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=HLF2S318A8q7yMrcC8QA:9 a=rz2QiUMzXMD4j6UH:21 a=fJpQLJjFt_BGb_za:21 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Aug 2017 02:26:12 -0000 On Thu, 3 Aug 2017, Konstantin Belousov wrote: > On Thu, Aug 03, 2017 at 07:34:56PM +1000, Bruce Evans wrote: >> I see another problem. Masking with 0xfffffffff and casting to unsigned >> are gratuitously different spellings for extracting the low 32 bits. >> I prefer the cast. > > Below is one more update. I reformulated the man page text, but not too > deep. Also I replaced masking with the cast. OK. > diff --git a/sys/sys/types.h b/sys/sys/types.h > index 30a08724443..eacbc1ba0c4 100644 > --- a/sys/sys/types.h > +++ b/sys/sys/types.h > @@ -364,9 +364,9 @@ __bitcount64(__uint64_t _x) > > #include > > -#define major(x) ((int)((dev_t)(x) >> 32)) /* major number */ > -#define minor(x) ((int)((x) & 0xffffffff)) /* minor number */ > -#define makedev(x, y) (((dev_t)(x) << 32) | (unsigned)(y)) /* create dev_t */ > +#define major(x) ((int)((dev_t)(x) >> 32)) > +#define minor(x) ((int)(x)) Another nice simplification. Strictly, it should be (int)(dev_t)(x) since the pseudo-prototype says that the arg is converted to dev_t, but yesterday I couldn't see any differences even for exotic x and exotic arches. Today I can see some difference for exotic x and perverse implementations. E.g., x with extra bits in a large signed integer type to invoke implementation-defined behaviour, and a perverse implementation that truncates to 0 for direct conversion to int but does the right thing for indirect conversion). But we depend on the implementation doing the right thing for other casts to int. Also, if dev_t == uintptr_t, it is valid for the caller to keep dev_t's in void *'s internally but not to pass void * to minor() or major() according to the prototype. However, casting in the macros breaks the warning for this. I think pure macros should not cast their args or pretend to have prototypes, but require callers to pass args of supported types. The old dev_t macros were closer to this -- they have expressions like ((x) << 8) which will fail if x is not an integral type or give wrong results ix x has extra bits. POSIX had to fix related problems for the ntohl() family. The result seems to be that the APIs act as if they are functions. So even if they are implemented as macros, the macros can't just cast their args, but must be wrapped by functions (which can be inline) to get less forceful conversions. FreeBSD's implementation does almost exactly this. It has to cast args for optimizing const args, but seems to get this right by not casting in the non-const case. However, I have uncommitted "fixes" which do cast in the non-const case. Apparently my fixes are backwards. > +#define makedev(x, y) (((dev_t)(x) << 32) | (unsigned)(y)) > > /* > * These declarations belong elsewhere, but are repeated here and in Bruce