14 ms·
C++ Patterns: The Badge
- arithma 7y agoA 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/0d96aec5c07e2d8dc24cfc6cd31f0c16 https://gist.github.com/arithma/0d96aec5c07e2d8dc24cfc6cd31f... Note: I have never used this in production as far as I remember.
- mmkos 7y agoPretty sure you described the attorney-client pattern.
- arithma 7y agoIf 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.
- nisuni 7y agoHow can {} return a Badge object? Some kind of operator overload?
- antisemiotic 7y agoBrace initialization, I suppose.
- cjhanks 7y agoIt's bracket initialization of a struct, in this case the struct has no variables - the type is implicit from the method declaration.
- eMSF 7y agoWhile 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 7y agoSo a badge seems a bit like a capability. Clever; I like it.
- monocasa 7y agoYeah, my thought too. Even the name badge invokes the idea of a capability.
- fiddlerwoaroof 7y agoAnd, I believe you could probably subclass Badge to make capabilities more granular.
- zwieback 7y agoI 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.
- detaro 7y agoI'd guess it's more architectural concern than runtime. Making sure only code that is supposed to uses a semi-private interface so people don't build dependencies that aren't wanted.
- bdowling 7y agoThe C++ access restrictions exist solely to teach and enforce proper API usage by other developers. They're supposed to trigger a reconsideration of the API when a new developer runs into a restriction, but more often in my experience the new developer will just add a new public interface.
- kazinator 7y agoAnd you can add yourself a public interface even if all you have is a compiled library, and header files. Flipping a "private:" to "public:" has no effect on the binary compatibility.
- slavik81 7y ago> Flipping a "private:" to "public:" has no effect on the binary compatibility. That's not the case on Windows. The access qualifier is part of the mangled name. https://en.wikiversity.org/wiki/Visual_C%2B%2B_name_mangling#Basic_Structure https://en.wikiversity.org/wiki/Visual_C%2B%2B_name_mangling...
- kazinator 7y agoThat shouldn't prevent the subversion of data members or inline functions, or the ability to add your own member functions to the class.
- cjhanks 7y agoI 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.
- jackewiehose 7y agoI 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 7y ago> 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.
- jhalstead 7y agoThis reminds me of the Passkey idiom: https://arne-mertz.de/2016/10/passkey-idiom/ https://arne-mertz.de/2016/10/passkey-idiom/
- b0sk 7y agoThis is pretty much the PassKey idiom
- akling 7y agoOh 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.
- shkurski 7y agoI 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?
- dllthomas 7y agoUp to this point the comments seem to be missing the obligatory "Treasure of the Sierra Madre" reference, so here I am fixing that.
- sorenjan 7y agoCould 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 7y agoIt'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 7y agoEmpty 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 7y agoIn this case, it should absolutely do it. Take a look: https://godbolt.org/z/DmeYL- https://godbolt.org/z/DmeYL-
- dllthomas 7y agoI'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.
- mwkaufma 7y agoI 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 7y agoDoesn'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 7y agoIt does, however, work fine if you use a proper "* (Badge<T>* )&stub" cast.
- GrayShade 7y agoIsn't casting unrelated types undefined behavior?
- brianush1 7y agoYes, 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.
- deleted 7y ago[deleted]
- ryanianian 7y agoHow 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 7y agoYes, this has various names. "Ticket" is traditional.
- kazinator 7y agoC++ 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 7y agoYour 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
- nly 7y agoPersonally 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 7y agoThe 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 7y agoYes, 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.
- gowld 7y ago> 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 7y agoI 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.
- Something1234 7y agoWouldn'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.
- jandrewrogers 7y agoThere are attributes for that, standardized as of C++17 and available in most compilers much earlier. -Wextra won't yell about anything.
- akling 7y agoIf 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) { ... }
- QuadrupleA 7y agoI 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).
- picacho 7y agoyeah, python LOC > 5000 try to debugging it while some dude just modified your type instance's guts.
- eps 7y agoOP's approach works if all dudes involved read the code before chaning it and stay off private parts even if they aren't guarded by the language constructs.
- YayamiOmate 7y agoSo it's an implicit contract based on syntax vs explicit? It's a user/m_ prefix vs compiler/private. Im not sure if former is better. It's a strong trade off imho. In my experience relying on people reading code and agreeing on imlicit contracts does not scale beyond 5 people, but maybe I've been mistreated by life.
- henning 7y agoThis 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 7y agoWhich popular programming languages provide proper ACLs (or equivalent functionality) for class members?
- inetknght 7y agoI'm aware of C++ and Xojo for sure.
- desdiv 7y agoIs 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 7y agoIt'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.
- deleted 7y ago[deleted]
- jmpeax 7y agoConsidering the whole problem could easily be solved with a comment: // should only be called by Device
- cgrealy 7y agoWhile 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.
- ksherlock 7y agoprotected and friend are both smelly keywords?
- dearrifling 7y agoSome people argue exactly that. I don't necessarily agree.
- Toine 7y agoSometimes, 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 7y agoIs 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.
- jupp0r 7y agoThis 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 7y agoI 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.
- choppaface 7y agoIsn’t this exactly the problem that categories in Objective C solve? Why not adopt that pattern versus this “Badge” thing?
- akling 7y agoHello 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 7y agoI 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 7y agoYeah, 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.)
- nercht12 7y agoSeems 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 7y agoCode 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.
- skocznymroczny 7y agoIn 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 7y agoMaybe C++ could add this feature at some point now that it too will get modules.
- AstralStorm 7y agoThis 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.
- mkettn 7y agowhats 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 7y agoHmm, 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?