Search code examples
c++stringsegmentation-faultgetlinedynamic-arrays

Segfault in custom string class


So I am trying to complete this very basic string class (MyString). Everything seemed to work, but when I uploaded it to the assignment site, it showed a segfault. The upload site uses electric fence, but it didn't give much insight as to where the fault occurred. It essentially runs through each function and returns a pass/fail/fault for it. In the case of the getline function, it returned a fault.

Also, the upload site uses valgrind which reported no errors.

EDIT: I almost forgot, when I called the function in the driver, it read from a file messages.txt, which contained one line of text: Testing this program... PLEASE WORK

Below is the getline function (as it exists in the implementation file) that appears to be the source of the fault:

// reads line from istream ... line end at newline char of choice) -- '\n' in this case
void MyString::getline(istream &inFile, char delimit)
{
    int index = 0;
    do
    {
        data[index] = inFile.get();
        index ++;
        if (index + 1 > capacity)
        {
            MyString tempStr;
            delete [] tempStr.data;
            tempStr.data = new char [capacity];
            for (int i = 0; i <= index; i++)
            {
                tempStr.data[i] = data[i];
            }
            capacity += 5;
            size = index;
            delete [] data;
            data = new char [capacity];
            for (int i = 0; i <= size; i++)
            {
                data[i] = tempStr.data[i];
            }
            delete [] tempStr.data;
            tempStr.data = NULL;
        }
    }
    while (!inFile.eof() && data[index-1] != delimit);
   if (data[index - 1] == delimit)
    {
        index -= 1;
        if (static_cast<double>(index)/capacity < .25 && capacity > 5)
        {
            capacity -= 5;
            char *temp = new char [capacity];
            for (int i = 0; i < index; i++)
            {
                temp[i] = data[i];
            }
            delete [] data;
            data = temp;
        }
    }
    data[index] = '\0';
    size = index + 1;
}

I feel like it's either something very simple I overlooked or a fundamental flaw in the way I approached this particular function. Any help is appreciated. I'm very new to programming (few weeks in) and am just trying to stay afloat -- got enrolled in CompSci 1 + 2 simultaneously.

Additionally, below is more of the implementation file -- particularly, the constructors (minus copy) and a few overloaded operators. While I could compile it on my end and concatenate class objects successfully, the upload site returned a fail when it tested "Concatenation." There wasn't any feedback as to which operator failed. I was curious what might cause that in my code. Thanks again.

#include <iostream>
#include <fstream>
#include "MyString.h"

using namespace std;

//default constructor - works
MyString::MyString()
{
    capacity = 5;
    size = 0;
    data = new char [capacity];
}

// constructor with character string
MyString::MyString(const char *cString) 
{
    int index = 0;
    capacity = 5;
    while ( cString[index] != '\0')
    {
        index++;
    }
    size = index + 1;
    while (size > capacity)
    {
        capacity += 5;
    }
    data = new char[capacity];
    for (int i = 0; i < size; i++)
    {
        data[i] = cString[i];
    }
}

// copy constructor
MyString::MyString(const MyString &aMyString)
{
    capacity = aMyString.capacity;
    size = aMyString.size;
    data = new char [capacity];
    for (int i = 0; i < size; i++)
    {
        data[i] = aMyString.data[i];
    }
}
// overloaded += operator
void MyString::operator+=(const MyString &aMyString)
{
    int tSize1 = size;
    int holder = 0;
    size += aMyString.size - 1;
    while (size > capacity)
    {
        capacity += 5;
    }
    char *tempArr = new char [capacity];
    for (int i = 0; i < (tSize1 - 1); i ++)
    {
        tempArr[i] = data[i];
    }
    for (int i = (tSize1 - 1); i < size; i++)
    {
        tempArr[i] = aMyString.data[holder];
        holder ++;
    }
    delete [] data;
    data = tempArr;
}

// overloaded + operator
MyString MyString::operator+(const MyString &aMyString) const
{
    int holder = 0;
    MyString tempS;
    int tSize1 = size + aMyString.size - 1;
    int tCap1 = capacity + aMyString.capacity;
    if (static_cast<double>(tSize1)/tCap1 < .25 && tCap1 > 5)
    {
        tCap1 -= 5;
    }
    tempS.size = tSize1;
    tempS.capacity = tCap1;
    delete [] tempS.data;
    tempS.data = new char [tempS.capacity];
    for (int i = 0; i < (size - 1); i ++)
    {
        tempS.data[i] = data[i];
    }
    for (int i = (size - 1); i < tSize1; i++)
    {
        tempS.data[i] = aMyString.data[holder];
        holder ++;
    }
    return tempS;
}

Solution

  • I don't know if these are all your bugs but I see two that stand out from the code:

    for (int i = 0; i <= index; i++)
    {
        tempStr.data[i] = data[i];
    }
    [snip]
    for (int i = 0; i <= size; i++)
    {
         data[i] = tempStr.data[i];
    }
    

    Both the for statements are accessing one more character than they should. If you want to process something 5 times for example in zero based indexing you check for i < 5 not i <= 5 . If I am not mistaken you have made that error in both these for loops. I believe they should be:

    for (int i = 0; i < index; i++)
    

    and

    for (int i = 0; i < size; i++)
    

    Writing memory beyond the edge of your arrays can cause problems like the segfaults.