
C++ Patterns: The Badge - brianush1
https://awesomekling.github.io/Serenity-C++-patterns-The-Badge/
======
QuadrupleA
I used to carefully design my C++ code with public / private / protected /
friend (and the pointer-to-impl pattern for "compile time private") but a few
years ago I started doing everything all-public ala python (struct instead of
class by default) with occasionally an m_ prefix for "intended to be private",
and I've never looked back. It's been great, made my programming life much
easier.

This badge thing is clever, but seems like yet more accidental/unnecessary
complexity on top of the already-probably-unnecessary private/protected/friend
- as requirements change you now have this extra layer of access-control cruft
to reason about, that you probably wouldn't even miss if it didn't exist.

Not to get too ranty, and no disrespect to Andreas, but the C++ culture seems
to thrive in unnecessary / accidental complexity, deep template hacks and the
like that add friction to refactoring, obfuscate underlying logic so you miss
opportunities to simplify, and turn straightforward programming problems into
brain-bending template puzzles.

OOP also; I've seen so many crazy class architectures that could just be a
handful of plain-old-functions, complex virutal "type families" that could
just be a TypeOfThingThisIs enum with some if statements in the respective
functions, etc.

I don't know the Serenity codebase and use-case very well, and perhaps this
kind of thing has a place in large libraries, but honestly even then I'd lean
towards just making everything simple & public and use a prefix when needed,
or describe appropriate usage in documentation. Seems to work in the python
world (minus a few outlier codebases that go OOP-crazy).

~~~
xleviator
In larger codebases with many developers, attention to the API is required in
order to be able to maintain code without breaking clients.

The main downside of Badge<T> is that it comes with a performance penalty and
ABI change for something that should be statically deducible (empty struct is
one byte in C++).

~~~
vnorilo
There is no performance penalty in any modern compiler that I am aware of
(effectively no code will be emitted).

You can have a look here:
[https://godbolt.org/z/DmeYL-](https://godbolt.org/z/DmeYL-)

~~~
xleviator
Sure within a single compilation unit. Otherwise, I like dearrifling's
suggestion.

~~~
vnorilo
The example also shows a call to a different translation unit. There's no code
for passing the empty struct. There's also none for handling it, as you can
see from the assembly output.

The one byte rule does nothing here, because its value is undefined and its
address not observable.

edit: if you find a case where an empty struct parameter causes code to be
emitted, let's consider filing bugs.

~~~
xleviator
Oops, I did miss that, but this seems to be compiler or calling convention
specific. x86_64 gcc 9.1, linked, shows no difference. However arm64 gcc 8.2
and x86-64 clang show a difference.

~~~
vnorilo
thanks for clarification! you’re of course right about the calling convention.

------
akling
Hello friends! Author here, nice to see so much discussion :)

I've spent most of my adult life working on large C++ codebases with public
API's (most notably Qt and WebKit), which has led me to cultivate a defensive
programming mindset. Someone mentioned Hyrum's law, which is 100% accurate in
my experience.

I should be honest and admit that Badge<T> is not yet diligently applied
everywhere in the Serenity codebase. I wanted to "feel it out" for a while
first, but now I've decided I like it, so I'm applying it more and more going
forward.

Oh and while I have your attention, I recently posted a May 2019 update
video[1] on the Serenity OS project if you're curious how things are going :)

