 |
C++Talk.NET
C++ language newsgroups
|
| View previous topic :: View next topic |
| Author |
Message |
Giovanni Trematerra
Guest
|
 Posted: Thu Apr 07, 2005 12:00 pm Post subject: Horrible design or undefined behavior |
 |
|
Hi,
please take a look this class design:
//file.h
class MyClass
{
public:
static MyClass * Create(int nPixelDynamicIn)
{
return new(nPixelDynamicIn) MyClass;
}
static void Destroy(MyClass *p)
{
delete p;
}
/*
OTHER Methods
*/
protected:
int m_nPixelDynamicIn;
unsigned char m_Table[1];
private:
// non implementate
MyClass(const MyClass &);
void operator = (const MyClass &);
// non sono chiamabili dall'esterno
MyClass();
~MyClass();
void * operator new(size_t sz, int nPixelDynamicIn);
void operator delete(void *, int nPixelDynamicIn);
void operator delete(void *);
};
//file.cpp
#include "Lut.h"
void * MyClass::operator new(size_t sz, int nPixelDynamicIn)
{
MyClass *p = reinterpret_cast<MyClass *>(new char [sizeof(MyClass) +
nPixelDynamicIn - 1]);
p->m_nPixelDynamicIn = nPixelDynamicIn;
return p;
}
MyClass::MyClass ()
{
//initialize m_Table
for (int i = 0; i < m_nPixelDynamicIn; i++)
{
m_Table[i] = 0;
}
}
MyClass::~MyClass ()
{
}
void MyClass::operator delete (void *p, int)
{
delete [] p;
}
void MyClass::operator delete (void *p)
{
delete [] p;
}
//client code
.....
MyClass *foo;
foo = MyClass::Create (4096);
//DO STUFF
MyClass::Destroy (foo);
===========
In according with Standard C++, the code above may to lead to undefined
behavior when MyClass:Destroy invoke delete operator?
Thanks in advance.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
| Back to top |
|
 |
