0

below you will find a code (that compiles/runs), which in brief invokes a function which allocates an array dynamically on the heap.

#include "stdafx.h"

#include <stdio.h>

class A
{

    public:

    A()
    {
        printf ( "constructor has been called !!! \n" );
    }

    ~A()
    {
        printf ( "destructor has been called !!! \n" );
    }

    char* f( void )
    {

        //char *data_raw = NULL;

        data_raw = NULL;

        data_raw = new char [ 20 ];

        for( int i = 0; i < 20; i++ )
        {
            data_raw[ i ] = '0';    
        }

        data_raw[  0 ] = 'h';
        data_raw[  1 ] = 'e';
        data_raw[  2 ] = 'l';
        data_raw[  3 ] = 'l';
        data_raw[  4 ] = 'o';
        data_raw[  5 ] = ' ';
        data_raw[  6 ] = 'w';
        data_raw[  7 ] = 'o';
        data_raw[  8 ] = 'r';
        data_raw[  9 ] = 'l';
        data_raw[ 10 ] = 'd';
        data_raw[ 11 ] = '!';

        return data_raw;

    } //data raw ptr-var is destroyed

    private:

        char *data_raw;

};


int  _tmain( int argc, _TCHAR* argv[] )
{

    char *data = NULL;

    printf( "data: %c", data );

    A a;

    data = a.f();

    delete [] data;  

    return 0;

}

My questions:

1) regarding destroying the memory allocated dynamically, should I use delete or delete [ ] ? both of them compile...

2) when I use the former ( delete ), the class destructor gets invoked, but not when I use delete [] ?

einpoklum
  • 86,754
  • 39
  • 223
  • 453
ekremer
  • 313
  • 3
  • 20
  • 2
    `new[]` always uses `delete[]` and `new` always uses `delete`. – François Andrieux Sep 25 '18 at 20:04
  • What problem are you trying to solve here? – einpoklum Sep 25 '18 at 20:05
  • `char` doesn't have a _class destructor_?? – πάντα ῥεῖ Sep 25 '18 at 20:05
  • There's no reason to assign `NULL` and then immediately assign the result of a `new` call. If anything you should assign `nullptr` during in the constructor. – tadman Sep 25 '18 at 20:05
  • 1
    One `delete[]` for ever `new[]` 2). Very rarely, C++ has a bunch of mechanisms to help you avoid raw memory management such as `std::vector` and `std::unique_ptr`. – George Sep 25 '18 at 20:06
  • `delete[] data` instead of `delete`. If you use `delete` is like release the memory which is assigned to the address `data[0]` but no the remain.. – Jorge Omar Medra Sep 25 '18 at 20:09
  • @Francois, but why ? because when I review both vars **data** and **data_raw** after the **delete**, memory has been deleted ( I mean the **"Hello World"** ) – ekremer Sep 25 '18 at 20:09
  • 2
    @ekremer It doesn't matter what you observe in that case. If you don't use the right `delete` form you have undefined behavior. The program can do anything at all and is unconstrained. Observations become by definition meaningless. Even if it *looks* like it works, it might stop working at any time for any reason. – François Andrieux Sep 25 '18 at 20:10
  • 1
    @JorgeOmarMedra — the behavior is undefined. It is not possible to release the memory assigned to `data[0]` independently of the rest. – Pete Becker Sep 25 '18 at 20:12
  • @JorgeOmarMedra, OK, great explanation. I will use delete[] ! – ekremer Sep 25 '18 at 20:14
  • @FrançoisAndrieux, thanks for your insightful explanation. – ekremer Sep 25 '18 at 20:17
  • @George, thanks, I am aware of the advantages of the smart pointers, but I am currently working on a code in C++99. My next step is migrate it to C++11 to take advantage of smart pointers, among others. – ekremer Sep 25 '18 at 20:20
  • Micronag: C++98. C got the 1999 version update, the lucky punk. – user4581301 Sep 25 '18 at 20:24
  • @PeteBecker It's possible and it is one of the most common mistake which provoke memory leaks. Considering an array `char* arr = new char[4]`. Using the pointer `char* p = arr` means that `p` points to the first element of the array (`&arr[0]`), that is the same address data `delete arr` receive, that is the reason that `delete[] arr` must be used instead of `delete arr`. To be honest, i was one of those persons who have fallen in the mistake. – Jorge Omar Medra Sep 25 '18 at 20:34
  • Just because you can try to do something does not make it possible. – user4581301 Sep 25 '18 at 20:40
  • @JorgeOmarMedra -- again, no, it's not possible to release the first element of a single block of memory without releasing the rest. What you're talking about is, possibly, heap corruption. The first block is not released; it's part of the full block, and the memory manager won't split them. – Pete Becker Sep 25 '18 at 21:20
  • @PeteBecker, Ok, i understand what you mean. You are talking about the memory handled at OS. When it uses `new X[10]` it reserves a block o memory that must be release in the same way, in block, because it is not possible to try to release one single item of that block, like `&X[0]`, `&X[1]` or `&X[n]`. Looking this at this way, you are right with your observation. The issue about using `delete X` instead of `delete[] X` is that it doesn't throw an exception or error and this situation gives a perception of the operation was made in some way. Thanks. – Jorge Omar Medra Sep 25 '18 at 23:07
  • @JorgeOmarMedra — right, except that I was talking about the memory manager in the standard library. Each application manages its own heap, getting memory from the OS as needed, but usually not giving it back to the OS until the application terminates. – Pete Becker Sep 26 '18 at 02:07

