C++ is bad: structs aren’t the same as structs
Here’s a fun little problem. Suppose you’ve got a system that makes use of the following struct:
struct User {
unsigned int user_id;
unsigned short access_level;
float account_balance; // stored as US dollars
}
Your system is ridiculously well-tested, all the tests pass, everything works totally fine. I’ll be so bold as to say it’s (kinda) bug-free.
One day, you decide to add a new field to the User struct, a string called name. You recompile everything, and suddenly all kinds of tests fail! New users have all kinds of access levels, some have account balances in the billions while others have accounts worth negative billions. What happened, why, and how do you fix it (without removing name again)? Edit: and no, the code does not rely on brittle assumptions about the value of sizeof(User), nor does it break type safety and have unsafe casts or void*’s pointing into the middle of Users. That would be too easy.
Think a little longer here; this is a tricky one and you really need to know C++ well to figure it out.
Alright, let’s see how you did. For starters, how would you describe the symptom succinctly? I’d say that the data for new users is not getting initialized properly. So, let’s look at where new user data gets initialized:
User* new_user = new User;
new_user->user_id = GenerateNewUserID();
if (CreatingAdministratorAccount())
new_user->access_level = Privileges::ROOT;
else if (UserShouldHaveElevatedPrivileges())
new_user->access_level = Privileges::TRUSTED;
InsertIntoUserPool(new_user); // Takes ownership of new_user
This code ought to be setting off flashing red warning sirens for you. The account_balance and (often) the access_level aren’t getting initialized at all! How did this code even work in the first place!?
The trick lies in the definition of User: it contains only built-in types (ints and floats). This is what’s known as Plain Old Data (POD) (for the full definition, see section 9.0 of the C++ Standard). If something is POD, C++ will “helpfully” autogenerate a default constructor that initializes all the elements to 0 (or 0.0, or false, or NULL, or whatever). “but,” I hear someone say, “this is a C-style struct instead of a class. You shouldn’t have a constructor at all!” Well, no. Structs are almost identical to classes. The only difference is that by default everything in a struct is public while everything in a class is private. Yes, this means you can have private and static members in your struct, or even inheritance and virtual functions if you want. and yes, this means that the definition of User above could have been a class instead of a struct (and it would still be POD). I chose structs because by convention they denote data without complicated member functions or inheritance or other object-oriented stuff (hearkening back to structs in C).
Getting back on track, although the autogenerated default constructor for classes(/structs) usually leaves everything uninitialized, if you’re constructing POD, the default constructor will actually initialize everything to 0. This is not mentioned by Stroustrup as far as I’m aware, so you probably haven’t come across this unless you delve into section 8.5.0 of the C++ Standard.
To clarify this idea, consider the following lines of code:
User user_a = { };
User user_b = User();
User* user_c = new User;
User user_d;
User user_e();
Do you know what each line does?
Here’s what’s happening:
user_awill be initialized to all 0’s from its default constructor because it is POD.user_bwill be initialized to all 0’s from its default constructor because it is POD.user_cwill point to aUserthat is initialized to all 0’s from its default constructor because it is POD.user_dwill have uninitialized/undefined values in its data. Even though it is POD, the default constructor was not called (note that there is no sign of initialization or construction on the line, either from an equals sign or from paretheses). Furthermore, note that if we had written our own default constructor instead of relying on the implicit one, it would actually have been called. This is an inconsistency in C++ that I don’t fully understand. (Edit: by declaring a user-defined constructor, it ceases to be POD, so the rules are totally different.)user_eis a declared but undefined function that takes no arguments and returns aUser. I have a whole rant on how this is confusing syntax that trips people up, but I’ll save that for another time.
So, that’s why the data was being zero-initialized to begin with.
When we add a string member, the User struct(/class) is suddenly no longer POD because strings are (non-built-in) objects! Sure, they typically come pre-installed with any compiler you have, but they’re not strictly built into C++ the way ints and floats are. So now that the struct is not POD, its autogenerated default constructor no longer zero-initializes anything, and you have uninitialized data running around.
The way to fix this is to never use the autogenerated default constructor and always write your own, even if all it does is set everything to 0. Yes, it’s (somewhat, kinda, not-really) redundant code, but it will save your ass when someone adds a string to your struct later on. The other way is to always assume that the autogenerated default constructor doesn’t zero-initialize anything, because even though it currently does it might not in the future.
but the real take-away here is that the behavior of classes(/structs) can change depending on the types of its member data in ways that are totally unintuitive. This is another way that C++ breaks the Principle of Least Surprise, and I think it’s a bad language because of this. Thankfully, modern languages like Java and Python automatically initialize everything to 0 (or 0.0 or False or None or whatever) no matter what, so they don’t need to deal with shenanigans like this.
Bonus question (independent of C++): storing the account_balance as a float (or even as a double!) is a terrible/dangerous idea; do you know why?
An interesting quirk of C++… Vewwy interwesting.
A question, though. You obviously know I’m new to C++, so this might be really basic, but what does the line:
new_user.access_level = Privileges::ROOT;
mean? Specifically the Priviliges::ROOT part. I’ve seen the :: when you’re defining a member function of a class outside of its declaration (I think that made sense) but I’m not quite sure what’s going on here.
As for the
account_balancething, I’ve no idea. It can’t be a precision or size thing. Does it have to do with the way you might withdraw or deposit money and things getting out of sync? Yeah, I don’t know…It’s referring to the ROOT object/data in the Privileges class/namespace. By convention, things in UPPERCASE are constants, and we already know it’s an unsigned short. I didn’t bother showing you the Privileges class, but I had intended the constants in it to be bit fields.
The :: notation is for getting at static (not-bound-to-an-instance) class data. If you’ve seen Java’s Math.Pi, this is the same thing, just a different syntax.
First basic rule of C++: ninety percent of “good coding practice” is to keep you from shooting yourself in the foot.
Heh ISO floating point…
As someone who has had this drilled into his head at Mudd, I’m not sure if I count as allowed to comment, but hey, I haven’t seen any other responses so far.
It almost gives it away, but the reason you should never, ever store data that requires precision – like financial info – is because, well, ISO floating-point (i.e. the float or double types in C, C++, Java, Python, or pretty much any language) is imprecise by design. It’s not possible to have arbitrary-precision numbers in POD like floats or doubles, because to have arbitrary precision you need an arbitrary number of bytes. Floating-point fudges this by storing a floating-point number encoded in fractional powers of two, which means that only numbers that are sums of integer powers of two (and fit within a certain range) are stored precisely, and such numbers are still limited by the number of available bytes. This also means that addition and multiplication aren’t commutative, for obvious reasons.
Hence you could have an Office Space type problem, where adding $1 to someone’s account balance ends up with a few extra fractional cents in their account, so you funnel the extra into a bank account owned by you and your disgruntled work buddies…
-Toli
Re: Heh ISO floating point…
Precisely! Using floating point numbers for money can land you in federal pound-me-in-the-ass prison. Addition and subtraction are noncommutative, which can lead to catastrophic cancellation (add a million dollars, add one cent, then subtract a million dollars, and you’re left with nothing) and round-off errors when calculating exchange rates can cause similar woes (convert from dollars to euros to dollars again, and the balance is different). On top of that, doubles take up 64 bits in RAM but 80 bits in the FPU (see “extended precision”), which means that floating point comparison can be nondeterministic (depending on what else needs the FPU and what optimizations the compiler is trying, there isn’t a good way to know how many bits of precision a given double will have).
If you need to work with money, store it as an integer to ensure that addition, multiplication, and comparisons work correctly. If you need to deal with non-integer amounts of money, just make your units sufficiently small (for instance, you could store everything in microcents). The important part is to ensure that you will neither create nor destroy money as you process it.
Re: Heh ISO floating point…
Wait, I think floating point operations are still commutative. However, they’re not associative, which is what leads to the problems.
Re: Heh ISO floating point…
You may be right about that. I believe ISO actually specifies commutativity for floating point ops, though associativity is definitely out.
Re: Heh ISO floating point…
I’m pretty sure floating point subtraction isn’t commutative. :)
So the real source of the problems sums up to never trusting the compiler to zero-out your memory. This is always the case for any POD stack variable. I don’t remember the C compiler ever guaranteeing that a new struct would be all zeroed and always wrote code to explicitly zero them when created, or using a custom malloc() to automatically clear it.
Do you know if this is part of the C++ spec, or a revision to the C spec, or something that is a compiler-dependent feature?
It’s definitely in the C++ spec (section 8.5.0, as described above); I don’t know if it’s in the C spec, too.
Looking into this further, this is actually part of C. From the C spec, section 6.7.9:
Trying this out with the examples I had used above,
user_a,user_d, anduser_ehave the same behavior in C and C++ (user_banduser_care not valid C). So, C++’s POD rules are analogous to C’s rules for similar situations.