By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
459,905 Members | 1,585 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 459,905 IT Pros & Developers. It's quick & easy.

Wierd Map Behavior

P: n/a
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::print()
{
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
******

Feb 20 '06 #1
Share this Question
Share on Google+
5 Replies


P: n/a
subaruwrx88011 wrote:
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
--
Please remove capital As from my address when replying by mail
Feb 20 '06 #2

P: n/a
I V
subaruwrx88011 wrote:
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).

Feb 20 '06 #3

P: n/a

Victor Bazarov wrote:
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

Feb 20 '06 #4

P: n/a
subaruwrx88011 wrote:
Victor Bazarov wrote:
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.


Don't use unions, then.

V
--
Please remove capital As from my address when replying by mail
Feb 20 '06 #5

P: n/a

I V wrote:
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.

Feb 20 '06 #6

This discussion thread is closed

Replies have been disabled for this discussion.