Re: [REVIEW] Hide BIT_* macros from userland code

On Fri, Dec 03, 2021 at 11:03:54AM +0100, Stefan Esser wrote:
> Am 02.12.21 um 17:46 schrieb Shawn Webb:
> > Hey Stefan,
> > 
> > On Thu, Dec 02, 2021 at 05:26:55PM +0100, Stefan Esser wrote:
> >> I have created
> >>
> >> 	https://reviews.freebsd.org/D33235
> >>
> >> to remove the BIT_* macros used in the kernel from the userland API.
> >>
> >> They conflict with differing definitions in some 3rd party code and
> >> lead to compile issues in a number of ports (via CPU_* macros based
> >> on the BIT_* macros).
> >>
> >> See PR259787 https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=259787
> >> for an example of such a problem.
> > 
> > I recently was in a position to evaluate BIT_* macros for userland
> > use. It was around the time when the conversation regarding hiding
> > BIT_* from userland, which conversation caused me to find another
> > solution.
> > 
> > I think such an API is incredibly useful, so I wonder if there's a way
> > to satisfy both. For example, maybe prefix the userland side with a
> > USERLAND_ or something similar? Kernel would use BIT_* and userland
> > would use USERLAND_BIT_* (just spitballing, not actually advocating
> > for "USERLAND_BIT_*" but rather just the idea of it.)
> 
> Hi Shawn,
> 
> I have updated the patch set in review D33235 and have added you to
> the reviewer list.
> 
> IMHO the approach proposed by Konstantin Belousov is better than the
> introduction of prefixed macro names for the userland.
> 
> A simple #define _WANT_FREEBSD_BITSET makes the __BIT* macros available
> by their traditional names, no other changes are required in the code.
> 
> This does not solve the potential case of a program that wants to use
> both the BSD and GLIBC variants of the macros in a single source file.
> But I think that such a case is constructed and does not occur in
> actual code.
> 
> And in any case, the IMHO __BIT* names are as good as the USERLAND_BIT*
> names you suggest (and I understand that you did not want that specific
> name - therefore a prefix of __ might be considered to match what you
> proposed ;-) ).
> 
> And you are of course free to map __BIT* to any other prefixed name in
> a header file in your code ...
> 
> An update of the bitset(9) man page might be a good idea, explaining
> the visibility rules and _WANT_FREEBSD_BITSET for system utilities that
> need to work with kernel style bitsets.

Awesome. kib's suggestion makes sense here. And thanks for adding me
to the review. I'll make sure to pay attention as it progresses. :-)

Thanks again,

-- 
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc