0

My code that I have is quite large and complicated so I won't waste your time reading it, but you're going to have to make certain assumtions about variables in it as a result. I will tell you the values of the variables which I have confirmed in the debugger so you know with certainty. Know that I have omitted a lot of unrelated code in here so what you see isn't everything but I have included everything that is relevant.

// This is defined in a class:
char**** m_DataKeys;


// This is in a member function of the same class:
m_DataKeys = new char*** [m_iNumOfHeroes];  // m_iNumOfHeroes = 2

while ( pkvHero )
{
  // iHeroNum = 0 and then 1      #define NUM_OF_ABILITIES 4
  m_DataKeys[iHeroNum] = new char** [NUM_OF_ABILITIES];

  for (int ability = 0; ability < NUM_OF_ABILITIES; ability++)
  {
    if (pkvExtraData)  // only is true when iHeroNum == 1 and ability == 0
    {
      // iNumOfExtraData == 2
      m_DataKeys[iHeroNum][ability] = new char* [iNumOfExtraData];

      while ( pkvSubKey )
      {
        // iCurExtraDataNum increments from 0 to 2
        m_DataKeys[iHeroNum][ability][iCurExtraDataNum] = new char [50];

I put a break point on the line

m_DataKeys[iHeroNum] = new char** [NUM_OF_ABILITIES];

Before the line is called and when iHeroNum == 0 the m_DataKeys array looks like:

m_DataKeys | 0x02072a60
  pointer | 0xffeeffee
    Error : expression cannot be evaluated

Which is expected. After the line gets called it looks like:

m_DataKeys | 0x02072a60
  pointer | 0x02496b00
    pointer | 0xffeeffee
      Error : expression cannot be evaluated

Which seems to look correct. However, since I set a breakpoint there, I hit play and had it hit it on the next loop around, where iHeroNum == 1 now and ran the line and m_DataKeys then looked like this:

m_DataKeys | 0x02072a60
  pointer | 0x02496b00
    pointer | 0xffeeffee
      Error : expression cannot be evaluated

Which is the exact same as before! The line didn't change the array.... At all!

For clarification, m_DataKeys is a 3 dimensional array of character pointers to character arrays of size 50.

I can't figure out why this is happening, it looks like my code is correct. Is it possible that the garbage collector is screwing me over here? Or maybe the new allocator?

Edit: A Symptom of a Larger Problem

Let me elaborate a little more on the structure of my code, because really, this is just a cheap solution to a bigger problem.

I already have structs as one of you wisely suggested:

struct HeroData
{
  // Lots o data here
  // ...
  // .
  //
  AbilityData* Abilities[NUM_OF_ABILITIES];
}

struct AbilityData
{
  // More data here
  // ...
  // .
  CUtlMap<char*,int> ExtraData [MAX_ABILITY_LEVELS];
}

Now when it got complicated and I had to do this DataKeys arrays of pointers to arrays of pointers crap is only when the need arose to be loading in some data to a dynamic structure, where both the keys, the values, and the numbers of data are completely dynamic. So I thought to use a map of char arrays to ints, but the only problem is that I can't store the actual char array in my map, I have to use a char *. I tried defining the map as:

CUtlMap<char[50],int> ExtraData [MAX_ABILITY_LEVELS];

But that really didn't work and it seems sort of strange to me anyway. So, I had to find some place to stick all these ExtraDataKeys and for some reason I thought it cool to do it like this. How can I store char arrays in objects like arrays or maps?

Ring
  • 2,249
  • 5
  • 27
  • 40
  • The value `0xffeeffee` hints that the memory could be already freed. Which compiler do you use? – Vlad Feb 20 '11 at 11:11
  • I'm using C++ with visual studio 2008, other than that I have no idea. After doing some more debugging i'm pretty sure I have some sort of array bounds problem or something... sigh. I really wish C++ had array bounds checking, this is ridiculous, but i'm too tired to do any more troubleshooting tonight, i'll have to look more into this tomorrow. – Ring Feb 20 '11 at 11:17
  • 1
    @Ring: With C++, you could replace native vectors with `std::vector`, which indeed has bounds-checking. Even if you see that with `std::vector` is too slow, you can come revert back to native vectors when the debugging is finished. – Vlad Feb 20 '11 at 11:37
  • @Ring: in C++ there is no built-in garbage collector (unless you are bringing in your own one). – Vlad Feb 20 '11 at 11:43
  • The line says `if (pkvExtraData) // only is true when iHeroNum == 1 and ability == 0`, which explains why, when you look in m_DataKeys on the second time the breakpoint is hit you get the same as before (you never went into the ability loop) – gnobal Feb 20 '11 at 12:14
  • Ugh, quadruple pointers, quite scary. :S – Matteo Italia Feb 20 '11 at 18:41
  • @vlad using std::vector::operator[] should by no mean be slower than a C-array [] as the underlying implementation is such an array and the compiler will inline everything in release mode – Tristram Gräbener Feb 20 '11 at 18:47
  • You cannot use arrays as keys in maps. Just replace all occurrences of `char[N]` and `char*` with `std::string`. This is C++ after all, not C. – fredoverflow Feb 21 '11 at 10:45

4 Answers4

3

Since you are using pointers as class members, my best guess is that you are violating The Rule Of Three. That is, you did not provide a copy constructor and a copy assignment operator for your class. That usually leads to strange data loss when passing objects of your class around.

Note that no sane C++ programmer would use char****. Here is my best attempt to fix your problem using vectors and strings, but there is probably a much better design for your specific problem:

#include <string>
#include <vector>

class Foo
{
    int m_iNumOfHeroes;
    std::vector<std::vector<std::vector<std::string> > > m_DataKeys;

    enum { NUM_OF_ABILITIES = 4, iNumOfExtraData = 2 };

public:

    explicit Foo(int iNumOfHeroes)
    : m_iNumOfHeroes(iNumOfHeroes)
    , m_DataKeys(m_iNumOfHeroes, std::vector<std::vector<std::string> >
                 (NUM_OF_ABILITIES, std::vector<std::string>(iNumOfExtraData)))
    {
    }
};

int main()
{
    Foo x(2);
}

In case you have never seen that colon syntax in the constructor before, that is a member initializer list.

I really wish C++ had array bounds checking

std::vector and std::string do have bounds checking if you use the foo.at(i) syntax instead of foo[i]. In Debug mode, even foo[i] has bounds checking enabled in Visual C++, IIRC.

Community
  • 1
  • 1
fredoverflow
  • 256,549
  • 94
  • 388
  • 662
1

Though the code might be correct, I personally find that working with something like a char **** can get pretty confusing pretty fast.
This is just my personal preference, but I always try to organize things in the most clear and unambiguous way I can, so what I would do in your situation would be something like

struct Ability
{
    char extraData[NUM_OF_EXTRA_DATA][50];
};

struct HeroData
{
    Ability abilities[NUM_OF_ABILITIES];
};

class Foo
{
    // here you can choose a 
    HeroData *heroArray;
    // and then you would alloc it with "heroArray = new HeroData[m_iNumOfHeroes];"
    // or you can more simply go with a
    std::vector<HeroData> heroVector;
};

I think this makes things more clear, making it easier for you and other programmers working on that code to keep track of what is what inside your arrays.

filipe
  • 3,370
  • 1
  • 16
  • 22
0

I think you expect the wrong thing to happen (that the visual display in the debugger would change), even though your code seems correct.

Your debugger displays m_DataKeys, *m_DataKeys and **m_DataKeys, which is the same as m_DataKeys, m_DataKeys[0] and m_DataKeys[0][0]. When you change m_DataKeys[1], you are not going to notice it in your debugger output.

The following might help you: in my debugger (MS Visual Studio 2005), if you enter e.g. m_DataKeys,5 as your watch expression, you will see the first 5 elements of the array, that is, m_DataKeys[0], m_DataKeys[1], ..., m_DataKeys[4] - arranged in a neat table. If this syntax (with the ,5) doesn't work for you, just add m_DataKeys[1] into the debugger's watch window.

anatolyg
  • 26,506
  • 9
  • 60
  • 134
0

Not sure why this didn't occur to me last night, but I was pretty tired. Heres what I decided to do:

struct AbilityData
{
  // Stuff

  CUtlMap<char*,int> ExtraData [MAX_ABILITY_LEVELS];
  char **DataKeys;
}

Thats what my abilityData struct now looks like, and it now works, but now I want to reorganize it to be like:

struct AbilityData
{
  // Stuff

  CUtlMap<char*,int[MAX_ABILITY_LEVELS]> ExtraData;
  char **DataKeys;
}

Because it makes more sense that way, but then I run into the same problem that I had before with the char array. It almost seems like to me it might just be best to ditch the whole map idea and make it like:

struct AbilityData
{
  // Stuff

  int *ExtraData;
  char **DataKeys;
}

Where ExtraData is now also a dynamically allocated array.

The only problem with that is that I now have to get my data via a function which will loop through all the DataKeys, find a matching key for my input string, then return the ExtraData associated with it.

Ring
  • 2,249
  • 5
  • 27
  • 40