Search code examples
c++listdirectory

Error when storing names of subdirectories as nodes in list


I open a directory that has multiple subdirectories with names of countries. What i want is to make a list and store the paths of these directories in the nodes. The issue comes up when i print the list and in the destructor. I don't seem to get what's wrong since i've used these functions multiple times before with no issue coming up.

The directory looks something like this:

input_dir
   >Tanzania
   >Honduras
   >iTaiwan
   >Albania
   >Qatar
   >Grenada
   >Thailand
   >Croatia
   >Guatemala
   >Uruguay

Here's all the code: My main:

int main(int argc, char *argv[])
{
  int num_elements; //command line arguments
  char* input_dir_path;

  num_elements = stoi(argv[1]);
  strcpy(input_dir_path, argv[2]);

  Countries all_countries;
  fetch_countries(num_elements,all_countries,input_dir_path);

  all_countries.print();

  return 0;
}

Here is the function where i open the directory and store the paths on the list:

void fetch_countries( int num_monitors,  Countries& all_countries, char* input_dir_path)
{
  DIR * input_dir = opendir(input_dir_path); // open the path

  if(input_dir == NULL){
    cout<< "Error opening directory" << endl;
    exit(1);
  }

  char buffer[257];
  struct dirent *entry;  // for the directory entries
  
  while ((entry = readdir(input_dir)) != NULL)
  {
    char *f_name = entry->d_name;
    if (entry->d_type != DT_DIR || !strcmp(f_name, ".") || !strcmp(f_name, ".."))  // Ignore . and .. dirs
      continue;
    // buffer contains instance: input_dir/Uruguay
    sprintf(buffer, "%s/%s", input_dir_path, f_name);
    all_countries.Insertion(buffer);  // storing all of the countries on a list
  }
  closedir(input_dir);
}

And finally the list functions:

class CountryNode
{
public:
    string country;
    CountryNode* next;
    CountryNode(const string country);
    ~CountryNode();
};

class Countries
{
public:
    CountryNode* head;
    CountryNode* tail;
    Countries();
    ~Countries();
    void Insertion(const string country);
    void print();
};

void Countries::Insertion(const string country)
{
  CountryNode* newnode = new CountryNode(country);

  //the list is empty
  if(head == NULL)
  {
    head = newnode;
    tail = newnode;
    return;
  }
  tail->next = newnode;
  tail = newnode;
}

Countries::Countries()
{
  head = NULL;
  tail = NULL;
}

void Countries::print()
{ // now we print the nodes
  CountryNode* current;
  current=head;
  while (current != NULL)
  {
    cout << current->country << endl;
    if (current->next != NULL)
      cout << "->";
    current = current->next;
  }
  cout << endl;
}

Countries::~Countries()
{
  CountryNode* current = head;
  CountryNode* next;

  while (current != NULL)
  {
    next = current->next;
    delete current;
    current = next;
  }
  
  head = NULL;
  tail = NULL;
}

CountryNode::CountryNode(const string country)
:country(country)
{}

CountryNode::~CountryNode() {}

Finally the error i get:

input_dir/Tanzania
->input_dir/Honduras
->input_dir/Taiwan
->input_dir/Albania
->input_dir/Qatar
->input_dir/Grenada
->input_dir/Thailand
->input_dir/Croatia
->input_dir/Guatemala
->input_dir/Uruguay
==16098== Conditional jump or move depends on uninitialised value(s)
==16098==    at 0x10BA29: Countries::print() (in /home/...)
==16098==    by 0x10A8F4: main (in /home/...)
==16098== 
==16098== Conditional jump or move depends on uninitialised value(s)
==16098==    at 0x10B9F4: Countries::print() (in /home/...)
==16098==    by 0x10A8F4: main (in /home/...)
==16098== 

==16098== Conditional jump or move depends on uninitialised value(s)
==16098==    at 0x10BA87: Countries::~Countries() (in /home/...)
==16098==    by 0x10A980: main (in /home/....)
==16098== 
==16098== 
==16098== HEAP SUMMARY:
==16098==     in use at exit: 0 bytes in 0 blocks
==16098==   total heap usage: 533 allocs, 533 frees, 153,121 bytes allocated
==16098== 
==16098== All heap blocks were freed -- no leaks are possible
==16098== 
==16098== Use --track-origins=yes to see where uninitialised values come from
==16098== For lists of detected and suppressed errors, rerun with: -s
==16098== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

Is there something i'm not seeing? Any help would be much appreciated


Solution

  • You never set newnode->next to NULL. So when you loop through the list of nodes, you run into an unitialized pointer when you get to the last node.

    You can change the CountryNode constructor to initialize it to this default.

    CountryNode::CountryNode(const string country)
        :country(country), next(NULL)
    {}