STL Map Scoping Issue

Discussion in 'C++' started by sfncook@gmail.com, Aug 12, 2007.

  1. Guest

    I have some code that needs to declare an STL Map in the static scope,
    insert values into the map in the function of one class and access the
    values in the functions of other classes. This seems like a pretty
    straight forward request, but when I implement it I get either garbage
    data when I try to access it or I get an Access Violation runtime
    error (depending on the state of my code while I try to figure out how
    to get this rediculously simple idea to work). What the heck?! What
    am I doing wrong? Here is some sample code that gives an Access
    Violation:

    typedef struct _ShipType {
    int a;
    int b;
    int c;
    } ShipType;

    static std::map<int, ShipType *> shipTypeMap;

    void GameLoader::LoadGame()
    {
    shipTypeMap[0] = new ShipType();
    shipTypeMap[0]->a = 1;
    shipTypeMap[0]->b = 2;
    shipTypeMap[0]->c = 3;
    }

    int main(int argc, char* argv[])
    {
    printf("Callback function test.\n");

    GameLoader * gl = new GameLoader();

    gl->LoadGame();

    std::cout<<shipTypeMap[0]->a<<std::endl;//<-----0xC000000005 Access
    Violation here

    delete shipTypeMap[0];

    getchar();

    return 0;
    }
     
    , Aug 12, 2007
    #1
    1. Advertising

  2. Hi!

    So far I have no idea what is causing the AV.

    schrieb:
    > delete shipTypeMap[0];


    At least you have undefined behaviour here: deleted pointers are
    invalid. invalid pointers may not be read or copied, but only be
    assigned to or destructed (when going out of scope). But elements of a
    map must be copyable. So you should first remove the pointer from the
    map (store it in a local variable) and then delete the object it points to.

    To avoid this hassle try using boost::shared_ptr.

    Frank
     
    Frank Birbacher, Aug 12, 2007
    #2
    1. Advertising

  3. wrote:
    > I have some code that needs to declare an STL Map in the static scope,
    > insert values into the map in the function of one class and access the
    > values in the functions of other classes. This seems like a pretty
    > straight forward request, but when I implement it I get either garbage
    > data when I try to access it or I get an Access Violation runtime
    > error (depending on the state of my code while I try to figure out how
    > to get this rediculously simple idea to work). What the heck?! What
    > am I doing wrong? Here is some sample code that gives an Access
    > Violation:
    >
    > typedef struct _ShipType {
    > int a;
    > int b;
    > int c;
    > } ShipType;
    >
    > static std::map<int, ShipType *> shipTypeMap;

    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    I can only think that you're seeing this in more than one compilation unit.
    >
    > void GameLoader::LoadGame()
    > {
    > shipTypeMap[0] = new ShipType();
    > shipTypeMap[0]->a = 1;
    > shipTypeMap[0]->b = 2;
    > shipTypeMap[0]->c = 3;
    > }
    >
    > int main(int argc, char* argv[])
    > {
    > printf("Callback function test.\n");
    >
    > GameLoader * gl = new GameLoader();
    >
    > gl->LoadGame();
    >
    > std::cout<<shipTypeMap[0]->a<<std::endl;//<-----0xC000000005 Access
    > Violation here
    >
    > delete shipTypeMap[0];
    >
    > getchar();
    >
    > return 0;
    > }
    >


    The code below compiles and runs as expected.


    #include <map>
    #include <iostream>

    typedef struct _ShipType {
    int a;
    int b;
    int c;
    } ShipType;

    static std::map<int, ShipType *> shipTypeMap;

    struct GameLoader
    {
    void LoadGame();
    };

    void GameLoader::LoadGame()
    {
    shipTypeMap[0] = new ShipType();
    shipTypeMap[0]->a = 1;
    shipTypeMap[0]->b = 2;
    shipTypeMap[0]->c = 3;
    }

    int main(int argc, char* argv[])
    {
    printf("Callback function test.\n");

    GameLoader * gl = new GameLoader();

    gl->LoadGame();

    std::cout<<shipTypeMap[0]->a<<std::endl;

    delete shipTypeMap[0];

    getchar();

    return 0;
    }
     
    Gianni Mariani, Aug 12, 2007
    #3
  4. Hi!

    I ran the following code. It doesn't crash on my system. I can't think
    of a reason why it should crash at the cout line. Anyway, I don't
    understand why you need static storage.

    #include <iostream>
    #include <ostream>
    #include <map>
    #include <stdio.h>

    struct GameLoader
    {
    void LoadGame();
    };

    typedef struct _ShipType {
    int a;
    int b;
    int c;
    } ShipType;

    static std::map<int, ShipType *> shipTypeMap;

    void GameLoader::LoadGame()
    {
    shipTypeMap[0] = new ShipType();
    shipTypeMap[0]->a = 1;
    shipTypeMap[0]->b = 2;
    shipTypeMap[0]->c = 3;
    }

    int main(int argc, char* argv[])
    {
    printf("Callback function test.\n");

    GameLoader * gl = new GameLoader();

    gl->LoadGame();

    std::cout<<shipTypeMap[0]->a<<std::endl;//<-----0xC000000005
    AccessViolation here

    delete shipTypeMap[0];

    getchar();

    return 0;
    }

    Frank
     
    Frank Birbacher, Aug 12, 2007
    #4
  5. On Sun, 12 Aug 2007 09:38:40 +0200, Frank Birbacher wrote:
    > schrieb:
    >> delete shipTypeMap[0];

    >
    > At least you have undefined behaviour here: deleted pointers are
    > invalid. invalid pointers may not be read or copied, but only be
    > assigned to or destructed (when going out of scope). But elements of a
    > map must be copyable. So you should first remove the pointer from the
    > map (store it in a local variable) and then delete the object it points
    > to.


    Must say I did not realise that until now. That is unbelievably ugly.

    Using the following should be safe though:

    template<class T> void deleteAndZero(T *&p)
    {
    delete p;
    p = NULL;
    }

    --
    Markus Schoder
     
    Markus Schoder, Aug 12, 2007
    #5
  6. James Kanze Guest

    On Aug 12, 9:38 am, Frank Birbacher <> wrote:

    > So far I have no idea what is causing the AV.


    > schrieb:


    > > delete shipTypeMap[0];


    > At least you have undefined behaviour here: deleted pointers are
    > invalid. invalid pointers may not be read or copied, but only be
    > assigned to or destructed (when going out of scope). But elements of a
    > map must be copyable. So you should first remove the pointer from the
    > map (store it in a local variable) and then delete the object it points to.


    That's in theory. In practice, of course, there's no problem on
    any real architecture; no implementation of std::map will read
    or copy the pointer unless you actually try to access it.

    > To avoid this hassle try using boost::shared_ptr.


    Sounds like added complexity for no gain.

    --
    James Kanze (GABI Software) email:
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
     
    James Kanze, Aug 12, 2007
    #6
  7. Guest

    Wow! You guys are great! Thanks for the responses so far. However, I
    forgot to mention one factor that must make the difference: The code
    is broken up into different files. I copied the code you had and ran
    it fine from a single file as well. However when I divide the code
    into different files (as is below) then I get the AV. Thanks a
    million for the help so far. Please teach me oh wise sages.

    ********* typedefs.h *****************
    #include "stdlib.h"
    #include <map>
    #include <vector>

    typedef struct _ShipType {
    int a;
    int b;
    int c;
    } ShipType;

    static std::map<int, ShipType *> shipTypeMap;


    ************** GameLoader.h ********************
    #pragma warning (disable : 4786) //This is a fix for a Microsoft
    acknowledged bug.

    #include "typedefs.h"

    class GameLoader
    {
    private:
    public:
    GameLoader(){};
    virtual ~GameLoader(){};
    void LoadGame();
    };

    ************ GameLoader.cpp *********************
    #include "GameLoader.h"

    void GameLoader::LoadGame()
    {
    shipTypeMap[0] = new ShipType();
    shipTypeMap[0]->a = 1;
    shipTypeMap[0]->b = 2;
    shipTypeMap[0]->c = 3;
    }


    ****************** MainFile.cpp *********************
    #include "GameLoader.h"

    #include <stdlib.h>
    #include <iostream>
    #include <map>

    int main(int argc, char* argv[])
    {
    printf("Callback function test.\n");

    GameLoader * gl = new GameLoader();

    gl->LoadGame();

    std::cout<<shipTypeMap[0]->a<<std::endl;

    delete shipTypeMap[0];

    getchar();

    return 0;
    }

    ^^^^^^^^^ FIN ^^^^^^^^^^^^^^^
     
    , Aug 12, 2007
    #7
  8. Hi!

    schrieb:
    > Wow! You guys are great! Thanks for the responses so far. However, I
    > forgot to mention one factor that must make the difference: The code
    > is broken up into different files.


    Indeed that is the difference, as Gianni already pointed out. :)

    > ********* typedefs.h *****************
    > #include "stdlib.h"
    > #include <map>
    > #include <vector>
    >
    > typedef struct _ShipType {
    > int a;
    > int b;
    > int c;
    > } ShipType;
    >
    > static std::map<int, ShipType *> shipTypeMap;


    This definition of shipTypeMap includes "static". That makes shipTypeMap
    appear mutiple times: once for each translation unit (= cpp file). So
    your initialisation code initialises one instance while the other code
    accesses the uninitialised one => oops. You need to do the following:

    typedefs.h:
    extern std::map<int, ShipType*> shipTypeMap;

    somewhere.cpp:
    std::map<int, ShipType*> shipTypeMap;

    This makes one global instance of shipTypeMap which is then access from
    all code.

    HTH,
    Frank
     
    Frank Birbacher, Aug 12, 2007
    #8
  9. Hi!

    James Kanze schrieb:
    > That's in theory. In practice, of course, there's no problem on
    > any real architecture; no implementation of std::map will read
    > or copy the pointer unless you actually try to access it.


    Yes, no problem in practise.

    >> To avoid this hassle try using boost::shared_ptr.

    >
    > Sounds like added complexity for no gain.


    Sounds like added memory security for little effort as well. Even if it
    is of little help with uninitialised pointers it is of great help for
    managing memory (less leaks) and exception safety.

    Frank
     
    Frank Birbacher, Aug 12, 2007
    #9
  10. Guest

    Why don't I visit this site more often? You guys are great. Thank a
    million.
     
    , Aug 12, 2007
    #10
  11. BobR Guest

    <> wrote in message...
    >


    Just a couple of *suggestions*....
    ( I think Frank nailed your problem.)

    > ********* typedefs.h *****************
    > #include "stdlib.h"


    Is "stdlib.h" a file you wrote? If you intended the standard stdlib, you
    should use:

    #include <stdlib.h>

    .... or the C++ version:

    #include <cstdlib> // you may need to add 'std::' to the 'calls'.

    > #include <map>


    > #include <vector>


    Remove that <vector> header if you are not using it.

    >
    > typedef struct _ShipType {


    An underline followed by an uppercase letter is reserved to implementation.
    I'd use something else there ( like Ship_Type).

    > int a;
    > int b;
    > int c;
    > } ShipType;


    Why not just:

    struct ShipType{
    int a;
    int b;
    int c;
    };

    [snip]

    > #include "typedefs.h"
    >
    > class GameLoader{


    > private:


    Ok if you need the documentation, but, 'class' defaults to 'private', so,
    it's not required.

    > public:
    > GameLoader(){};
    > virtual ~GameLoader(){};
    > void LoadGame();
    > };
    >
    > ************ GameLoader.cpp *********************
    > #include "GameLoader.h"
    >
    > void GameLoader::LoadGame(){
    > shipTypeMap[0] = new ShipType();
    > shipTypeMap[0]->a = 1;
    > shipTypeMap[0]->b = 2;
    > shipTypeMap[0]->c = 3;
    > }


    If you add a constructor to your struct:
    struct ShipType{
    int a;
    int b;
    int c;
    ShipType( int aa, int bb, int cc) : a(aa), b(bb), c(cc){}
    // ShipType() : a(0), b(0), c(0){} // might also want this
    }; // note: no longer a POD type

    .... then you could do:

    void GameLoader::LoadGame(){
    shipTypeMap[0] = new ShipType( 1, 2, 3 );
    }

    >
    > ****************** MainFile.cpp *********************
    > #include "GameLoader.h"
    >
    > #include <stdlib.h>
    > #include <iostream>
    > #include <map>
    >
    > int main(int argc, char* argv[])


    If you are not using the command line args:

    int main()

    > {
    > printf("Callback function test.\n");
    > GameLoader * gl = new GameLoader();
    > gl->LoadGame();
    > std::cout<<shipTypeMap[0]->a<<std::endl;
    > delete shipTypeMap[0];
    > getchar();
    > return 0;
    > }


    --
    Bob R
    POVrookie
     
    BobR, Aug 12, 2007
    #11
  12. Guest

    Thank you! Very constructive comments that I will take to heart.
     
    , Aug 12, 2007
    #12
  13. Bo Persson Guest

    Markus Schoder wrote:
    :: On Sun, 12 Aug 2007 09:38:40 +0200, Frank Birbacher wrote:
    ::: schrieb:
    :::: delete shipTypeMap[0];
    :::
    ::: At least you have undefined behaviour here: deleted pointers are
    ::: invalid. invalid pointers may not be read or copied, but only be
    ::: assigned to or destructed (when going out of scope). But elements
    ::: of a map must be copyable. So you should first remove the pointer
    ::: from the map (store it in a local variable) and then delete the
    ::: object it points to.
    ::
    :: Must say I did not realise that until now. That is unbelievably
    :: ugly.

    But that is also the way some hardware work(ed). Loading a pointer
    into an address register might involve verifying access rights and/or
    validity.

    On a segmented system, like Windows 286 with 64 kB per segment,
    deleting an entire segment might make the runtime release the segment
    to to OS, which promptly removed the segment from the descriptor
    table. Now, loading that pointer into a segment register would case a
    missing segment trap.


    This segment hardware is still present in 32 bit x86 processors,
    though we don't use it much. The language rules are there to allow its
    use, if anybody should want to!

    ::
    :: Using the following should be safe though:
    ::
    :: template<class T> void deleteAndZero(T *&p)
    :: {
    :: delete p;
    :: p = NULL;
    :: }


    Yes.


    Bo Persson
     
    Bo Persson, Aug 15, 2007
    #13
    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. Marcus
    Replies:
    2
    Views:
    592
    Marcus
    Dec 9, 2005
  2. Replies:
    2
    Views:
    556
    klaus hoffmann
    Feb 22, 2006
  3. Replies:
    2
    Views:
    677
    Ivan Vecerina
    May 4, 2006
  4. kl
    Replies:
    7
    Views:
    1,291
    James Kanze
    Jan 1, 2008
  5. Luca Risolia

    STL map to STL vector

    Luca Risolia, Jan 13, 2014, in forum: C++
    Replies:
    32
    Views:
    375
    Seungbeom Kim
    Jan 18, 2014
Loading...

Share This Page