2 Answers2

2

You should simply never ever use a class such as A, particularly not the way you use it.

Specifically:

  • You should not transfer ownership of memory while keeping a pointer to the owned object (unless it's a weak reference). This almost always means you're making the invalid assumption that the pointer is valid, despite the fact that the memory could easily have been deallocated long before the non-owning pointer is used.
  • You should not directly allocate and deallocate memory except in very special cases. Use existing container classes, or if none are appropriate, use smart pointers - std::unique_ptr, std::shared_ptr and std::weak_ptr - as appropriate.
  • You shouldn't pass pointers to allocated memory where the allocated amount (or a lower bound on the allocated amount) is not well-known to the new owner. Either pass references to proper objects, or pass containers using move semantics, or pass spans around. Note that this might happen when you simply just pass values around (due to copy ellisions).
  • You should usually prefer keeping ownership of memory with the creator object, allowing for non-owning access and letting the creator object destroy what it created.

Your code follows none of these guidelines! It's scary and dangerous!

I would rewrite your code so as to follow your guidelines, but since it's not clear why you even need method f or class A, I would end up with just:

#include <iostream>
#include <string>

std::string A_f() { return "hello world"; }

int main( int argc, const char* argv[] )
{
    std::cout << A_f();
}

or just

#include <iostream>

int main( int argc, const char* argv[] )
{
    std::cout << "hello world";
}
einpoklum
  • 86,754
  • 39
  • 223
  • 453
  • thanks for your response. I tried to simplify the code in the function f(). This code actually opens a file and retrieves its size. After that it dynamically creates a char array and reads the file, assigning the data read into the array. Finally it returns the array. – ekremer Sep 25 '18 at 20:34
  • @ekremer: 1. Answers can only address questions. Note that you can give examples calling unimplemented functions, e.g. `auto result = complex_computation(param1, param2);`. 2. It doesn't really matter that you get your string from a file; you would still not want the A class at all; you would simply have a slightly more involved freestanding `A_f()` function. Otherwise my answer would be essentially the same. – einpoklum Sep 25 '18 at 20:45
1

you call delete[] when you initialize an array in the heap with new and you use delete when initializing an object in the heap with new.

on a side note, i would suggest that you delete the members of a class by its owner. you will be asking for a lot of trouble if you do what you doing here. deleting the array outside of the A.

for this example, there is nothing stopping you to call delete[] data_raw in the destructor of the class A. no need to acceses it from the outside of A or even return a pointer to the array.

class A
{
public:
   A()
  {
    printf ( "constructor has been called !!! \n" );
    data_raw = NULL;
    data_raw = new char [ 20 ];
    //....rest ofthe code from f( void )
    //....no need to return data_raw        
  }

  ~A()
  {
    printf ( "destructor has been called !!! \n" );
    delete [] data_raw; 
  }

private:
  char *data_raw;
};
Yucel_K
  • 646
  • 5
  • 15