[1]
[https://www.youtube.com/watch?v=KHpGvwBTRxM](https://www.youtube.com/watch?v=KHpGvwBTRxM)

~~~
w0utert
I like this idea, except for the fact that the extra parametrer pollutes the
method signature and method calls, for something that really should only be a
compile-time directive.

I'm actually curious why something like this has never been included as a
language feature, it seems trivial to come up with a syntax for fine-grained
access control, that is fully backwards-compatible and a true zero-cost
abstraction.

Most obvious first shot:

    
    
      class A
      {
        private:
          void badgedFunction() const friend Device, SomeOtherClass;
      };
    

Did you ever check with the C++ working group if there are any proposals for
language extensions for this purpose?

~~~
akling
Yeah, polluting the signatures is a shame.

I've only just decided that I enjoy using this pattern so my internal
standards committee hadn't progressed to the proposal stage yet. :)

Your syntax makes perfect sense to me, and I think it would make a great
addition to the language. I wonder what the real working group would say. (I
tried quickly googling for something that might sound like an existing
proposal but came up empty.)

------
nly
Personally I'd move the register_device() and unregister_device() functions
outside of the VFS class entirely, perhaps to namespace scope.

Alternatively, there are other ways to leverage friendship and access control
in C++. Here's one option:

    
    
        template <typename Registry>
        class DeviceManager;
    
        class Device {
            template<typename Registry> friend class DeviceManager;
            int y;
        };
    
        class VFS {
            friend struct DeviceManager<VFS>;
            int x;
        };
    
        template <typename Registry>
        struct DeviceManager {
            static void register_device (Registry& registry, Device& device) {
                registry.x = 1; // accessing privates
                device.y = 2;   // accessing privates
            }
        };
    
        int main() {
            VFS vfs;
            Device dev;
            DeviceManager<VFS>::register_device (vfs, dev);
        }
    

Here all DeviceManager<>'s can reach inside Devices, but only the VFS device
manager can reach inside VFS.

This is also flexible. Remove 'static' and you get yourself a stateful
mediator/manager. Add a variadic template register_devices_s_ member to
Devicemanager and you've got yourself a convenience function for registering
multiple devices (of heterogeneous types) without bloating the VFS API and,
without runtime cost, and without losing type safety (by e.g. casting down to
Device& from USBDevice&).

~~~
kazinator
The thing is, there is no restriction on who calls
DeviceManager<VFS>::register_device. main() is able to do it.

This doesn't seem so different from:

    
    
       class A { private: friend void foo(A &, B &); int x; };
       class B { private: friend void foo(A &, B &); int y; };
    
       void foo(A &a, B &b)
       {
          a.x = 1;
          b.y = 2;
       }
       
       int main() {
          A a; B b;
          foo(a, b);
       }
    

We've just obfuscated it a bit by using a template class with one function in
it, and declaring that to be the friend instead of just a non-member function.

~~~
nly
Yes, your version is what I mean by moving it to namespace scope, and it's
probably what I'd do to start off with.

> there is no restriction on who calls DeviceManager<VFS>::register_device

Well, you can make it private inside DeviceManager and give the class its own
friends :-))

Another benefit of my solution is that adding things to DeviceManager<VFS>
doesn't require changing the VFS.h or Device.h headers at all. Anything that
links these 2 classes is an entirely separate concern and very much in line
with thinking in terms of models or concepts rather than types. What you want
at the end of the day is an absolute bare minimum of member functions in any
class.

