Wierd Map Behavior

Discussion in 'C++' started by subaruwrx88011, Feb 20, 2006.

  1. 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
    ******
     
    subaruwrx88011, Feb 20, 2006
    #1
    1. Advertising

  2. 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
     
    Victor Bazarov, Feb 20, 2006
    #2
    1. Advertising

  3. subaruwrx88011

    I V Guest

    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).
     
    I V, Feb 20, 2006
    #3
  4. 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
     
    subaruwrx88011, Feb 20, 2006
    #4
  5. 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
     
    Victor Bazarov, Feb 20, 2006
    #5
  6. 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.
     
    subaruwrx88011, Feb 20, 2006
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. paul reed

    Wierd Behavior of __doPostBack

    paul reed, Jul 8, 2003, in forum: ASP .Net
    Replies:
    1
    Views:
    573
    Bassel Tabbara [MSFT]
    Jul 8, 2003
  2. Steve

    Wierd datalist behavior

    Steve, Jul 8, 2003, in forum: ASP .Net
    Replies:
    0
    Views:
    335
    Steve
    Jul 8, 2003
  3. =?Utf-8?B?TWljaGFlbCBMb3VnaHJ5?=

    Wierd behavior with text box postback

    =?Utf-8?B?TWljaGFlbCBMb3VnaHJ5?=, Jun 21, 2005, in forum: ASP .Net
    Replies:
    3
    Views:
    572
    =?Utf-8?B?TWljaGFlbCBMb3VnaHJ5?=
    Jun 22, 2005
  4. Qun Cao

    wierd threading behavior

    Qun Cao, Oct 15, 2005, in forum: Python
    Replies:
    6
    Views:
    297
    dcrespo
    Oct 18, 2005
  5. newbiegalore

    wierd behavior in program

    newbiegalore, Feb 23, 2008, in forum: C Programming
    Replies:
    4
    Views:
    342
    newbiegalore
    Feb 23, 2008
Loading...

Share This Page