Date: Fri, 6 Apr 2018 11:04:21 +0100 From: Roger Pau =?utf-8?B?TW9ubsOp?= <royger@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: Warner Losh <imp@bsdimp.com>, Ian Lepore <ian@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r332072 - head/sys/sys Message-ID: <20180406100421.kumj77n6sfgagozx@MacBook-Pro-de-Roger.local> In-Reply-To: <20180406022720.I3453@besplex.bde.org> References: <201804051431.w35EVtg4047897@repo.freebsd.org> <1522942377.49673.245.camel@freebsd.org> <20180405154619.q3blip266qa3z5ut@MacBook-Pro-de-Roger.local> <CANCZdfputnwHg-gChVKfvZdHn1nHcHXpE0WVLfejHUZfeC8jLg@mail.gmail.com> <20180406022720.I3453@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Apr 06, 2018 at 03:12:08AM +1000, Bruce Evans wrote: > On Thu, 5 Apr 2018, Warner Losh wrote: > > > On Thu, Apr 5, 2018 at 9:46 AM, Roger Pau Monné <royger@freebsd.org> wrote: > > > > > On Thu, Apr 05, 2018 at 09:32:57AM -0600, Ian Lepore wrote: > > > > On Thu, 2018-04-05 at 14:31 +0000, Roger Pau Monné wrote: > > > > > Log: > > > > > introduce GiB and MiB macros > > > > > ... > > > > > +/* Unit conversion macros. */ > > > > > +#define GiB(v) (v ## ULL << 30) > > > > > +#define MiB(v) (v ## ULL << 20) > > > > > + > > > > > #endif /* _SYS_PARAM_H_ */ > > > > > > > > These names don't make it clear whether the conversion is bytes->GiB or > > > > GiB->bytes. The names seem way too generic for a public namespace in a > > > > file as heavily included behind your back as param.h is. > > > > > > > > Also, this completely reasonable usage won't work, likely with > > > > confusing compile error messages: > > > > > > > > int bytes, gibytes; > > > > ... > > > > bytes = GiB(gibytes); > > > > > > I find those helpful for their specific usage. I could introduce > > > static inline functions like: > > > > > > size_t gb_to_bytes(size_t)... > > > > > > But I assume this is also going to cause further discussion. > > Yes, it gives even more namespace pollution and type errors. Macros > at least don't expose their internals if they are not used. > > size_t is actually already part of the undocumented namespace pollution > in <sys/param.h>. > > The type errors are restriction to just one type in another way. Type- > generic APIs that avoid such restrictions are much harder to implement > using inline functions than macros. > > > Yea, traditional macro names would be "gibtob" and "btogib" but I didn't > > just reply to bikeshed a name: > > > > But you don't need to specify a type, consider the current btodb macro: > > #define btodb(bytes) /* calculates (bytes / DEV_BSIZE) > > */ \ > > (sizeof (bytes) > sizeof(long) \ > > ? (daddr_t)((unsigned long long)(bytes) >> DEV_BSHIFT) \ > > : (daddr_t)((unsigned long)(bytes) >> DEV_BSHIFT)) > > > > which shows how to do this in a macro, which is orthogonal to any name you > > may choose. I can also bikeshed function vs macro :) > > This macro is mostly my mistake in 1995-1996. The long long abominations > in it were supposed to be temporary (until C99 standardized something > better). It was originally MD for i386 only and then the sizes of almost > all types are known and fixed so it is easier to hard-code minimal sizes > that work. The optimization of avoiding using 64-bit types was more needed > in 1995-1996 since CPUs were slower and compilers did less strength reduction. > > btodb() is much easier than dbtob() since it shifts right, so it can't > overflow unless the cast of 'bytes' is wrong so that it truncations. > dbtob() doesn't try hard to be optimal. It just always upcasts to > off_t. > > jake later convinced me (in connection with his PAE and sparc64 work) that > it should be the caller's responsibility to avoid overflow. Any casts in > the macro limits it to the types in it. This is why the page to byte > conversion macros don't have any casts in them. PAE usually needs 64-bit > results, but this would just be a pessimization for normal i386, and > deciding the casts in the macro as above is complicated. > > So correct GB() macros would look like ((v) << 30), where the caller must > cast v to a large enough type. E.g., for variable v which might be larger > than 4, on 32-bit systems, the caller must write something like > GB((uintmax_t)v). But it is easier for writing to just multiply v by 1G. > This is also easier for reading since it is unclear that GB() is even a > conversion or which direction it goes in. A longer descriptive name would > be about as clear and long as an explicit multiplication. > > I usually write 1G as ((type)1024 * 1024 * 1024) since the decimal and > even hex values of 1G have too many digits to be clear, and > multiplication is clearer than shifting and allows the type to be in > the factor. > > Disk block size conversions need to use macros since the DEV_BSIZE = 512 > was variable in theory (in practice this is now a fixed virtual size). > Conversions to G don't need macros since the magic number in them is no > more magic than the G in their name. I personally find the following chunk: if (addr < GiB(4)) ... Much more easier to read and parse than: if (addr < (4 * 1024 * 1024 * 1024)) ... But I won't insist anymore. I will revert this and introduce the macros locally where I need them. Roger.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180406100421.kumj77n6sfgagozx>