Exception errors when using strcpy

1

I'm working on a BST for class. There are 5 files in the class, 2 1/2 of which I cannot edit (as an exercise in OOP). I cannot edit data.h, driver.cpp, or the public members of bst.cpp.

I'm getting some exception errors when trying to use strcpy in my data.cpp file. These are relevant because my insert function in bst.cpp is sent a data object as an argument from the driver.

The errors are in the form of

Unhandled exception at 0x0F3840D9 (msvcr120d.dll) in asgmt04.exe: 0xC0000005: 
Access violation writing location 0x00000000.

Here's some code

In bst.cpp

void BST::insert(const Data& data)
{
    if (index > capacity)
        grow();

    if (items[index].isEmpty == true)
    {
        items[index].data.setName(data.getName());
        nItems++;
        items[index].isEmpty = false;
        items[index].loc = index;
    }
    else if (data < items[index].data)
    {
        index = (2 * index) + 1;
        insert(data);
    }
    else
    {
        index = (2 * index) + 2;
        insert(data);
    }
}

Again, I can't edit the function prototype because it's a public member.

In data.h

char const * const getName() const { return name; }

In data.cpp

void Data::setName(char const * const name)
{
    strcpy(this->name, name);
}

I also tried using the overloaded = operator and ran into the same issue. The code that called it looked like

items[index].data = data; //second arg is the one passed into insert function

And in data.cpp

Data& Data::operator=(const Data& data2)
{
    strcpy(this->name, data2.name);
    return *this;
}
c++
exception
operator-overloading
strcpy
asked on Stack Overflow Dec 9, 2014 by FutureShocked • edited Mar 4, 2019 by Zoe

1 Answer

1

I suspect that at time you execute the line

strcpy(this->name, data2.name);

there isn't enough room in this->name to hold data2.name. Here's a suggestion:

Data& Data::operator=(const Data& data2)
{
    // Prevent self assignment.
    if ( this != &data2 )
    {
       if (strlen(this->name) < strlen(data2.name) )
       {
          // Assuming that you used new to allocate memory.
          delete [] this->name;
          this->name = new char[strlen(data2.name) + 1];
       }
       strcpy(this->name, data2.name);
    }
    return *this;
}

Update, in response to comment by OP

If Data::name is allowed to be NULL, then there need to be more checks.

Data& Data::operator=(const Data& data2)
{
   // Prevent self assignment.
   if ( this != &data2 )
   {
      if ( this->name == NULL )
      {
         if ( data2.name == NULL )
         {
            // Nothing needs to be done.
         }
         else
         {
            this->name = new char[strlen(data2.name) + 1];
            strcpy(this->name, data2.name);
         }
      }
      else 
      {
         if ( data2.name == NULL )
         {
            delete this->name;
            this->name = NULL;
         }
         else
         {
            if ( strlen(this->name) < strlen(data2.name) )
            {
               // Assuming that you used new to allocate memory.
               delete [] this->name;
               this->name = new char[strlen(data2.name) + 1];
            }
            strcpy(this->name, data2.name);
         }
      }
   }
   return *this;
}
answered on Stack Overflow Dec 9, 2014 by R Sahu • edited Dec 9, 2014 by R Sahu

User contributions licensed under CC BY-SA 3.0