Safety of calling C++ functions with enum created using static_cast

Viewed 165

I have been using enumerations for function input parameters, but I have noticed that this may be very dangerous.

For example:

enum class MYENUM {
    X1 = 0, X2
};

std::map<MYENUM, int> mymap;
//init mymap here with known enum values of X1 and X2


int MyFunc(const MYENUM& input) {
    return mymap.at(input);
}

int main() {
    MyFunc(static_cast<MYENUM>(10000));
}

So 10000 is not a valid enum value. How would you personally find a solution to this? Would you encase the map access in a try block and catch the exception? What if I have many functions which access container information with user input enum values - would you then put try catch in all of them too?

3 Answers

How would you personally find a solution to this?

Easy. One of the reasons for class enums introduction was type-safety - they would not accept any value automatically. So, the code you have here

MyFunc(static_cast<MYENUM>(10000));

Just circumvents the safety which was offered by the language. The solution is simple - do not circumvent the safety by casting, and the compiler will not allow you to use incorrect values.

And if someone else uses the cast - this is not of your problem. There are endless ways someone can bypass built-in type safety and provide completely bogus values to the functions. There is nothing library writer can do about it.

So 10000 is not a valid enum value

That may be true, but not because it's not equal to X1 or X2. An enum doesn't have to hold one of the named enumerators. It can hold any value in its integer range.

Now, the actual range of MYENUM is up to the compiler, as long as the integral underlying type it picks can represent both X1 and X2. So, it could be a int8_t, for example, which cannot hold 10000.

(In practice, it's probably an int here.)

How would you personally find a solution to this? Would you encase the map access in a try block and catch the exception?

Yes, if you like.

There are many approaches to error handling for functions like this, whether they involve enums or not. You can throw an exception (the .at() call will already do this for you, but you could catch it and throw an exception of your own type if you liked).

You could populate an error code, return a placeholder value, return a std::optional… or just ignore it, and document that the user of your function must pass a named member of the enum, or fall foul of a sort of "undefined behaviour" (possibly choosing to have the function act as if X1 were passed in that case, so that it has something to do at least).

All the usual options.

What if I have many functions which access container information with user input enum values - would you then put try catch in all of them too?

That is indeed a possible consideration to bear in mind when choosing your input argument error handling methodology.

There is nothing inherently dangerous about enum and function signatures with enum parameters are not bad practice. In your particular example, 10000 is still a valid enum, just not one that has a name.

This only becomes a problem when looking up in a std::map<MyEnum, Something> or using a switch (myEnum). To mitigate these problems, you could perform a sanity-check for your enum values:

// specify a type for MyEnum explicitly so that we know we can fit values
// such as 100000
enum class MyEnum : unsigned {
    X1 = 0, X2
};

// On a side note, enums are so small in size that it makes no sense to
// pass them by reference, simply pass them by value everywhere.
constexpr bool sanityCheck(MyEnum e) {
    return e == MyEnum::X1 || e == MyEnum::X2;
}

int MyFunc(const MyEnum& input) {
    assert(sanityCheck(input));
    return mymap[input];
}

You could even create an assert function that only compiles on release builds to have zero runtime cost. This will be much more effective than putting a try-catch everywhere.

But the best solution is to simply be careful when you create MyEnum using a static_cast. Don't pass the result into functions accepting MyEnum if you're not 100% sure that the value has a name.

Related