To be honest, whatever the solution, the key is that having the Device class
self-register to the VFS in its constructor (as in Andreas's blog post) is,
imho, an anti-pattern and is what is really leading to this Badge mess.

------
sorenjan
Could you make the badge the last argument and give it a default value, so you
don't need the initialization at the function call? It looks a bit strange and
would need an explanation for why the empty initialization list is there IMO.

I'm not sure if it would work, cppreference has this to say about default
arguments:

> The names used in the default arguments are looked up, checked for
> accessibility, and bound at the point of declaration, but are executed at
> the point of the function call

[https://en.cppreference.com/w/cpp/language/default_arguments](https://en.cppreference.com/w/cpp/language/default_arguments)

~~~
ryanianian
It's an empty struct so the compiler will elide any storage either way, but
agreed having it be the first arg is a bit odd. Perhaps it's for regularity in
the cases where you want to make variadic functions 'badged' in this way.

~~~
dllthomas
Empty structs have size 1, per the C++ spec. The compiler probably won't elide
the storage unless it can _show_ that no one probably cares.

~~~
vnorilo
In this case, it should absolutely do it.

Take a look: [https://godbolt.org/z/DmeYL-](https://godbolt.org/z/DmeYL-)

~~~
dllthomas
I'm surprised, particularly about the cross-linking bit as that seems to
violate the spec (per my recollection of the spec, which is the most likely
thing to be wrong).

It's worth noting that it does apply to the latest gcc and Clang but not to
all the compilers listed there. For an example of a difference, the latest
djggp (7.2.0) inserts an extra push in the call when the empty argument is
present.

~~~
vnorilo
If you use a calling convention that mandates the use of stack, you would see
that push, or at least have a byte reserved.

AFAICT the C++ standard itself does not specify the particulars of calling
convention, which is a function of the platform, compiler and
flags/attributes.

------
jackewiehose
I do C++ for almost 20 years but never came across this simple technique
before, so I'm quite surprised that I actually like it.

I usually avoid 'friend' in these situations and just write

    
    
        //private:
    

before the method declaration. But it actually happened that co-workers used
these private methods because they use code-completion instead of reading the
header...

But I might would call the class Friend<T> or something.

~~~
derefr
> But it actually happened that co-workers used these private methods because
> they use code-completion instead of reading the header...

A nice feature I've noticed in Elixir is that giving a function a "@doc false"
annotation will actually prevent IDE/REPL autocomplete from completing the
function. (It's still there to call if you type it yourself.) Maybe that's
something C++ tooling could copy.

------
zwieback
I like it but is access restriction really a problem? In my years of C++
programming it always seemed to be a theoretical problem more than something
that leads to crashes.

~~~
jandrewrogers
This solves a chronic architectural maintenance problem in complex C++ code
bases, which I've also run into countless times.

Any visible interface, public or private (via friend), will eventually be used
by other programmers for other than its intended use case simply because it is
there and accessible. For the maintainer of an interface with a single
intended purpose, what should be a clean, tidy modification within the scope
of its intended use ends up breaking all kinds of unrelated code that the
maintainer wasn't even aware of that is using the interface in ways the
designer didn't anticipate. It is a common form of architectural rot. Reducing
the incidence of this in large code bases is usually enforced by soft means,
like sternly worded comments, vigorous policing, etc but that only goes so
far. Ideally, an access policy would be in the code itself and enforced by the
compiler.

The public/private labels don't address this. If you make things public, the
world has the ability to do unseemly things with your interface. If you friend
a bunch of classes so they can access private methods, those classes have the
ability to do unseemly things with your private implementation details. Both
cases embrittle the architecture. What you really want is fine-grained ACLs to
class members to document both intent and dependencies.

This method of restricting usage of methods to specific classes at compile
time is clever and provides good documentation of design intent, I haven't
seen that pattern before. I also would guess that the compiler elides the
Badge argument since it is never used.

~~~
nwallin
There's a name for this phenomena: Hyrum's Law.

> With a sufficient number of users of an API, it does not matter what you
> promise in the contract: all observable behaviors of your system will be
> depended on by somebody.

[http://www.hyrumslaw.com/](http://www.hyrumslaw.com/)

~~~
kjeetgill
I never knew it had a "real name". I always referenced back to the comic:
[https://xkcd.com/1172/](https://xkcd.com/1172/)

Essentially, a bugfix that prevents CPU overheating breaks someone's workflow
involving a temperature sensor (jokingly).

------
arithma
A different pattern I have thought of would be to provide an accessor
interface object, and certain functionality is limited to be accessible only
through that object. Controlling who has access to that interface can be
flexible (possibly returning it along with construction, or passing it in at
creation time...) Albeit, this is not without overhead, and for that, Badge is
actually better, in terms of performance, if it fits the use case. What
happens when you have multiple Device objects though, in this case, they can
all access VFS..

\--

C++'s private inheritance can be used here to avoid some of the gymnastics,
wherein, that private aspect of the class can be passed out into the
appropriate party at creation time or otherwise.

To illustrate:
[https://gist.github.com/arithma/0d96aec5c07e2d8dc24cfc6cd31f...](https://gist.github.com/arithma/0d96aec5c07e2d8dc24cfc6cd31f0c16)

Note: I have never used this in production as far as I remember.

~~~
mmkos
Pretty sure you described the attorney-client pattern.

~~~
arithma
If it is, it's the first time I hear about it. It always gives me a kick when
I make up a similar design pattern to some grey beard's invention. Alas, I
should read up on those patterns to avoid reinventing stuff and sharing things
with people with appropriate names.

------
mwkaufma
I like it! The self-documentation of where the call is coming-from is quite
nice (though a more descriptive name like CallFrom<T> would make that even
clearer, were it the principle intent). I can't help myself, though:

    
    
        template<typename T>
        Badge<T> FakeBadge() {
          struct Stub {}
          return reinterpret_cast<Badge<T>>(Stub());
        }

~~~
kazinator
Doesn't subvert Device badges for me. Complete sample:

    
    
      template<typename T>
      class Badge {
          friend T;
          Badge() {}
      };
    
      template<typename T>
      Badge<T> FakeBadge() {
        struct Stub {};
        return reinterpret_cast<Badge<T>>(Stub());
      };
    
      class Device {
      public:
        static Badge<Device> getBadge() { return Badge<Device>(); }
      };
    
      void needDeviceBadge(Badge<Device> badge)
      {
      }
    
      int main()
      {
         needDeviceBadge(Device::getBadge());   // no error
         needDeviceBadge(FakeBadge<Device>());  // error!
         return 0;
      }
    

g++ errors:

    
    
      badge.cc: In instantiation of ‘Badge<T> FakeBadge() [with T = Device]’:
      badge.cc:25:38:   required from here
      badge.cc:10:10: error: invalid cast from type ‘FakeBadge() [with T = Device]::Stub’ to type ‘Badge<Device>’
         return reinterpret_cast<Badge<T>>(Stub());
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    
    

A reinterpret_cast won't turn an instance of the Stub local class into a
Badge<Device>.

That doesn't even have to do with friendship/protection. If we move that line
into the Device class, the reinterpret_cast still doesn't compile.

~~~
a1369209993
It does, however, work fine if you use a proper "* (Badge<T>* )&stub" cast.

~~~
GrayShade
Isn't casting unrelated types undefined behavior?

~~~
brianush1
Yes, in the standard it isn't defined, but on most--or maybe even all?--
systems and compilers it's perfectly defined, since there is no reason why a
compiler would layout two different empty structs in different ways; they're
both empty anyways.

Edit: Actually, from reading some of the other comments in the thread, the
standard apparently specifies that empty structs are 1 byte in size, so
technically they would both be the same size, so casting between them should
be fine.

~~~
kazinator
It's not defined on systems unless a document spells it out. However, the
actual behavior can be deduced to be harmless from black-box testing. compiler
source code and generate object code.

------
jhalstead
This reminds me of the Passkey idiom: [https://arne-mertz.de/2016/10/passkey-
idiom/](https://arne-mertz.de/2016/10/passkey-idiom/)

~~~
akling
Oh wow, that's so similar! So similar in fact, that I must have seen this blog
post at some point and internalized the idea :)

The improvement that Badge<T> makes is the <T> part; you don't need to look up
the PassKey declaration to see who can construct PassKeys, it's encoded right
there in the type instead.

I do like how PassKey makes the copy ctor private. I think we can improve on
that and delete both the copy and move ctors in Badge.

------
ryanianian
How is this different from the (relatively common afaik) passkey idiom
[https://arne-mertz.de/2016/10/passkey-idiom/](https://arne-
mertz.de/2016/10/passkey-idiom/) ?

~~~
ncmncm
Yes, this has various names. "Ticket" is traditional.

------
henning
This is a typical example of a solution to an artificial, self-imposed problem
created by C++'s assumptions.

As a programmer, I would feel frustrated to have to figure this out instead of
spending time on something a user would actually care about.

~~~
jandrewrogers
Which popular programming languages provide proper ACLs (or equivalent
functionality) for class members?

~~~
inetknght
I'm aware of C++ and Xojo for sure.

~~~
desdiv
Is it possible to have a proper compile time ACL for C++?

Arguably the Badge pattern as presented in the article is just an honor system
that provides zero security, since it's trivial to forge a fake Badge:
[https://repl.it/repls/BountifulQuerulousCron](https://repl.it/repls/BountifulQuerulousCron)

~~~
vnorilo
It's less trivial if you make the copy/move constructors private.

[https://repl.it/repls/HatefulMadeupMicrostation](https://repl.it/repls/HatefulMadeupMicrostation)

Of course, type systems are orthogonal to any real security, at least in most
languages, and certainly all languages with no-holds-barred interface to
machine code.

------
jupp0r
This is especially helpful in conjunction with private constructors and
make_shared or make_unique. You cannot even friend make_shared because of
namespace aliasing in standard libraries, but shared_from_this can make
improperly allocated objects error at runtime. Private badges/tokens solve
that problem by allowing public constructors requiring a parameter value of
the badge/token type that has limited visibility, but can be constructed
inside a factory and passed to make_shared.

------
mindfulplay
I often wonder how this would map to other languages (as C++ often entails
complexity and clever work arounds such as this article).

In Java or C# (and of course in C++), you could have a token system where the
token can only be provided by Device :

VFS: void register(DeviceToken):

DeviceToken: empty interface (marker)

Device: private DeviceToken provideDeviceToken();

This hides both VFS and Device classes from accessing each others' private
members.

You can also constrain how gets to provide DeviceToken by adding sufficient
comments in code.

------
kazinator
C++ could solve this without badges, if it didn't have a very tiny, silly
misfeature.

The misfeature is this: a class A can only declare a specific member function
of class B as a _friend_ , if that B member function is public!

Example:

The idea here is that Device has a static member function called
_Device::registrationHelper_. That specific function (not the entire Device
class) is declared a friend to VFS, and so that function can call
_VFS::registerDevice_.

[Edit: this doesn't quite match the Badge solution, though. If this worked, it
would have the problem that the _registrationHelper_ has access to all of
_VFS_ , which is almost as bad as the entire _Device_ class being a friend of
_VFS_. That is one of the problems which motivate Badge. Nice try, though! The
C++ restriction is probably worth fixing anyway. What the C++ friendship
mechanism really needs here is to be able to say that a specific member
function in B is allowed to access a specific member of A.]

    
    
      class Device;
    
      class VFS;
    
      class Device {
      public:  // we want this private!
         static void registrationHelper(VFS &v, Device &d);
      private:
         void registerWith(VFS &vfs) { registrationHelper(vfs, *this); }
      };
    
      class VFS {
      private:
         void registerDevice(const Device &);
         friend void Device::registrationHelper(VFS &, Device &);
      };
    
      void Device::registrationHelper(VFS &v, Device &d)
      {
         v.registerDevice(d);
      }
    

But we are forced to make _Device::registrationHelper_ public, which defeats
the purpose: anyone can call it and use it is a utility to register devices
with VFSs.

If we make Device::registrationHelper private to prevent this, then the
"friend void Device::registrationHelper" declaration in VFS fails.

This is an oversight; the privacy of the _Device::registrationHelper_
identifier means that the _VFS_ class is not even allowed to mention its name
for the sake of declaring it a friend.

That should be fixed in C++. Declaring a function a _friend_ shouldn't require
it to be accessible; we are not lifting its address or calling it; we are just
saying that it can call us. Allowing a function to call us is not a form of
_access_ to that function, yet is being prevented by _access_ specifiers.

~~~
nly
Your solution is similar to mine and you might find it helpful:

[https://news.ycombinator.com/item?id=20160957](https://news.ycombinator.com/item?id=20160957)

------
shkurski
I might be wrong, but doesn't it look like a way to enforce SRP violation? In
the provided example we don't only let the device know how to register itself
via VFS, but we also make sure there will be no future DeviceManager to do
that. Unless we duplicate the entire interface with Badge<DeviceManager> tag.
Am I missing any non-obvious benefits of this approach?

------
cgrealy
While this is a clever little pattern, I'd argue that if you have a method of
class X that can only be called from class Y, that is a code smell.

~~~
Toine
Sometimes, clever little patterns like that are useful to get stuff done in
time. However, it that case it definitely smells a lot. Also, the global
singleton stinks.

To me this is a violation of the Interface Segregation Principle. I think the
author went the right way to fix it, but not far enough.

He added a parameter to segregate the device registration operations from
other operations. It works, but the segregation is artificial : you still need
a Device object (which seems to be used by a lot of classes, another smell,
god object ?)

Instead of asking for an entire Device object, register_device should ask for
a different type, specific to that operation, like « NewDevice », «
UnregisteredDevice », etc. This object would depend on very specific
information only available at the device creation. Now, if another developer
tried to call register_device, he would need to create a weird object that he
never heard of, with parameters he cannot even provide.

Thoughts ?

~~~
dkersten
Is an object really a god object if lots of people use it? I thought it was
the opposite way around: a large object that contains too much functionality,
but this device could be a tiny single purpose object for all we know, that a
lot of clients need to use.

------
dllthomas
Up to this point the comments seem to be missing the obligatory "Treasure of
the Sierra Madre" reference, so here I am fixing that.

------
skocznymroczny
In D, you can access private members of a class within a single module. I like
it, because it allows to move methods outside of the class, but doesn't
require explicit friend declarations. And you still have the data protection
because other modules can't access the private fields.

~~~
nikbackm
Maybe C++ could add this feature at some point now that it too will get
modules.

------
cjhanks
I had never seen this before, I cannot think of another way to do this. I
suspect it has 1 byte overhead (on the stack) cost, but that's pretty cheap
and may be optimized out.

------
nisuni
How can {} return a Badge object? Some kind of operator overload?

~~~
cjhanks
It's bracket initialization of a struct, in this case the struct has no
variables - the type is implicit from the method declaration.

~~~
eMSF
While it makes no difference in this case, it's worth noting that Badge<T> is
not an aggregate type because it has a user-provided constructor. Therefore
the object is value initialized, i.e. the braces are equivalent to
'Badge<Device>()'.

edit: what this means is that even if the struct had data members, you
couldn't brace initialize it like an aggregate type, and to achieve something
syntactically similar, you would need a suitable constructor.

------
notacoward
So a badge seems a bit like a capability. Clever; I like it.

~~~
monocasa
Yeah, my thought too. Even the name badge invokes the idea of a capability.

~~~
fiddlerwoaroof
And, I believe you could probably subclass Badge to make capabilities more
granular.

------
AstralStorm
This is essentially a capability system enforced by the compiler, which means
your code is not actually in control for any third party caller.

A glaring security hole. Any old hacker can forge or clone a data structure.
This "badge" (AKA token) has to be explicitly unpredictably replay-proof
generated and hard to forge, and also automatically verified.

------
nercht12
Seems more of a people problem than a software problem. That's normal. Why not
just use a comment that says "Don't use this. It will break."? If people can't
obey simple instructions, you have a different problem entirely.

~~~
barrkel
Code completion doesn't necessarily read comments. Why would people read the
headers before using?

If people would only obey simple rules, C++ would be a memory safe language.
Alas, they don't.

------
mkettn
whats the advantage over declaring register_device as a functor object from an
anon class? for example:

    
    
      class Device;
      class VFS {
      public:
       class {
       friend Device;
       void operator()(int y) {/* do stuff with y here*/}}     register_device;
      };
      class Device {
      public:
       void foo(VFS fs) {
        fs.register_device(1);
       }
      };
      
      int main(int argc, char *argv[]) {
      // fails:
      // VFS().register_device(2);
      // works:
       Device().foo(VFS());
       return 0;
      }
    

and no need for a badge class (erm... well but an anon class).

my c++ skills are a bit rusty, because most of the time python works just
fine.

~~~
akling
Hmm, IMO Badge has two main advantages over your approach:

1\. Aesthetics. (This is down to personal taste of course, but I much prefer
how Badge gets the job done without needing multiple lines of code at every
declaration.)

2\. What if I need to put the function implementation out-of-line?

------
Something1234
Wouldn't -Wextra yell about the Badge being an unused parameter in the
register devices? You would have to do some kind of dummy call that the
compiler would have to optimize away.

~~~
akling
If an argument is nameless, the compiler won't complain about it being unused.

No warning:

    
    
        void VFS::register_device(Badge<Device>, Device& device) { ... }
    

Warning:

    
    
        void VFS::register_device(Badge<Device> badge, Device& device) { ... }

------
gowld
> These functions are called by all Device objects when they are constructed
> and destroyed respectively. They allow the VFS to keep track of the
> available device objects so they can be opened through files in the /dev
> directory.

> Now, nobody except Device should ever be calling VFS::register_device() or
> VFS::unregister_device(),

Why?

Either have VFS call the constructor because non-registered devices are banned
and RAII is good, or don't create arbitrary restrictions to hamstring how the
rest of the system manages Devices.

~~~
asveikau
I kind of agree that it's not clear why a VFS needs to register devices that
only register themselves, however, if you suspend disbelief on that one it's a
clever trick that is easy to imagine being useful.

------
choppaface
Isn’t this exactly the problem that categories in Objective C solve? Why not
adopt that pattern versus this “Badge” thing?

