1

I'm working on an email validation program for my cmpsci class and am having trouble with this one part.

What I'm doing is reading a list of valid top level domains from a text file into a vector class I wrote myself (I have to use a custom vector class unfortunately). The problem is that the program reads in and adds the first few domains to the vector all well and fine, but then crashes when it gets to the "org" line. I'm completely stumped why it works for the first few and then crashes. Also, I have to use a custom string class; that's why I have the weird getline function (so I get the input in a char* for my String constructor). I've tried using the standard string class with this function and it still crashed in the same way so I can rule out the source of the problem being my string class. The whole program is quite large so I am only posting the most relevant parts. Let me know if more code is needed please. Any help would be awesome since I have no clue where to go from here. Thanks!

The ReadTlds function:

void Tld::ReadTlds() {
    // Load the TLD's into the vector
    validTlds = Vector<String>(0); // Init vector; declaration from header file: "static Vector<String>validTlds;"
    ifstream in(TLD_FILE);
    while(!in.eof()) {
        char tmpInput[MAX_TLD_LENGTH];   // MAX_TLD_LENGTH equals 30
        in.getline(tmpInput, MAX_TLD_LENGTH);
        validTlds.Add(String(tmpInput)); // Crashes here!
    }
}

My custom vector class:

#pragma once

#include <sstream>

#define INIT_CAPACITY 100
#define CAPACITY_BOOST 100

template<typename T> class Vector {
public:
 // Default constructor
  Vector() {
   Data=NULL;
   size=0;
   capacity=INIT_CAPACITY;
  }
 // Init constructor
 Vector(int Capacity) : size(0), capacity(Capacity) {
  Data = new T[capacity];
 }

 // Destructor
 ~Vector() {
  size=0;
  Data = NULL;
  delete[] Data;
 }

 // Accessors
 int GetSize() const {return size;}

 T* GetData() {return Data;}

 void SetSize(const int size) {this->size = size;}


  // Functions
  void Add(const T& newElement) {
   Insert(newElement, size);
  }

  void Insert(const T& newElement, int index) {
  // Check if index is in bounds
  if((index<0) || (index>capacity)) {
   std::stringstream err;
   err << "Vector::Insert(): Index " << index << " out of bounds (0-" << capacity-1 << ")";
   throw err.str();
  }

   // Check capacity
   if(size>=capacity)
   Grow();

   // Move all elements right of index to the right
   for(int i=size-1; i>=index; i--)
   Data[i+1]=Data[i];

    // Put the new element at the specified index
   Data[index] = newElement;
   size++;
  }

  void Remove(int index) {
   // Check if index is in bounds
  if((index<0) || (index>capacity-1)) {
   std::stringstream err;
   err << "Vector::Remove():Index " << index << " out of bounds (0-" << capacity-1 << ")";
   throw err.str();
  }

  // Move all elements right of index to the left
   for(int i=index+1; i<size; i++)
    Data[i-1]=Data[i];
  }

 // Index operator
 T& operator [] (int index) const {
  // Check if index is in bounds
  if((index<0) || (index>capacity-1)) {
   std::stringstream err;
   err << "Vector operator[]:Index " << index << " out of bounds (0-" << capacity-1 << ")";
   throw err.str();
  }
 return Data[index];
 }

 // Assignment oper
 Vector<T>& operator = (const Vector<T>& right) {
   Data = new T[right.GetSize()];
  for(int i=0; i<right.GetSize(); i++)
   Data[i] = right[i];
  size = right.GetSize();
  return *this;
 }

    private:
 T *Data;
 int size; // Current vector size
 int capacity; // Max size of vector

 void Grow() {
  capacity+=CAPACITY_BOOST;
  T* newData = new T[capacity];
  for(int i=0; i<capacity; i++)
   newData[i] = Data[i];

  // Dispose old array
  Data = NULL;
  delete[] Data;
  // Assign new array to the old array's variable
  Data = newData;
 }
    };

The input file:

aero
asia
biz
cat
com
coop
edu
gov
info
int
jobs
mil
mobi
museum
name
net
org  <-- crashes when this line is read
pro
tel
travel

The error Visual Studio throws is:

    Unhandled exception at 0x5fb04013 (msvcp100d.dll) in Email4.exe: 0xC0000005: Access violation reading location 0xabababbb.
