0

With the struct declaration:

struct Dat {
   int count; // number of files
   string *files;
   Dat(): count(0), files(nullptr) {}
   ~Dat() {
      delete[] files;
   }
}

And then I built files like so:

buildFiles(int argc, char *argv, Dat my) {
   my.files = new string[my.count]; // my.files = new string[3]
   int filedex = 0;
   for (int i=0; i<argc; ++i) {
      if (some condition for argv[i]) { // only looking for filenames from argv
         my.files[filedex] = argv[i]; // store filename in my structure
         filedex++;
      }
   }
   if (filedex != my.count) cerr << "BAD";
}


int main (int argc, char *argv[])
{
   Dat my;
   // functions that define my.count to be non-zero, e.g. my.count = 3
   buildsFiles(argc, argv, my);
   // blahblah
   return 0;
} 

How do I now delete the dynamically allocated variables, files? I've tried delete[] my.files and delete[] my->files but neither seem to work. I'm very new to c++ so any help would be much appreciated.

G Doe
  • 41
  • 3
  • Use `std::vector files;` and drop the pointer usage. – PaulMcKenzie Jun 05 '17 at 02:23
  • 1
    This is a roundabout and error-prone way of doing something as simple [as this](http://coliru.stacked-crooked.com/a/b63b38b0750f1254). Your problem with the `Dat` struct is that you're passing it by value, and doing that incurs copies. Your `struct` is not safely copyable or assignable (does not follow the "rule of 3"). So much work involved, when you could simply use `std::vector` as in the example I linked to. – PaulMcKenzie Jun 05 '17 at 02:32
  • 1
    [Rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – PaulMcKenzie Jun 05 '17 at 02:41

3 Answers3

0

When Dat my; leaves scope the destructor is called, which deletes the array of char pointers. You don't need to do anything about the stored strings because they're just pointers to the contents of the argv array, which is handled by c++

Bit right now you've got errors. my.count starts as zero, so it's allocating zero space. Then you're writing to memory, even though no space has been allocated. That's dangerous - it's on the stack so you can overwrite the stack's important values.

You need to pre-allocate some space and check that you don't try to store too many things in the array. Right now you're playing russian roulette with memory writes.

If you want to be able to resize a structure on the fly you need to use std::vector or some sort of linked list.

If you insist on manual allocation, then you need to pre-allocate enough space (either a set amount or by counting the number of things to store), and put checks in to detect if you've tried to access something outside of the allocated memory.

Jason Lang
  • 1,079
  • 9
  • 17
  • My actual code is a bit more intertwined with helper functions. I declare 'my' in the main but I have helper functions that builds my 'string *files'. However, when I check with valgrind, I get memory leaks. – G Doe Jun 05 '17 at 02:20
0

Have a look at this code, which will detect memory leaks:

#include <crtdbg.h>

#define DEBUG_NEW new(_NORMAL_BLOCK, __FILE__, __LINE__)
#define new DEBUG_NEW

struct A
{
    int* num;
};

int main()
{
    A a;
    a.num = new int[10];

    delete[] a.num;

    _CrtDumpMemoryLeaks();

}

This excerpt has no memory leaks. Because for every new[] a delete[] was called.

But if you were to comment out delete[] a.num;, then it would detect a leak:

Detected memory leaks!
Dumping objects ->main.cpp(14) : {66} normal block at 0x0000024280E47630, 40 bytes long.
 Data: <                > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD 
Object dump complete.

So for every new you need one delete and for every new[] you need one delete[].

Shiro
  • 2,610
  • 2
  • 20
  • 36
0

you shuld used refrence in buildFiles(int argc, char *argv[], Dat &my) like this, otherwise it will copy a my object from main function, but there is no copy constructor in Dat,so it's not safe, the destructor will be called twice. ps:you should jugge if pointer is NULL before delete it and set ad nullptr atfer.

shulianghe
  • 94
  • 1
  • 3