Ismail Pazarbasi
Guest
|
| Posted: Thu Apr 07, 2005 3:02 pm Post subject: Re: Horrible design or undefined behavior |
|
|
Destroy does not use array form of delete operator (delete[]). It says
"delete p". So "operator delete(void*p)" will be called. it is a
"void*", not MyClass*. So, array form of delete will not be called for
MyClass. However, if MyClass::Destroy would call delete [] p, then it
could cause an access violation. Or, if you could define global delete
operator, as following, and do not define delete operator for MyClass,
then it could cause a problem again. It is not an undefined behavior, I
guess. Run-time automatically generates an access violation and
application crashes, or a method like following could cause recursion.
Applying array delete operator on zero-sized arrays do not cause any
trouble.
void operator delete(void* p)
{
delete [] p; // recurses?
}
MyClass::Destroy(MyClass* p)
{
delete p;
}
Ismail
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
| Back to top |
|
|
Branimir Maksimovic
Guest
|
| Posted: Fri Apr 08, 2005 7:45 am Post subject: Re: Horrible design or undefined behavior |
|
|
Giovanni Trematerra wrote:
| Quote: |
void MyClass::operator delete (void *p, int)
{
delete [] p;
}
void MyClass::operator delete (void *p)
{
delete [] p;
}
In according with Standard C++, the code above may to lead to
undefined
behavior when MyClass:Destroy invoke delete operator?
|
Of course deleting a uncomplete type would result in ub.
I've taken some time to improve your idea.
#include <new> // needed for placement new
class MyClass
{
public:
static MyClass * Create(int nPixelDynamicIn)
{
/* there is no need to overload operator new and delete in this case,
simple placement new will do the thing
*/
void *p = ::operator new(sizeof(MyClass)+nPixelDynamicIn);
return new(p) MyClass(nPixelDynamicIn,static_cast<unsigned
char*>(p)+sizeof(MyClass));
}
static void Destroy(MyClass *p)
{
p->~MyClass();
::operator delete(p);
}
/*
OTHER Methods
*/
protected:
int m_nPixelDynamicIn;
unsigned char* m_Table; // better make this member a pointer
// and append array behind object
// instead of using struct hack
private:
// constructor is convenient way to set data for array
MyClass(int nPixelDynamicIn, unsigned char* pTable);
// non implementate
MyClass(const MyClass &);
void operator = (const MyClass &);
// non sono chiamabili dall'esterno
MyClass();
~MyClass();
};
//file.cpp
MyClass::MyClass (int nPixelDynamicIn, unsigned char* pTable)
: m_nPixelDynamicIn(nPixelDynamicIn), m_Table(pTable)
{
//initialize m_Table
for (int i = 0; i < m_nPixelDynamicIn; i++)
{
m_Table[i] = 0;
}
}
MyClass::~MyClass ()
{
}
int main()
{
//client code
MyClass *foo;
foo = MyClass::Create (4096);
//DO STUFF
MyClass::Destroy (foo);
}
Greetings, Bane.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
| Back to top |
|
|
B. Perlman
Guest
|
| Posted: Sun Apr 10, 2005 8:58 am Post subject: Re: Horrible design or undefined behavior |
|
|
I've left in snippets of your code, interspersed with some questions
and comments. My answer to your question is below.
| Quote: | static MyClass * Create(int nPixelDynamicIn)
{
return new(nPixelDynamicIn) MyClass;
}
|
Note: this function expects pre-allocated memory, as it uses placement
new. Did you mean that, or did you mean to use the new operator which
you defined below?
| Quote: | static void Destroy(MyClass *p)
{
delete p;
snip
void * MyClass::operator new(size_t sz, int nPixelDynamicIn)
{
MyClass *p = reinterpret_cast<MyClass *>(new char
[sizeof(MyClass) +
nPixelDynamicIn - 1]);
p->m_nPixelDynamicIn = nPixelDynamicIn;
return p;
}
|
When is this used? Your client code doesn't use it and your class
definition doesn't use it. Is this an oversight?
| Quote: | snip
//client code
....
MyClass *foo;
foo = MyClass::Create (4096);
//DO STUFF
MyClass::Destroy (foo);
===========
In according with Standard C++, the code above may to lead to
undefined
behavior when MyClass:Destroy invoke delete operator?
|
What is at 4096? I don't know what operating system you are using, if
there is one. If you are in a system like Windows, you can't hard-code
addresses ike that. If you are in an embedded system where you
control the memory directly, you can call Create(...) as you do above,
as long as you ensure that you have sufficient memory at 4096 dedicated
to this object *. However, in this case, I would think that you can't
call "delete" since your object is not in allocated memory.
*(Note: you have to make sure that you don't ever try to create two
different MyClass objects at the same address, as by calling the
function containing "client code" more than once)
Batya
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
| Back to top |
|
|
Ben Hutchings
Guest
|
| Posted: Sat Apr 16, 2005 12:17 am Post subject: Re: Horrible design or undefined behavior |
|
|
Giovanni Trematerra wrote:
| Quote: | Hi,
please take a look this class design:
//file.h
class MyClass
{
public:
static MyClass * Create(int nPixelDynamicIn)
{
return new(nPixelDynamicIn) MyClass;
}
static void Destroy(MyClass *p)
{
delete p;
}
snip
protected:
int m_nPixelDynamicIn;
unsigned char m_Table[1];
private:
snip
MyClass();
~MyClass();
void * operator new(size_t sz, int nPixelDynamicIn);
void operator delete(void *, int nPixelDynamicIn);
void operator delete(void *);
};
//file.cpp
#include "Lut.h"
void * MyClass::operator new(size_t sz, int nPixelDynamicIn)
{
MyClass *p = reinterpret_cast<MyClass *>(new char [sizeof(MyClass) +
nPixelDynamicIn - 1]);
p->m_nPixelDynamicIn = nPixelDynamicIn;
|
I think this assignment has undefined behaviour since the MyClass
constructor hasn't been called.
| Quote: | return p;
}
MyClass::MyClass ()
{
//initialize m_Table
for (int i = 0; i < m_nPixelDynamicIn; i++)
{
m_Table[i] = 0;
|
This has undefined behaviour for i > 0 because it overruns the
array bounds.
<snip>
| Quote: | void MyClass::operator delete (void *p, int)
{
delete [] p;
}
void MyClass::operator delete (void *p)
{
delete [] p;
}
snip
In according with Standard C++, the code above may to lead to undefined
behavior when MyClass:Destroy invoke delete operator?
|
It seems like deleting through a pointer of type void * should be
ill-formed but possibly it has undefined behaviour.
There's a simpler and (I think) legal way to do this:
#include <new>
class MyClass
{
public:
static MyClass * Create(int nPixelDynamicIn)
{
void * mem = new unsigned char[sizeof(MyClass) + nPixelDynamicIn];
return new (mem) MyClass(nPixelDynamicIn);
}
static void Destroy(MyClass *p)
{
p->~MyClass();
delete[] reinterpret_cast<unsigned char *>(p);
}
private:
int m_nPixelDynamicIn;
unsigned char * getTable()
{ return reinterpret_cast<unsigned char *>(this + 1); }
unsigned char const * getTable() const
{ return reinterpret_cast<unsigned char const *>(this + 1); }
MyClass(const MyClass &);
void operator = (const MyClass &);
MyClass(int nPixelDynamicIn)
: m_nPixelDynamicIn(nPixelDynamicIn)
{
unsigned char * table = getTable();
for (int i = 0; i != m_nPixelDynamicIn; ++i)
table[i] = 0;
}
~MyClass() {}
};
--
Ben Hutchings
Having problems with C++ templates? Your questions may be answered by
<http://womble.decadentplace.org.uk/c++/template-faq.html>.
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
|
|
| Back to top |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in to check your private messages
Log in
