Victor Bazarov
10/9/2008 10:16:00 PM
mc wrote:
> Why is there a need for the original's destructor to be called?
Because every object constructed must be at some point destructed.
That's the whole idea of OO, objects have lifetime, they are born and
then they die, all in due time in due order. If you don't allow the
object to be properly disposed of, the resources that object allocates
are not freed. And please don't tell us that you know what resources
that object uses and that it doesn't need to free any. It's the
principles that you have to stick to, otherwise your code is impossible
to maintain.
> After all,
> the original is copied verbatim and consequently the destructor is called
> once as expected.
The constructor is called for two objects, the destructor is only called
for one. That's a mistake in logic.
> Be that as it may, this is indeed quite obfuscated. The reasoning for
> that is that the main caller does not know the object is being allocated
> using new and hence is not supposed to call delete.
I believe you've got a problem of ownership to solve. Whoever creates
the object, owns it, unless it transfers the ownership somehow. No, it
is not in the language specification how to transfer ownership, it's in
the design of the program.
> So we had to find a way
> to free the memory used and went for a new in place (see actual code below):
>
> const Region& MCU::buffer(void) const
> {
> char* p = new char[sizeof(Region)];
> return (*new(p) Region(std::string(TEMP), 1500));
> }
This function seems to transfer the ownership of the object to the
caller. That means it's the caller's responsibility to dispose of the
object properly.
If the object is created dynamically, it's a bad idea to return a
reference to it. A more descriptive interface would return a pointer
instead. Even better to return a smart pointer. Those can transfer
ownership by assignment or initialisation, and if the ownership is not
transferred, the pointer owns the object and destroys it when it is
destroyed itself.
> with the Region ctdt:
>
> Region::Region(const std::string& name, size_t size)
> {
> _this = reinterpret_cast<char *>(this);
> _cache = new unsigned char[size], _offset = 0;
> _fd = ::open(name.c_str(), O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR
> | S_IRGRP | S_IROTH);
> assert(_fd != -1);
> }
>
>
> Output::Buffer::~Buffer()
> {
> if (_fd != -1)
> {
> ::write(_fd, (char *)_cache, _offset);
> ::close(_fd);
> }
> delete[] _cache;
> delete[] _this;
> }
>
>
>
> Does that make sense?
A little, I suppose. It seems that somebody designed your class to be
an owner of another instance of the same class, which it's responsible
for disposing of. It's very, very obscure. And in that sense, when you
construct a copy from that hanging instance, why don't you save a
pointer to the object instead of the pointer to void? Using your
original terms,
class Foo
{
Foo const *pPrototype;
public:
Foo() : pOther(0) {}
Foo(Foo const& rP) : pPrototype(&rP) // taking ownership here
{ /* whatever */ }
/*virtual?*/ ~Foo() { delete pPrototype; }
};
The instance constructed by your 'MCU::foo' function will have the
'pPrototype' member set to NULL, which is OK to 'delete'. The
copy-constructed instance will take on removing the prototype instance
along when destroyed.
V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask