Wierd Map Behavior

[subaruwrx88011]

I am very new to c++ and very new to maps. Below is the source of my
program. The output is very strange. Am I inserting correctly into the
map? Any help will be greatly appreciated.

#include <string>
#include <iostream>
#include <map>
#include <sstream>

class CCSI_Env
{

public:


int setItemState(std::string item, std::string state);

void print();

private:

struct StateStruct
{
std::string type;
union
{
int m_integer;
char* m_string;
float m_float;
} stateUnion;
};

//Map with string for the key and StateType for Value
typedef std::map<std::string, StateStruct> ItemStateMap;

//Item/State value pair of the enviroment
static ItemStateMap m_itemMap;

};

CCSI_Env::ItemStateMap CCSI_Env::m_itemMap = ItemStateMap();


int CCSI_Env::setItemState(std::string p_item, std::string p_state)
{
StateStruct itemState;

//Store state as union
itemState.type = "STRING";
itemState.stateUnion.m_string = (char*)p_state.c_str();//Convert to
char*

//Store item with state in map
m_itemMap[p_item] = itemState;


return 0;
}

void CCSI_Env:rint()
{
ItemStateMap::iterator mapIterator;
std::string message;

std::cout<<"******"<<std::endl;

for(mapIterator = m_itemMap.begin(); mapIterator != m_itemMap.end();
mapIterator++)
{
if(mapIterator->second.type == "STRING")
{
message = mapIterator->first + " = " +
mapIterator->second.stateUnion.m_string;
std::cout << message << std::endl;
}

}

std::cout<<"******"<<std::endl;

}





int main()
{
CCSI_Env m_env;
int status;
std::string s,t;



s = "ONE";
t = "Online";
status = m_env.setItemState(s,t);

m_env.print();

s = "TWO";
t = "Online";
status = m_env.setItemState(s,t);

m_env.print();

s = "THREE";
t = "Offline";
status = m_env.setItemState(s,t);

m_env.print();

s = "FOUR";
t = "Offline";
status = m_env.setItemState(s,t);

m_env.print();
}

The output should be:
******
ONE = Online
******
******
ONE = Online
TWO = Online
******
******
ONE = Online
THREE = Offline
TWO = Online
******
******
FOUR = Offline
ONE = Online
THREE = Offline
TWO = Online
******

 

[Victor Bazarov]

I am very new to c++ and very new to maps. Below is the source of my
program. The output is very strange. Am I inserting correctly into the
map? Any help will be greatly appreciated.

[...]
int CCSI_Env::setItemState(std::string p_item, std::string p_state)

'p_state' is a temporary variable that exists only while this function
executes. As soon as this function returns, 'p_state' does not exist
any more.

{
StateStruct itemState;

//Store state as union
itemState.type = "STRING";
itemState.stateUnion.m_string = (char*)p_state.c_str();//Convert to
char*

Here you extract some value from the temporary object and hang onto it.
As soon as this function exits, the pointer you extracted _is_invalid_.

What else you expect? It's UB to use that pointer _outside_ of this
function.

And FCOL don't use any casts like that above. Rewrite your 'StateStruct'
to use std::string instead of a char pointer.

[...]

V

 

[I V]

I am very new to c++ and very new to maps. Below is the source of my
program. The output is very strange. Am I inserting correctly into the
map? Any help will be greatly appreciated.

The problem is not your use of map, it is this line:

itemState.stateUnion.m_string = (char*)p_state.c_str();

c_str() returns a pointer to an internal buffer of the std::string
object, p_state in this case. But p_state is destroyed when you return
from the setItemState function. So the pointer in your StateStruct now
points to some random region of memory. Accessing this is undefined
behaviour (it could print some random string, which is what it does on
my computer, or it could crash, or it could do anything else).

In general, don't user c_str() when you will need to access the
returned char* at some later date, because c_str() returns a pointer
which can be invalidated when you call a non-const member function of
the string (and which is invalidated when the string is destroyed, of
course).

The easiest solution is to store a std::string, rather than a char*, in
your StateStruct. You can't put a std::string in a union, but you
should probably just get rid of the union; the wasted space is unlikely
to matter unless you have millions of StateStruct objects. If you find
that you actually do need to save that space, consider using the
variant type from http://www.boost.org/ (which is like a smarter,
typesafe, union).

 

[subaruwrx88011]

And FCOL don't use any casts like that above. Rewrite your 'StateStruct'
to use std::string instead of a char pointer.

The compiler complains about using a string inside a union.

test.cpp:24: error: member `std::string
CCSI_Env::StateStruct::<anonymous
union>::m_string' with constructor not allowed in union

 

[Victor Bazarov]

The compiler complains about using a string inside a union.

Don't use unions, then.

V

 

[subaruwrx88011]

The easiest solution is to store a std::string, rather than a char*, in
your StateStruct. You can't put a std::string in a union, but you
should probably just get rid of the union; the wasted space is unlikely
to matter unless you have millions of StateStruct objects. If you find
that you actually do need to save that space, consider using the
variant type from http://www.boost.org/ (which is like a smarter,
typesafe, union).

Hey, thanks man. That worked perfectly. Appreciate it.