0

Here is a simple code:

#include <iostream>
#include <string>

typedef struct Car{
    std::string model;
} Car;

std::string get_model() {
    std::string str = "Maserati";
    return str;
}

int main() {
    const int nCars = 2;
    //Car *list = new Car[nCars];                   // works everywhere g++/VC++
    Car *list = (Car *)malloc(nCars * sizeof(Car)); // works in g++, not VC++

    list[0].model = get_model();
    std::cout << "model=" << list[0].model << std::endl;    
    // delete[] list;
    free(list);
    return 0;
}

There is no problem when I used malloc() or new in g++. However, malloc() does not work in Visual C++. Should I use new always when I allocate the C++ class object?

(a debtor)<><

Youngsup Kim
  • 2,235
  • 5
  • 13
  • 18
  • 9
    Do not use `malloc` in C++, it does not call the constructor, therefore the objects are invalid. – mch Feb 07 '19 at 12:37
  • 4
    There were problems with g++ too, but the nature of undefined behaviour is that it doesn’t always produce an immediately observable effect. – molbdnilo Feb 07 '19 at 12:37
  • 1
    I suggest you free *list (or ev. delete it using new) for sake of completeness/elegance. – L.C. Feb 07 '19 at 14:11
  • Molbdnilo, that's what I observed. Thanx. L.C, let me add free() for completeness, as you suggested. Thanx. – Youngsup Kim Feb 09 '19 at 02:18

3 Answers3

3

You are allocating memory without calling a constructor or calling the destructor when the object is about to be removed. This is what new[] and delete[] does for you, so use them - or better yet, use smart pointers - or even better, a standard container, like std::vector to keep the objects for you.

Your code with the missing parts added:

#include <iostream>
#include <string>

struct Car {
    std::string model;

    Car() { std::cout << "ctor\n"; }
    ~Car() { std::cout << "dtor\n"; }
};

int main() {
    const int nCars = 2;

    // allocate memory
    Car *list = (Car *)malloc(nCars * sizeof(Car));

    // manually calling constructors
    for(int i=0; i<nCars; ++i) {
        new(&list[i]) Car();
    }

    // use objects here

    // manually calling destructors
    for(int i=0; i<nCars; ++i) {
        list[i].~Car();
    }

    // freeing memory
    free(list);
}

Compare with using new[] and delete[]:

int main() {
    const int nCars = 2;

    // create cars
    Car* list = new Car[nCars];

    // use objects here

    // delete cars
    delete[] list;
}

Compare with using a container:

int main() {
    const int nCars = 2;

    // create cars
    std::vector<Car> list(nCars);

    // use objects here
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • The rule of thumb is to use new if C++ objects involved. It is simpler. Thanx. – Youngsup Kim Feb 07 '19 at 14:00
  • 2
    If the choice is between `malloc/free` and `new/delete`, then yes! But `new/delete` should also be avoided if you can. You can almost always replace them with a container or smart pointer (like `std::unique_ptr`) that will destroy the elements even if an exception occurs etc. – Ted Lyngmo Feb 07 '19 at 14:21
2

Yes.

While it is wrong to say, "Never use malloc() in C++," it is definitely true that you should never use malloc() to instantiate a class.

Keep in mind that C++ is, in a sense, a hybrid language in that it effectively supports an almost complete subset of C and adds the superset of C++ functionality. malloc() has a role to play when using built-in types like int, char, float, etc.

For objects, however, new must be used. It might be true that you have found that malloc() works in many cases, but new and delete will cause constructors and destructors to be called, which will never happen with malloc() and free().

hyde
  • 60,639
  • 21
  • 115
  • 176
David Hoelzer
  • 15,862
  • 4
  • 48
  • 67
-3

The problem here is because you're allocating a memory for the std::string of your Car struct but do not call the std::string constructor.

You should call a placement new for each item in the array to call the constructor and initialize the std::string field in the Car struct:

int main() {
    const int nCars = 2;
    Car* list = (Car *)malloc(nCars * sizeof(Car));

    for (int i = 0; i < nCars; ++i)
        new(&list[i])Car();

    list[0].model = get_model();
    std::cout << "model=" << list[0].model << std::endl;
}

-- ORIGINAL ANSWER --

Here's my original answer (which is inorrect due to an extra overhead that may be required for arrays: https://en.cppreference.com/w/cpp/language/new#Allocation)

If you have to use malloc then I suggest you use a in-place constructor with the returned memory block:

int main() {
    const int nCars = 2;
    Car *list = new (malloc(nCars * sizeof(Car)))Car[nCars];

    list[0].model = get_model();
    std::cout << "model=" << list[0].model << std::endl;
    return 0;
}
Marcin Zawiejski
  • 1,123
  • 1
  • 9
  • 23
  • 2
    `new (malloc(/*...*/)) class-type[/*...*/]`: this is heresy! I wouldn't suggest it :D – YSC Feb 07 '19 at 12:45
  • Please don't. This is another UB and memory leak piled *on top* of the original snippet. – Passer By Feb 07 '19 at 12:55
  • 2
    Yes. Basically, you don't know [how much space needs a placement new on an array](https://stackoverflow.com/q/8720425/5470596). – YSC Feb 07 '19 at 13:04
  • Thanks @YSC. I've updated my answer now but kept the original one as a warning :) Can you see any reason why a per-item placement new won't work here? – Marcin Zawiejski Feb 07 '19 at 13:36