0

I'm trying to scan data from a txt file line by line. I have a function that scans the whole file into a local generic linked list, after returns with the list. (by value). I have a generic linked list in my main function that gets the list returned by the function. Here starts the problems. My list class has explicit dtor, if I remove this dtor (ofc its memory leak), the whole thing works, but if is use this dtor, I get an error

Read access violation _Pnext was 0xDDDDDDE1

I don't know how can be the problem around the dtor. Maybe does it called somewhere where shouldn't?

I tried to construct the list in different way in the function and return with pointer but it didn't help.

This code is not my whole project, just the important things i hope.

class Card {
   private:
      string name1;
      string name2;
      string city;
      string job;
      string number;
      string email;

   public:
      Card() {}
      Card(string name1, string name2, string city, string job, string number, 
           string email);
      ~Card();
};

template <class L>
class MyList {
   private:
      struct Node {
         Node* next;
         L data;
         ~Node() {}
      };
      Node* head;

   public:
      MyList() { this->head = NULL; }
      ~MyList() {
         Node* current = head;
         while (current != NULL) {
            Node* next = current->next;
            delete current;
            current = next;
         }
         head = NULL;
      }

      void add(const L& li) {
         Node* last = new Node;
         last->data = li;
         last->next = head;
         head = last;
      }

      /*class iterator { ... }
        iterator begin() {}
        iterator end() {}
       */
};

MyList<Card> scan(string name){
   MyList<Card> list;
   ifstream file(name);
   if (file.is_open()) {
      string line;
      while (getline(file, line)){
         string name1;
         string name2;
         string city;
         string job;
         string number;
         string email;

         istringstream iline(line);

         getline(iline, name1, ',');
         getline(iline, name2, ',');
         getline(iline, city, ',');
         getline(iline, job, ',');
         getline(iline, number, ',');
         getline(iline, email, ',');

         Card c(name1, name2, city, job, number, email);
         list.add(c);
      }
      file.close();
   }
   return list;
}

int main()
{
   string filename;
   MyList<Card> list;

   cin >> filename;
   list = scan(filename);

   return 0;
}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Exxiler
  • 1
  • 2
  • Read up on [The Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) and implement the necessary functions properly -- the copy constructor and the the copy assignment operator. – R Sahu May 13 '19 at 21:53
  • I'm beginning to wonder whether these assignments are meant for the student to do self-study, since the obvious problem is the lack of a user-defined copy constructor and assignment operator. If this is a school assignment, was your teacher silent on this issue? – PaulMcKenzie May 13 '19 at 21:55
  • On this very site: [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – WhozCraig May 13 '19 at 22:05
  • Wikipedia keeps a good [list of magic programming values](https://en.wikipedia.org/wiki/Magic_number_(programming)#Magic_debug_values). When you get an insane, but highly repetitious number like 0xDDDDDDE1, look it up. There will probably be ofsets from an origin and other little details that make the least significant end look a little fuzzy, but you can usually get a lot of helpful information. A close match, DDDDDDDD, is apparently used by Visual C++ to mark freed heap memory. – user4581301 May 13 '19 at 22:31

1 Answers1

0

This looks like a Rule of Three violation.

Your list can be copied using the default copy constructor or assignment operator. But doing so simply duplicates the pointers instead of creating a deep copy.

If you duplicate the MyList object without making copies of the list, then only one of those objects' destructors can behave correctly. The other will be releasing memory that has already been released.

Implement the following members for MyList:

MyList(const MyList& other)
{
    // TODO: do a deep copy of `other`
}

MyList& operator=(const MyList & other)
{
    if (this != &other)
    {
       MyList copy(other);
       std::swap(*this, copy);
    }
    return *this;
}
paddy
  • 60,864
  • 6
  • 61
  • 103
  • 1
    Not to nitpick, but I think it is better to have the copy constructor have the meaty part of the code, and the assignment operator just piggy-back on it by using copy / swap. – PaulMcKenzie May 13 '19 at 21:57
  • Thanks for the help, but unfortunately i still have problems with it. I wrote some versions of cpy ctor, but the file scan still not works. I get Unhandled exception at 0x77AC6E08 (ntdll.dll) and SO, at my Card Class's default ctor. – Exxiler May 15 '19 at 17:21
  • If you want me to comment on that or adjust my answer, you must add those implementations to your question. – paddy May 15 '19 at 21:34
  • @Exxiler it is sickeningly easy to have multiple problems in code. The best solution I know of for this is to minimize the amount of code added to a program between compiling testing and debugging. If you add a short function or a few lines to an existing function and the program suddenly stops behaving as expected, the bug's usually easy to find because it's in the new code. Of course the new code could merely expose an existing bug, but on the whole, testing early and often eliminates bugs before they get entrenched. – user4581301 May 15 '19 at 22:47