C++ nullpointer dereference when adding instance to std::map within the constructor

Question

I am trying to get an error-code list for my C++ application, so that errors are shared across multiple projects. Each error contains simply a unique code (to give to exit() and similars) and a message to display. Other than a constructor and some casting methods, I also need some function to get an error by its code, so that when program A exits I can read the return code from program B and display it to the user/log/whatever. This implies, obviously, keeping track of the whole error list. To do so, i keep a static std::unordered_map inside the Error class, so that the Error constructor adds the being-created instance to it. However, for some reason I can't quite understand, I keep getting a nullpointer dereference error when adding the Error to the map. My code is as follows:

error_codes.h

namespace ErrorCodes{
    class Error{
    private:
        int code;
        std::string message;

        static int counter;

        static std::unordered_map<int, Error*> allErrors;

        Error(int code, std::string message);

    public:
        Error(): code(1), message(""){}

        explicit Error(std::string message);

        Error getByCode(int code);

        operator int() const{
            return code;
        }

        operator std::string() const{
            return message;
        }

        std::ostream& operator<<(std::ostream& os)
        {
            os << std::string("ERR_")+std::to_string(code)+std::string(": ") + message;
            return os;
        }

        operator QString() const{
            return QString::fromStdString(message);
        }

        bool operator == (Error other){
            return other.code == code;
        }


    };

    //List all errors here
    const Error ERR_ERROR_CODE_NOT_FOUND("Couldn\'t find error with code %1"); //Follow QString formatting style for more complex messages
    const Error ERR_GLOBAL_ID_NOT_FOUND("Problem with the global Id, identifier not found.");
    const Error ERR_DIR_NOT_FOUND_OR_CREATED("Dir not found or created, check configuration.");
}

and then my error_codes.cpp:

using namespace ErrorCodes;

int Error::counter = 0;
std::unordered_map<int, Error*> Error::allErrors = std::unordered_map<int, Error*>();

Error::Error(int code, std::string message) : code(code), message(message){
    std::pair pair = std::make_pair(code, this);
    allErrors.emplace(pair); //It fails here
}

Error::Error(std::string message){
    int code = --counter;
    Error(code, message);
}

Error Error::getByCode(int code){
    auto it = allErrors.find(code);
    if(it!=allErrors.end()){
        return *(it->second);
    }else{
        qCritical() << QString(ERR_ERROR_CODE_NOT_FOUND).arg(code);
        exit(ERR_ERROR_CODE_NOT_FOUND);
    }
}

PS: Ideally, I wouldn't make Error a class, but rather a struct, and have some kind of collection (such as an array) that contains all the errors and will allow me to retrieve them by the error code. However, I did not find a way of having the errors available within an array AND as a "constant" (so that anywhere in my code I can just write ErrorCodes::XXX) without having to manually create an array with all the Errors (for example, an enum would be perfect for this purpose). I would be grateful if anyone else knew a better way to do this.

Answer 1

Currently, you create Error instances before the unordered_map is initialized. Initialize the static variables before using them. Example:

Header:

namespace ErrorCodes {
// class definition

// List all errors here
extern const Error ERR_ERROR_CODE_NOT_FOUND;
extern const Error ERR_GLOBAL_ID_NOT_FOUND;
extern const Error ERR_DIR_NOT_FOUND_OR_CREATED;
}  // namespace ErrorCodes

.cpp file:

namespace ErrorCodes {

// Initialize the static variables:
int Error::counter = 0;
std::unordered_map<int, Error*> Error::allErrors{};

// Create all the errors afterwards:
const Error ERR_ERROR_CODE_NOT_FOUND("Couldn\'t find error with code %1");
const Error ERR_GLOBAL_ID_NOT_FOUND("Problem with the global Id, identifier not found.");
const Error ERR_DIR_NOT_FOUND_OR_CREATED("Dir not found or created, check configuration.");
//...
}  // namespace ErrorCodes

Also note that constructor delegation needs to use the member initializer list.

Example:

Error::Error(int code, std::string message) : code(code), message(std::move(message)) {
    allErrors.emplace(code, this);  // It doesn't fail here anymore
}

Error::Error(std::string message) : Error(--counter, std::move(message)) {}

---

Alternatively, define both the static variables and constants inline.

Header:

namespace ErrorCodes {
class Error {
private:
    inline static int counter = 0;
    inline static std::unordered_map<int, Error*> allErrors{};

    //...
};

// List all errors here
inline const Error ERR_ERROR_CODE_NOT_FOUND("Couldn\'t find error with code %1");
inline const Error ERR_GLOBAL_ID_NOT_FOUND("Problem with the global Id, identifier not found.");
inline const Error ERR_DIR_NOT_FOUND_OR_CREATED("Dir not found or created, check configuration.");

}  // namespace ErrorCodes

Answer 2

Seems like you're using the wrong delegate constructor syntax

Error::Error(std::string message){
    int code = --counter;
    Error(code, message); //temporary
}

Should be

Error::Error(std::string message) : Error(--counter, message) {}

Otherwise, you're constructing a temporary Error which only exists in the stack frame of the delegate constructor, when the constructor is exited, the "this" pointer of the temporary error points to an invalid area of memory.

Error::Error(int code, std::string message) : code(code), message(message){
    std::pair pair = std::make_pair(code, this); // <- as soon as the function exits, "this" will be an invalid pointer
    allErrors.emplace(pair);
}

I'm not exactly sure why with only this code you get the segmentation fault directly rather than getting it when trying to dereference an error from the map but it's a good place to start.