6

What is "minimal framework" (necessary methods) of complex object (with explicitly malloced internal data), which I want to store in STL container, e.g. <vector>?

For my assumptions (example of complex object Doit):

#include <vector>
#include <cstring>
using namespace std;
class Doit {
    private:
        char *a;
    public:
        Doit(){a=(char*)malloc(10);}
        ~Doit(){free(a);}
};

int main(){
    vector<Doit> v(10);
}

gives

*** glibc detected *** ./a.out: double free or corruption (fasttop): 0x0804b008 ***
Aborted

and in valgrind:

malloc/free: 2 allocs, 12 frees, 50 bytes allocated.

UPDATE:

Minimal methods for such object are: (based on sbi answer)

class DoIt{
    private:
        char *a;
    public:
        DoIt() { a=new char[10]; }
        ~DoIt() { delete[] a; }
        DoIt(const DoIt& rhs) { a=new char[10]; std::copy(rhs.a,rhs.a+10,a); }
        DoIt& operator=(const DoIt& rhs) { DoIt tmp(rhs); swap(tmp); return *this;}
        void swap(DoIt& rhs) { std::swap(a,rhs.a); }
};

Thanks, sbi, https://stackoverflow.com/users/140719/sbi

Community
  • 1
  • 1
osgx
  • 90,338
  • 53
  • 357
  • 513
  • 5
    You really should use vectors of instead of doing your own mallocs. This is C++ after all. – Joe Mar 23 '10 at 15:37
  • no. but I really need a pointer inside a object, which is malloced. – osgx Mar 23 '10 at 15:38
  • @Joe, My code a but bigger, and I need to store objects in STL containers. This is just easy sample of such code. Real objects have several pointers and ints inside. – osgx Mar 23 '10 at 15:39
  • 2
    @osgx, why insist on malloc? Is there a good reason to not use new? – Glen Mar 23 '10 at 15:40
  • Problem is not with malloc or vector. Problem is on implementing right methods for using class as STL payload. – osgx Mar 23 '10 at 15:43
  • 2
    @osgx: No, the memory doesn't need to be malloced. It should be newed. And, actually, Joe is right and this should be hidden inside a `std::vector`. – sbi Mar 23 '10 at 15:43
  • the difference between new and malloc is for other question. Thanks. – osgx Mar 23 '10 at 15:51
  • 3
    Of course this would be a lot simpler if you did not use C inside C++. Do __NOT__ dynamically allocate memory unless you really need to (and you don't). Use a std::vecrtor. You can initiallise it in the constructor to have 10 characters then the rest of the code is unchanged. – Martin York Mar 23 '10 at 16:11
  • @Martin York, the original Object is container (STL-like, but written from scratch) itself and it uses `::operator new` for creating array of other objects. The `char* ` and malloc used here only **for example**. – osgx Mar 23 '10 at 16:40
  • @Martin York, here malloc and free was very useful to debug my stl-like container. With them I can easy check (with any memory leak detector), if I have balanced constructors and destructors. – osgx Mar 23 '10 at 18:03
  • 1
    See here for a more detailed explanation of sbi response: http://stackoverflow.com/questions/255612/c-dynamically-allocating-an-array-of-objects/255744#255744 – Martin York Mar 24 '10 at 15:30
  • 1
    SBI solution to this problem is fine for char. But if the design is naively used hold another type (i.e. keep char* a, but use placement new put objects into the buffer) has a flaw. You need to take into account the stored types copy constructor failing and tidy up correctly. – Martin York Mar 24 '10 at 15:34
  • @Martin York, I actually had the problem with complex objects, stored within my container, but solved it with element-by-element copying. How must I handle the throw from objects's copy constructor inside copy constructor of container? – osgx Mar 24 '10 at 15:51
  • @osgx: Yes. See here for details: http://stackoverflow.com/questions/2508284/c-copy-constructor-and-shallow-copy/2509272#2509272 – Martin York Mar 24 '10 at 16:08
  • @Martin: Actually, looking at my answer now, I think the biggest mistake I made was not mentioning that the `DoIt` class should not hold raw dynamic arrays at all. Doing elaborate things like using placement `new` to put other objects into a char array should not even be attempted except when you know exactly what you do. (If you fail to obey the Rule of Three it's unlikely you know enough to attempt this.) – sbi Mar 24 '10 at 20:36
  • @sbi: Absolutely. PS. I always call it the rule of four. There are four auto generated methods that need to be implemented correctly. – Martin York Mar 25 '10 at 14:03
  • @Martin, re _Rule of Four:_ I'd rather keep going with _Rule of Three_. There are many cases where you need a def ctor, but none of the other three. – sbi Mar 25 '10 at 15:02
  • @sbi: Yes in most cases its just Constructor. But when you have pointers you better have defined all four compiler generated methods otherwise you will be in trouble. :-) – Martin York Mar 25 '10 at 15:06

3 Answers3

10

Note that Charles has answered your question perfectly.

Anyway, as per the Rule of Three, your class, having a destructor, should have a copy constructor and an assignment operator, too.

Here's how I would do it:

class Doit {
    private:
        char *a;
    public:
        Doit()                   : a(new char[10]) {}
        ~Doit()                    {delete[] a;}
        DoIt(const DoIt& rhs)    : a(new char[10]) {std::copy(rhs.a,rhs.a+10,a);}
        void swap(DoIt& rhs)       {std::swap(a,rhs.a);}
        DoIt& operator=(DoIt rhs)  {swap(rhs); return *this;}
};
sbi
  • 219,715
  • 46
  • 258
  • 445
  • Can you provide the code of copy constructor and assignment operator for my case? – osgx Mar 23 '10 at 15:35
  • I think you're missing a colon after the Doit() constructor – AshleysBrain Mar 23 '10 at 16:00
  • @AshleysBrain, nope Doit() a(new char[10]) {} seems perfectly legal to me. If it was without the {} then I would agree with you. – Konrad Mar 23 '10 at 16:10
  • @Konrad: `Doit() a(new char[10]) {}` certainly missed a `:`. I fixed this. – sbi Mar 23 '10 at 16:14
  • @sbi, not to nit-pick (but I will anyway), but shouldn't operator= be implemented as `DoIt tmp(rhs); swap(tmp); return *this;` – Glen Mar 23 '10 at 16:17
  • Not `Doit() : a(new char[10] () ) { }` ? – MSalters Mar 23 '10 at 16:17
  • @Glen, please, don't blame sbi. He showed me an idea, and this code was written in the answer form without compiling. But I get the idea, and typos are fixed in question update. – osgx Mar 23 '10 at 16:42
  • 1
    @osgx, no one's blaming anyone for anything. Clarifications like this are designed to help the person who asked the question, in this case you, get the best possible answer. – Glen Mar 23 '10 at 16:50
  • @MSalters: What would this bring? You'd initialize the characters which are likely to be overwritten anyway. However, all this would be moot when `std::vector` would have been used - as would be appropriate for this. – sbi Mar 23 '10 at 17:53
6

All types that you use must be CopyConstructible and Assignable.

CopyConstructible for a type T means that if t is a T or a const T then the expression T(t) must produce an equivalent T to the original t; t.~T() must be valid (accessible destructor); and &t must give the address of t as a [const] T*.

Assignable means that for a T, t and a T value u, the expression t = u must make t equivalent to u and be of type T&.

Note that all these requirements are met by simple built-in types and POD-structs. If you do anything non-trivial in a destructor or constructor you must ensure that the copy constructor and copy assignment operator preserver the equivalence semantics.

CB Bailey
  • 755,051
  • 104
  • 632
  • 656
  • 1
    Can you provide a link to table of: which Concepts (Assignable, CopyConstructible, others?) are required for different STL containers (vector/map/priority_queue/hash/etc) to store non-trivial object in them. – osgx Oct 10 '12 at 23:26
  • 2
    They're not "Concepts", they are type _requirements_ and you can find them in [the standard](http://stackoverflow.com/questions/81656/where-do-i-find-the-current-c-or-c-standard-documents). – CB Bailey Oct 11 '12 at 06:10
0

All vector requires is that the object is "assignable", which means that it needs an copy-constructor, destructor, and assignment operator, which are all generated by default if you don't supply them yourself.

As sbi says, if you need one of those functions then you probably need them all. In your case, you'll need to also provide a copy constructor and assignment operator to avoid heap corruption.

Peter Alexander
  • 53,344
  • 14
  • 119
  • 168