Member Avatar for saishn

Again, I don't know what I've done wrong.

#include <iostream>
using std::cout;
using std::endl;

#include <cstring>
using std::strcpy;
using std::strcat;

#include <cstdio>
using std::sprintf;

void destroyMultiCharArray(char * * anArray, int size)
{
    for (int i = 0; i < size; i++)
    {
        delete [] anArray[ i ];
    }
    delete [] anArray;
    anArray = 0;
}
class aClass
{
    public:
        aClass();
        ~aClass();
        bool addPattern(char * type, char * ptrn);

    private:
        int numPtrns;
        char ** ptrnTypes;
        char ** ptrns;
};
aClass::aClass()
{
    this->numPtrns = 0;

    char u[] = "upatdjskoddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd2333333333333333333";

    char i[] = "dsud8923333333333333duiuuuuuuuuuuuuut";

    char q[] = "HDddddddddddd*@#(##########HDJD";

    char p[] = "dJ(#&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&@******************************************DGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGGG*#######################";

    char h[] = "#(((((((((((((((((((((((((DKKKKKKKKKKKKKKKKKKKKKKKKKK#(((((((((((((((((((((((((DKMMMMMMMMMMMMMMMMMMMMMMMMMM#IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII";


    this->addPattern("u", u);
    this->addPattern("i", i);
    this->addPattern("q", q);
    this->addPattern("p", p);
    this->addPattern("h", h);
}
aClass::~aClass()
{
    destroyMultiCharArray(ptrnTypes, numPtrns);
    destroyMultiCharArray(ptrns, numPtrns);
}
bool aClass::addPattern(char * type, char * ptrn)
{

    char * * typesHolder = 0;
    char * * ptrnsHolder = 0;
    if (this->numPtrns)
    {

        //holders point to the current array
        //which will be used to repopulate
        //the ptrn and types arrays
        typesHolder = this->ptrnTypes;
        ptrnsHolder = this->ptrns;
    }

    int newPtrnsNum = this->numPtrns + 1;
    //Making room for one more pattern
    this->ptrnTypes = new char * [newPtrnsNum];
    this->ptrns = new char * [newPtrnsNum];

    //now first adding the new pattern
    //and then repopulating using the
    //holders
    this->ptrnTypes[ 0 ] = new char [strlen(type) + 1];
    strcpy(this->ptrnTypes[ 0 ], type);

    this->ptrns[ 0 ] = new char [strlen(ptrn) + 1];
    strcpy(this->ptrns[ 0 ], ptrn);

    if (typesHolder)
    {
        int i;

        //Holds the current index of the ptrn
        //arrays
        int ptIndex;
        for (i = 0, ptIndex = 1 ; i < this->numPtrns; i++, ++ptIndex)
        {
            this->ptrnTypes[ ptIndex ] = new char [
                                        (strlen(typesHolder[ i ] + 1))
                                        ];
            strcpy(this->ptrnTypes[ ptIndex ] , typesHolder[ i ]);

            this->ptrns[ ptIndex ] = new char [
                                        (strlen(ptrnsHolder[ i ] + 1))
                                        ];
            strcpy(this->ptrns[ ptIndex ], ptrnsHolder[ i ]);
        }

        destroyMultiCharArray(typesHolder, this->numPtrns);
        destroyMultiCharArray(ptrnsHolder, this->numPtrns);
        typesHolder = 0;
        ptrnsHolder = 0;
    }

    this->numPtrns = newPtrnsNum;
    return true;
}

int main()
{
    aClass aObj;

    return 0;
}

As mentioned in the title, this Segfaults on me. I don't understand why. I've used gdb and it (this code which comes out of a larger snippet which fails inside of destroyMulti) fails in the addPattern function at "this->ptrnTypes[ ptIndex ] = new char [ ..." .

What's wrong?

If you could also give some tips on debugging memory issues that cause these problems please do. I've seen "valgrind" but haven't tried it yet.

As always, thanks for any help that you can provide.

Btw, I made the strings that long because it ran fine when I used a short string. I know(think) that I've mishandled the memory somehow, but don't know where.

  • 1 Contributor
  • 1 Reply
  • 318 Views
  • 7 Hours Discussion Span
  • Latest Post Latest Post by saishn

Dani AI

Generated

Good catch, — the symptom and your fix line up exactly with an under-allocation / buffer-overwrite caused by a misplaced +1. By putting the +1 inside the length expression you ended up measuring the string starting one character later and then allocated fewer bytes than needed. The subsequent copy wrote the full original string (including the terminator) into a too-small buffer, corrupting the heap and producing a segfault with long inputs while short inputs could appear to “work” by luck.

A safer, modern approach is to avoid manual new[]/delete[] entirely and use RAII containers so you can’t silently miscompute sizes. For example:

#include <vector>
#include <string>

std::vector<std::string> types;
std::vector<std::string> patterns;

bool addPattern(const std::string& type, const std::string& pat) {
  types.insert(types.begin(), type);     // keep original ordering
  patterns.insert(patterns.begin(), pat);
  return true;
}

This eliminates the whole class of allocation/termination bugs and makes lifetime management automatic.

Practical debugging tips: compile with debug symbols and warnings enabled, and run with sanitizers to catch invalid writes and overruns (AddressSanitizer/UBSan). Example compile line: g++ -g -O0 -Wall -Wextra -fsanitize=address,undefined -fno-omit-frame-pointer. Valgrind (valgrind --leak-check=full ./prog) and gdb watchpoints are also useful for tracking the exact offending write. These tools give an immediate stack trace to the allocation/copy that corrupts memory.

One extra note: your helper sets its local pointer to null after deleting, but that does not change the caller’s member pointer. If you need the member reset, either make the helper take the pointer by reference or explicitly set the member to nullptr after the call (in a destructor this is usually unnecessary, but it matters outside destruction).

Member Avatar for saishn

Found the error

this->ptrnTypes[ ptIndex ] = new char [
                                        (strlen(typesHolder[ i ] + 1))
                                        ];
            strcpy(this->ptrnTypes[ ptIndex ] , typesHolder[ i ]);

            this->ptrns[ ptIndex ] = new char [
                                        (strlen(ptrnsHolder[ i ] + 1))
                                        ];
            strcpy(this->ptrns[ ptIndex ], ptrnsHolder[ i ]);

The strlen() lines should be

strlen(typesHolder[i])
strlen(ptrnsHolder[i])

The closing ")" was surrounding the + 1 as well.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.