0

I have to use malloc to allocate memory. I have a custom class that needs a custom operator=. Let's say it is A:

class A {
public:
  int n;
  A(int n) : n(n) {}
  A& operator=(const A& other) {
   n = other.n;
   return *this;
  }
};

I allocate memory with malloc:

int main() {
   A* a = (A*) malloc(sizeof(A));
   A b(1);

   //Is it safe to do this as long as I copy everything in operator=?
   *a = b;

   //Clean up
   a->~A();
   free(a);
   return 0;
}

I know I can also use placement new:

a = new (a) A(b);

Is it safe to copy a custom class to uninitialized memory?

Thanks

Pedro Palacios
  • 153
  • 9
  • 2
    What makes you think you have to use `malloc` ... – Fantastic Mr Fox Apr 30 '18 at 12:34
  • I was working on a embedded system where I thought new wasn't implemented. I implemented a vector class with malloc and I was greeted by some obscure runtime errors, which disappeared when I changed it to new. Now I'm just curious, because I haven't found this anywhere. – Pedro Palacios Apr 30 '18 at 12:37
  • The problem isn't malloc, the problem is there is no object `A` at that spot unless you create it properly. Assignment relies on it being created already. The more complex your code becomes, the more likely this will pop up and bite you. – StoryTeller - Unslander Monica Apr 30 '18 at 12:40

3 Answers3

5

Placement new is correct

using A& operator=(const A& other) with a non constructed "this" is incorrect (imagine if you have a non trivial type as std::string inside A the affectation should destroy a non initialized string before affecting the new value).

Once you have do placement new, you can use assignment.

auto p = new (a) A;
*p = b; // ok
Jarod42
  • 203,559
  • 14
  • 181
  • 302
0

No, it is not "safe" to do this. The assignment operator copies its argument into an existing object. A constructor turns raw memory into an object. When you're dealing with raw memory you don't have an object, so assigning to it doesn't make any sense. You must use a constructor. So, placement new is the way to go.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
0

As already answered, copy assignment (or any member function) on a non-existing object is not OK.

Besides placement new, you could instead call std::uninitialized_copy_n or std::uninitialized_copy(which will do the placement construction internally. This is handy when you need to copy-construct multiple objects into uninitialized memory. Example for your single object:

std::uninitialized_copy_n(&b, 1, a);
eerorika
  • 232,697
  • 12
  • 197
  • 326
  • The real question then becomes how would you destruct the objects, e.g. std::uninitialized_delete_n... – Gregory Morse May 28 '22 at 17:07
  • Actually I suppose C++ 17 did this with std::destroy_n otherwise maybe std::for_each with std::destroy_at (nope C++ 20) - so ::delete(ptr) would do it. – Gregory Morse May 28 '22 at 17:13
  • @GregoryMorse the storage is no longer uninitialized after you have constructed the objects, so `std::uninitialized_delete_n` wouldn't make sense. The correct solution is to simply call the destructor directly which is what Pedro does in their example. Use any loop to repeat for multiple objects. destroy_n and _at see nice shorthands to do this if you can use C++17. ::delete(ptr) is wrong. It releases the memory and does not destroy the objects. – eerorika May 28 '22 at 17:55
  • Agreed the destructor is the correct way. The new C++17 and C++20 functions do just that. – Gregory Morse May 28 '22 at 18:50