5
  • post the error message. that will help quite a bit :) Commented Nov 5, 2010 at 5:09
  • SO isn't about debugging your app for you. Take some basic debugging steps towards reducing complexity and isolating the fault; you'll probably solve the problem yourself, and if you don't you'll at least have a more manageable block of code to post. Commented Nov 5, 2010 at 5:09
  • Megar, I've identified the exact line the program crashes on. It has a comment next to it above in the ReadTlds function. I have only posted the relevant code. Excuse me for posting the whole vector class but I feel as though the problem lies within it, but I have no idea where. Commented Nov 5, 2010 at 5:12
  • Learning to debug is more important than learning to write a Vector class. The crash occurs in a line that invokes two custom classes, so separate them; do the String part in one line and the validTlds part in the next. It always crashes on the "org" datum, so put the "org" datum first and see what happens. Simplify, experiment, narrow it down. Commented Nov 5, 2010 at 5:20
  • you should know that your code to read the file has an error in it too: while(!in.eof()) { is an error since eof won't be set till after you read! a better way to write that code (and shorter) would be like this: char tmpInput[MAX_TLD_LENGTH]; while(in.getline(tmpInput, MAX_TLD_LENGTH)) { validTlds.Add(String(tmpInput)); } Commented Nov 5, 2010 at 5:47

4 Answers 4

4

The problem is in your grow function:

void Grow() {
  capacity+=CAPACITY_BOOST;
  T* newData = new T[capacity];
  for(int i=0; i<capacity; i++)
   newData[i] = Data[i];

You increase the capacity, but then copy elements that didn't exist in the old array. It should be something like:

void Grow() {
  int old_capacity = capacity;
  capacity+=CAPACITY_BOOST;
  T* newData = new T[capacity];
  for(int i=0; i<old_capacity; i++)
   newData[i] = Data[i];

You also NULL out Data before deleting it in both Grow and the destructor, which causes a memory leak. In both cases, you really don't need to set it to NULL at all, since there's no change of it being accidentally double-deleted (in Grow it's set to a new pointer immediately, in the destructor the object's lifetime is over). So just

delete[] Data;

alone is fine.

Also I think

if(size>=capacity)

can be:

if(size == capacity)

since size should never be over capacity. That would mean you'd already overflowed the buffer.

Sign up to request clarification or add additional context in comments.

3 Comments

Thank you Matthew. I've corrected the grow function as well as the memory leak (my eyes glossed over those lines a thousand times without ever catching that). The program is now crashing in the init constructor of the vector class which I'm currently trying to debug. Your help is very much appreciated!
@Mike, I don't see how it could crash in either vector constructor. By the way, the memory leak issue also applies to the destructor.
My apologies, I went through the code a little more and even though the default constructor is called which initalizes the the Data array to "Data = new T[INIT_CAPACITY];" (I've corrected it as per the suggestions from other readers) when the Insert function is called, Data is equal to a bad pointer. That seems like a job for tomorrow though. I've been at this way too long today. :p And yes, I corrected the memory leak issue in the destructor too.
1

Matthew is probably right. Still, there's a valuable lesson to be learned here.

When you hit a problem like this, don't stop walking your code in your ReadTlds function. Keep walking inside the Vector class. Functions like Insert and Grow probably hold the error, but if you don't walk through them, you'll never find it.

Debugging is it's own very special skill. It takes a long time to get it down pat.

2 Comments

Thank you for the advice Stephano. I was stepping inside my string and vector classes, but forgot to mention it here. Trust me, I spent the better part of today debugging and trying everything I could think of to figure out what the hell was wrong with this program. Stack Overflow is always my last resort.
Awesome, that is good to hear. Debugging is really one of the finer arts of programming, and we all have days where we stare at the code for hours and just can't make sense of it.
1

edit it's a late night and I misread your code, but I left my post to comment back

3 Comments

Hi biryree! Even if I initialize the vector with "Vector<String>(100)" it still crashes in the same way. It works as is for the first 16 lines of the input file, but crashes on the 17th.
@Mike sorry, I was mistaken and misread your code. Matthew Flaschen is correct and your Grow() function isn't behaving correctly.
Thank you birryree, I've fixed the grow function.
0

Also in the default ctor you do

Data = NULL;
capacity=INIT_CAPACITY;

(EDIT: expanded explanation here) But never allocate the memory for Data. Shouldn't it be:

  Vector() {
   Data= new T[INIT_CAPCITY];
   size=0;
   capacity=INIT_CAPACITY;
  }

And remove is missing

--size

EDIT: Fellow readers help me out here:

Data is of type T* but everywhere else you are assigning and allocating it just like T instead of T* . My C++ days are too long gone to remember whether using a T& actually resolves this.

Also I can't remember that if you have an array of pointers and destruct it, that the dtor for the single instances in the array are destroyed.

Also in the assignment operator, wouldn't you be copying the pinters? so you just have to rely on the fact the the instance where you copyid from is never deleted (because then your objects would be dead too).

hth Mario

2 Comments

Thanks for that catch in the remove function; its been corrected. What do you mean by not allocating memory for the capacity variable? Does giving it a value of INIT_CAPACITY (which is equal to 100) not allocate it with memory?
Sorry, bad wording. I expanded my explanation. Of course the memory for the int itself is allocated but not for Data.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.