Date: Sat, 5 Aug 2017 12:26:03 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Bruce Evans <brde@optusnet.com.au>, Hans Petter Selasky <hps@selasky.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r321920 - head/sys/sys Message-ID: <20170805112924.W1055@besplex.bde.org> In-Reply-To: <20170803120729.GO1700@kib.kiev.ua> References: <201708021014.v72AEHEk061037@repo.freebsd.org> <f7a4a90c-f1d8-b381-27fe-3cf76b574a29@selasky.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>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <sys/select.h> > > -#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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170805112924.W1055>