Segmentation fault / using std::map

Hello. I think I have pretty much isolated my problem though I do not understand it. I am trying to do a really simple class to get information from a configuration file and store those settings. I guess it is some kind of basic finite state machine. I am currently trying to make it work with my debug class but for some reason the condition that checks if "logfile.name" is set always returns true, causing a failed attempt to open file and for some reason causes a segmentation fault. All code in main() is commented and it is the only piece of code that actually runs at this moment. I don't think the file opening class is causing any trouble here as I have opened files with it before without trouble so I will not include its code here.

What I'm pretty sure of is that my code inside IsStateSet() is wrong, but I do not know really how to check for empty strings using maps. I had return states[state] != "" before but it caused the very same issue.

Code:

---

//
// system.cpp
//


#include "system.h"
#include "iomanagement.h"

//*****************************
// Error logging implementation
//*****************************

#ifdef DEBUG_MODE
    C_DBG_LOGGER::C_DBG_LOGGER() {
        if(MASTER_CONFIG.IsStateSet("logfile.name")) {
            output.OpenFile(MASTER_CONFIG.ReadState("logfile.name"));
        } else {
            output.OpenFile("dbg_log.txt");
        }
    }

    void C_DBG_LOGGER::Write(const char* msg) {
        if(output.IsFileOpen()) {
            output.WriteToFile(msg);
        }
    }

    C_DBG_LOGGER DBG_LOGGER;
#endif // DEBUG_MODE


//************************************
// Finite state machine implementation
//************************************

FSMachine::FSMachine() {

}

bool FSMachine::IsStateSet(std::string state) {
    return states[state].size() > 5; // (minimum is x.xxx, therefore 5)
}

void FSMachine::SetState(std::string state, std::string& value) {
    states[state] = value;
}

std::string FSMachine::ReadState(std::string state) {
    return states[state];
}

FSMachine MASTER_CONFIG;

---

Code:

---

//
// system.h
//

#ifndef SYSTEM_H
#define        SYSTEM_H

#define DEBUG_MODE

#include <map>
#include <string>

//*********************************************
// Setup the general macro that will be used to
// write errors to log file. Only define it if
// debugging is desired.
//*********************************************

#ifdef DEBUG_MODE
    #include "iomanagement.h"

    class C_DBG_LOGGER {
    private:
        Writer output;
    public:
        C_DBG_LOGGER();

        void Write(const char*);
    };

    extern C_DBG_LOGGER DBG_LOGGER;

    #define DIE(a) DBG_LOGGER.Write(a)
#else
    #define DIE(a) //..do nothing
#endif // DEBUG_MODE


//***************************
// Basic finite state machine
//***************************

class FSMachine {
private:
    std::map<std::string, std::string> states;
public:
    FSMachine();

    bool IsStateSet(std::string);
    void SetState(std::string, std::string& value);
    std::string ReadState(std::string);
};

extern FSMachine MASTER_CONFIG;

#endif        // SYSTEM_H

---

Any insight is very much welcome.

Note: Of course I tried using the debugger but for some reason it is not working (error on start). I am looking into that right now.

Thanks.

I found the answer to my debugger problem on the Netbeans forum, but for some reason the debugger will not break on any breakpoint except on the return 0; in main().

:/

Edit: Forgot to mention that I am using Netbeans using g++ with -wall on Ubuntu (11.04 I think).

Am not sure on the map thing either but i would in any case probably have written this with a flag variable to indicate if the state is set or not, i dont know about your full implementation plan or whatever but you could workaround:

Code:

---

class FSMachine {

    //your class stuff

    bool is_set;

    //..
};


//...


void FSMachine::SetState(std::string state, std::string& value)
{
   
    if(value.length())
    {
        states[state] = value;
        is_set = true;
    }
    else
    {
        //is_set = false??
        //whatever is required
        //maybe have this function return a value for error checks?
    }
}

---

I have made a slight modification to account for your suggestion but I get the same result. I will try using only the code for the FSMachine class in a separate empty project to test it thoroughly.

Code:

---

FSMachine::FSMachine() {

}

bool FSMachine::IsStateSet(std::string state) {
    return states[state].second;
}

void FSMachine::SetState(std::string state, std::string& value) {
    states[state] = std::make_pair<std::string, bool>(value, true);
}

std::string FSMachine::ReadState(std::string state) {
    return states[state].first;
}

---

Code:

---

class FSMachine {
private:
    std::map<std::string, std::pair<std::string, bool> > states;
public:
    FSMachine();

    bool IsStateSet(std::string);
    void SetState(std::string, std::string& value);
    std::string ReadState(std::string);
};

extern FSMachine MASTER_CONFIG;

---

On a stylistic side, you've misunderstood what a finite state machine is. Your class isn't one.

On a correctness side, the problem is that DBG_LOGGER is defined before MASTER_CONFIG in the file. This means that its constructor is called first, and thus it accesses MASTER_CONFIG, which *has not yet been initialized*! This is obviously completely invalid and can only cause trouble.

The underlying problem is that you have too many globals.

I have also tried using map::find in the following fashion with the same results.

Code:

---

bool FSMachine::IsStateSet(std::string state) {
    return (states.find(state) == states.end());
}

---

It seems that it will still return true even though I have not set any key at all and if I just change it to return 0 or return false the program runs fine (but of course it does not "work" in the way that I want it to).

Thank you I'm looking into that right now. What do you suggest for the globals then ? I'm not usually a fan of them either (particularly because, as this seems to be the case now, they cause trouble). Would namespaces be a good choice ?

No, namespaces won't help. What you can do to fix your current problem is put the definition of MASTER_CONFIG above that of DBG_LOGGER - that should fix the error you're seeing.

Getting rid of globals is really a larger refactoring issue. Having some logging stream as a global isn't even bad. You just shouldn't leave it to the constructor to open the stream. Instead, maybe your main() should open the log file as one of its first actions. For the master config, I would provide a few free or static member functions for querying that ensure the config is correctly initialized when they run.

It turns out that namespaces do not work in that particular situation for probably the same reason as you mentioned. A quick search on Google for "global variables alternative" turns up much hatred and a few design tricks which do not seem to apply to this situation.

Edit: You were right switching them has solved the problem. Thank you very much. I will also change that misleading "FSMachine" name :P