-1

I got following problem. I want to resize my array of pointers on structure ( car ) . I got following code.

 Class Car{
      ...
    char * Owner_Name; 
    char * carID
    };

 Class Register {
  ...

  int CarCNT;
  int car_num_default;
  Car ** dataBase;

    Register ()
   {  //constructor
     car_num_default = 5 ; //for example;
     dataBase = new Car * [car_num_default]; 
   }

};

Now when I add 6th. car I need to resize my array of pointer to car. How should I do that without create any memory leak ? Or memory error ? :) I tried folowing code but it makes some memory leaks..

void Register:: Add ( const char * carID, const char * owner)
{
   if (carCNT == car_num_default) // now it's full array need resize 
       {  
          car ** tmp = new car * [carCNT]; //create new array ; 
                for(int i = 0 ; i < carCNT;i++)
                 tmp[i] =  new car(databaze[i]->car_ID,databaze[i]->owner_name);

        free(database); //free memory and than alloc new bigger 

           database = new car * [car_num_default * 5];
            for(int i = 0; i < carCNT; i++)
             data_by_RZ[i] = tmp [i];

            free(tmp);
          car_num_def = car_num_def * 5;
    }

     databaze[carCNT] = new car(....);
     carCNT++;

}

Thanks for any help!

adam_
  • 25
  • 2
  • 4
    This does not smell like `c`. – Sourav Ghosh Apr 15 '15 at 14:32
  • 2
    Frankly, this doesn't smell exactly like c++ either. There is no keyword `Class`. – eerorika Apr 15 '15 at 14:38
  • possible duplicate of [Is there any danger in calling free() or delete instead of delete\[\]?](http://stackoverflow.com/questions/1612031/is-there-any-danger-in-calling-free-or-delete-instead-of-delete) – eerorika Apr 15 '15 at 14:43
  • Without STL, you can always allocate on heap and do realloc(). But if the objects held in the array are class objects, you want new and delete to run constructors/destructors. – Erik Alapää Apr 15 '15 at 14:45
  • @dragon_Lord_Of_Math If you can't use STL, then create your own. Write your own vector class (it doesn't have to have all the bells and whistles of `std::vector`), and once you've written it, you can use it anywhere where you "can't use STL" . What you're doing now is IMO not useful as all you're doing are these one-off allocation and deallocation routines in the middle of business logic. – PaulMcKenzie Apr 15 '15 at 15:00

2 Answers2

1

Here's a list of obvious bugs in your memory management:

  • You allocate with new[] but deallocate with free. See Is there any danger in calling free() or delete instead of delete[]?
  • On reallocation, you create new car instances and copy the data of existing car objects instead of copying the pointers to the existing car obejcts. This causes all the previous cars objects to leak. This bug is only when copying database to the tmp table. The copy from tmp to the new database would be correct if tmp contained the pointers to the old car objects.
  • You needlessly create a tmp array and copy the database to it. You should simply create the new, bigger array, copy, deallocate the old and then set the database pointer. This bug does not cause a leak, but is entirely pointless and does waste memory bandwidth.*

* Here's the code as requested:

Car** tmp = new Car*[car_num_default * 5];
for(int i = 0; i < CarCNT; i++)
    tmp[i] = database[i];
delete[] database;
database = tmp;
Community
  • 1
  • 1
eerorika
  • 232,697
  • 12
  • 197
  • 326
  • Could you please post a bit of code. I tried to do "You should simply create the new, bigger array, copy, deallocate the old and then set the database pointer." before but the resolution was seqmentation fault... – adam_ Apr 15 '15 at 15:03
  • @dragon_Lord_Of_Math Then you didn't do it right. It could look [like this](http://pastebin.com/wjGtGFxm). – WhozCraig Apr 15 '15 at 15:09
0

Remember that functions destroy their parameters after the function finishes its execution.

Since you are passing carID and owner as pointers and inserting them into the array, the values are destructed after execution and the pointers placed inside the array become invalid causing the leak. Use pointers only when you want to make a change to the pointer itself but never store it as it will soon be destructed.

It seems that you are using pointers in places you shouldnt or where you don't have to. Here is a simpler version of your code that does the same:

Class Car{
      ...
    char  Owner_Name; 
    char  carID;
    };

 Class Register {
  ...

  int CarCNT;
  int car_num_default;
  Car * dataBase;

    Register ()
   {  //constructor
     car_num_default = 5 ; //for example;
     dataBase = new Car [car_num_default]; 
   }

};
void Register:: Add ( const char  carID, const char owner)
{
   if (CarCNT == car_num_default) // now it's full array need resize 
       {  
          car * tmp = new car  [carCNT]; //create new array ; 
                for(int i = 0 ; i < carCNT;i++)
                 tmp[i] =  new car(dataBase[i].car_ID,dataBase[i].owner_name);

        free(dataBase); //free memory and than alloc new bigger 

           dataBase = new car  [car_num_default * 5];
            for(int i = 0; i < carCNT; i++)
             data_by_RZ[i] = tmp [i];

            free(tmp);
          car_num_def = car_num_def * 5;
    }
    Car x(...);
    dataBase[CarCNT] = x;
    carCNT++;

}

Finally, I have four points to make:

1) This doesnt seem like C++ or you are not using proper naming

2) I am not sure how this code complied since most variable names are incorrect (i tried to fix what i came across).

3) Excessive use of pointers (memory locations in other words) is the main cause of memory leaks. Handle them with care and only use them when you have to.

4) IFFF you have to use pointers that much then add destructors (Anti-constructor) like this ~Car or ~Register on any class that is used as a pointer to signal when the element is destroyed. That way you know where the leak is taking place by writing to the console or handle the destruction gracefully.

Hope that helped

Tarek
  • 687
  • 5
  • 11