How to delete a struct? 1

[Dannybe2]

I'm using Visual Studio .Net and I have run the memory leak test on my program. It is failing to release a few structures I have created in my program and I'm not sure how I'm supposed to do it.

Here is a simple version of the program I am using:

struct s
{
bool a;
char* b;
int c;
};

void main()
{
s* cont = new s();

while(1)
{
fillCont(s);
}
delete cont;
}

void fillCont(s* cont)
{
r->a = true;
r->b = "Item1";
r->c = 100;
}

Any help would be appreciated,
Thanks

 

[ArkM]

1. This snippet can't raise memory leak because of it can't be compiled (r at fillCont() is not declared. If you (we) correct the error, it runs forever on while(1) - without memory leak too...
2. Well, new/delete sequence is OK:

s* cont = new s;
...
delete cont;
// or
s* arrs = new s[n];
...
delete []arrs;

What's the true problem?...

 

[Dannybe2]

Yeah of course I meant s not r.

OK, that makes sense to me, so I'll try and expand on it a bit to try and find the problem.

I'm actually assigning the data in s from records pulled out of a database, so the function fillCont from above looks more like this:

void fillCont(s* cont)
{
  record = getRecord();
  s->a = atoi(record[0]);
  s->b = strdup(record[1]);
  s->c = atoi(record[2]);
}

The data being allocated is the same as the data I'm setting here, so I'm guessing that these allocations aren't being freed up...

 

[ArkM]

Oh my!... cont->b, not s->b.

It's your problem:

cont->b = strdup(...);

But struct s has no explicit destructor. Default destructor does not deallocate b pointer (you have memory leak).
Write constructor and destructor of you struct:

struct S // Traditionally class names starts with Capital
         //letter...
{
S():b(0){}
~S() { free(b); } // use free after strdup(), not delete b
};

Warning: if you assign not-dynamically-allocated pointer to b member (as in cont->b = "Static memory string"), you can't free such pointers. So be careful. It's this (dangerous) class (struct) design defect...

 

[Dannybe2]

Of course it should have been cont not s, I'm confusing myself trying to translate my code into a simpler version.

Your second comment is exactly what I was after however, I thought there might have been a way to have constructors and deconstructors in a struct. I had been trying to delete b as well.

I'm still a little confused by the syntax of your code though.

Am I able to define functions in a struct just as I would in a class, for example like the code below:

//h file
struct S
{
    bool a;
    char* b;
    int c;

    S();
    ~S();
};

//cpp file
S:S()
{
    b = 0;
}

S:~S()
{
    free(b);
}

Having the destructor would mean I would have to delete it and then reinitialize it every loop right?

or maybe just say:

if(b)
{
    free(b);
}
cont->b = strdup(record[1]);

Would that not perhaps be more efficient?

Thanks a lot for your help so far.

 

[cpjust]

Don't mix new & free() or malloc() & delete! Otherwise very bad things will happen.

Yes, you can have functions in a struct. A class and struct are the same thing except by default everything in a class is private while everything in a struct is public.

Since the b member of your struct is a pointer, you need to delete it yourself before you overwrite it.

 

[ArkM]

Member function defined (== has a body) in a class/struct declaration treats as inline.
Strictly speaking, you may skip null test for b: free(0) is legal (do nothing).
Cpjust is right (see also my comment in S destructor before: never mix free(p) after p = new and delete p after p = malloc (strdup() is non-standard function from malloc family). Better use your own function, for example:

char* StrDup(const char* cstr)
{
  char* p = 0;
  if (cstr)
  {
     int sz = strlen(cstr) + 1;
     memcpy(p = new char[sz],cstr,sz);
  }
  return p;
}
/// Then delete a pointer with delete []p;

It's a very erron-prone approach to use this class. You may forget to free b or may assign not dynamic allocated pointer then try to free it. Better use std::string member to save texts in this member: std::string class has its own destructor and assign char* op.
Of course, in that case you may also define operator =(const char*) for this structure (to assign a string to b member), for example:

S& S::operator =(const char* cstr)
{
   if (cstr != b)
   {
      delete []b;
      b = StrDup(cstr);
   }
   return *this;
}
// Now legal:
S s;
...
s = "This is a string";
/// Don't forget to free this member in destructor.

In that case better use true class (not a structure) with private b (define operator char*() { return b; } to access this member).
Moral: this structure design is wrong. Don't use it. Try another solution.