Access violation reading location 0xcdcdcdcd. C++

Question

Essentially what I'm trying to do at this point is build a program to allow you to pick from 3 different classes (Tank, Mele, Ranged) when you pick the class you would then give it a name of 20 or fewer chars. After picking 5 classes and giving each champ a name it would print out the name and health of each class you picked. code stands like this:

#include "Driver.h"
#include <stdio.h> 
#include "Mele.h"
#include "Ranged.h"
#include "Tank.h"

int main(void)
{

Champion *champ[5];
int i, choice;

printf("Enter the number for which class you would like to add to your team\n");
for(i = 0; i <= 4; i++)
{
    char name[20];
    //printf("Enter the number for which class you would like to add to your team");
    printf("1 = Tank\n");
    printf("2 = Ranged\n");
    printf("3 = Mele\n");
    scanf_s("%d", &choice);
    if(choice == 1)
    {
        printf("Give him a name!\n");
        scanf("%s", name);
        champ[i] = new Tank(name);
    }
    else if(choice == 2){
        printf("Give him a name!\n");
        scanf("%s", name);
        champ[i] = new Ranged(name);
    }
    else if(choice == 3){
        printf("Give him a name!\n");
        scanf("%s", name);
        champ[i] = new Mele(name);
    }
    else
    {
        printf("You did not enter a number between 1 and 3 please try again!\n");
        i = i - 1;
    }
}
for(i = 0; i <= 4; i++)
{
    printf("%s has %f health", champ[i]->getName(), champ[i]->getHealth());
}
return 0;
}

That's the main function

The champion class looks like:

Champion::Champion(void)
{
}
Champion::Champion(char name1[])
{ 
    name = name1;
}

char* Champion::getName(void)
{   
    return name;
}   

double Champion::getHealth(void)
{
    return health;
}

int Champion::getFluid(void)
{
    return fluid;
}

double Champion::getArmor(void)
{
    return armor;
}

double Champion::getSpecialA(void)
{
    return specialA;
}

double Champion::getDamage(void)
{
    return physDamage;
}

void Champion::setHealth(double health1)
{
    health = health1;
}
void Champion::setFluid(int fluid1)
{
    fluid = fluid1;
} 
void Champion::setArmor(double armor1)
{
    armor = armor1;
}
void Champion::getSpecialA(double specialA1)
{
    specialA = specialA1;
}
void Champion::setDamage(double physDamage1)
{
    physDamage = physDamage1;
}

Then i have 4 other classes called Tank, Ranged and Mele; all of these inherit from Champion and have the same set up as Champion. when i run the program i get this:

'dragons_rage.exe': Loaded 'C:\Users\Tom\Documents\Visual Studio 2010\Projects\dragons_rage\Debug\dragons_rage.exe', Symbols loaded.
'dragons_rage.exe': Loaded 'C:\Windows\SysWOW64\ntdll.dll', Cannot find or open the PDB file
'dragons_rage.exe': Loaded 'C:\Windows\SysWOW64\kernel32.dll', Cannot find or open the PDB file
'dragons_rage.exe': Loaded 'C:\Windows\SysWOW64\KernelBase.dll', Cannot find or open the PDB file
'dragons_rage.exe': Loaded 'C:\Windows\SysWOW64\msvcr100d.dll', Symbols loaded.
First-chance exception at 0x5dfc14cf (msvcr100d.dll) in dragons_rage.exe: 0xC0000005: Access violation reading location 0xcdcdcdcd.
Unhandled exception at 0x5dfc14cf (msvcr100d.dll) in dragons_rage.exe: 0xC0000005: Access violation reading location 0xcdcdcdcd.
The program '[516] dragons_rage.exe: Native' has exited with code -1073741819 (0xc0000005).

Im not exactly sure what these errors are and what they mean if i could get some help that would be amazing thank you!!!!

Answer 1

Make it a habit to set the pointer members to 0 (NULL) is the constructor if they are not pointing to any address on the heap or on the stack. Doing this will save you from the 0xcc... invalid addresses. If you are done with the pointer even if the pointer is part of a double pointer, set it back to 0 after free the object which it points to. It is your responsibility which writing the code to do so. The other option is go do a managed memory programming language instead.

And with your code, it's

Champion** champ = (Champion**)malloc(sizeof(Champion*) * 5);
// or
Champion** champ = (Champion**)calloc(5, sizeof(Champion*));
// or
Champion** champ = new Champion*[5];

Which way you choose to allocation the champ double pointer is up to you. I prefer (Champion**)malloc(sizeof(Champion*) * 5) because I am used to C style coding.

Answer 2

check the

return name;

in

Champion::getName()

where is name defined? Is it initialized?

---

When you do name = name1 in the constructor, you are just copying the pointer. In the case of your program, this is a pointer to a local variable in your for loop. That variable goes out of scope once you leave that for loop. You should use std::string::copy() or strcpy() to copy your string.

Answer 3

The variable char name[20] is not valid after leaving the first for()

You should copy the value of the array in the constructor to an array inside Champion or dynamically allocate memory for the name.

This is one option:

#include <stdio.h>
#include <string.h>

class Champion {
  char name[20];

 public:

 Champion(const char theName[],int size ){

  for( int i=0;i < size; i++ ){
    name[i] = theName[i];
  }
 }
 const char* getName(){
  return name;
 }
};

int main(int argc, const char* argv[]){

 Champion *c;
 const char name[] = "vamos";
 c = new Champion(name,strlen(name));
 printf("%s",c->getName());
 return 0;
}

Answer 4

Your loop only runs 4 times, so the last Champion pointer is never initialized.

for(i = 0; i < 4; i++)

Should be changed to:

for(i = 0; i < 5; i++)

Also none of the members of your Champion class are initialized in the constructor, so reading them will cause undefined behaviour.

The code you wrote is more or less C with classes. Look up std::string, it will make your code much simpler and more correct. As it is right now, your program contains multiple buffer overflow vulnerabilities and dangling pointers.

If I were evil, I would create a Tank with a name much longer than 20 characters and likely be able to crash your program or worse, execute arbitrary code by overwriting your text segment.

Answer 5

One thing that jumps out at me is the Champion constructor:

Champion::Champion(char name1[])
{
}

This doesn't do anything with the character array -- it doesn't initialize any "name" member. WHen the name is handed out later, is it null, or worse, garbage? You presumably need to copy that argument into your member variable, so you have a name you can use later.