«

Hi,

Is this code fundamentally broken?

class B
{
}

class D: public B
{
}


void proc(B* b)
{
D* d = dynamic_cast<D*>b;
// do things to d
}

void f()
{
B* b = new B;
proc (b);
delete b;
}



I know it isn't exception safe and should be using RAII (legacy code
).
I'm more worried about which destructor gets called. I assume ~D()
*doesn't* get called. I assume I should be deleteing D (or better using
RAII)
rather than B.

Unfortunatly B is heavily derived from and changing it will have to be
in a
lot of places. But then not fixing it...


--
Nick Keighley

Nick Keighley wrote:

> Hi,
>
> Is this code fundamentally broken?
>
> class B
> {
> }
>
> class D: public B
> {
> }
>
>
> void proc(B* b)
> {
> D* d = dynamic_cast<D*>b;
> // do things to d
> }
>
> void f()
> {
> B* b = new B;

Do you mean:

B* b = new D;

> proc (b);

If b does not point to an object of type D, the dynamic_cast in proc should
barff.

> delete b;
> }
>
>
>
> I know it isn't exception safe and should be using RAII (legacy code
> ).
> I'm more worried about which destructor gets called. I assume ~D()
> *doesn't* get called. I assume I should be deleteing D (or better using
> RAII)
> rather than B.
>
> Unfortunatly B is heavily derived from and changing it will have to be
> in a lot of places. But then not fixing it...

The class B needs to have a virtual destructor. Then deleting derived
objects via B* will work as it should.


Best

Kai-Uwe Bux

On 24 Jan, 08:54, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
> Nick Keighleywrote:

> > Is this code fundamentally broken?
>
> > class B
> > {
> > }
>
> > class D: public B
> > {
> > }
>
> > void proc(B* b)
> > {
> > D* d = dynamic_cast<D*>b;
> > // do things to d
> > }
>
> > void f()
> > {
> > B* b = new B;
>
> Do you mean:
>
> B* b = new D;

er, no I don't think so. I intended to new the base class, cast to the
derived class
then delete the base class.

> > proc (b);If b does not point to an object of type D, the dynamic_cast in proc should
> barff.

?? I thought you could do this with a dynamic_cast?

> > delete b;
> > }
>
> > I know it isn't exception safe and should be using RAII (legacy code
> > ).
> > I'm more worried about which destructor gets called. I assume ~D()
> > *doesn't* get called. I assume I should be deleteing D (or better using
> > RAII) rather than B.
>
> > Unfortunatly B is heavily derived from and changing it will have to be
> > in a lot of places. But then not fixing it...
>
> The class B needs to have a virtual destructor. Then deleting derived
> objects via B* will work as it should.

in reality it does have a virtual DTOR


--
Nick Keighley

Nick Keighley wrote:

> On 24 Jan, 08:54, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
>> Nick Keighleywrote:
>
>> > Is this code fundamentally broken?
>>
>> > class B
>> > {
>> > }
>>
>> > class D: public B
>> > {
>> > }
>>
>> > void proc(B* b)
>> > {
>> > D* d = dynamic_cast<D*>b;
>> > // do things to d
>> > }
>>
>> > void f()
>> > {
>> > B* b = new B;
>>
>> Do you mean:
>>
>> B* b = new D;
>
> er, no I don't think so. I intended to new the base class, cast to the
> derived class then delete the base class.

If you only have a B, then why is it a problem that it's not destroyed by
D's destructor?

>> > proc (b);If b does not point to an object of type D, the
>> > dynamic_cast in proc should barff.
>
> ?? I thought you could do this with a dynamic_cast?

Do what? The result of a dynamic_cast<D*>(b) is as follows:

if the parameter actually points to a D, return a pointer to it
otherwise return a null pointer

>> > delete b;
>> > }

Nick Keighley wrote:

> On 24 Jan, 08:54, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
>> Nick Keighleywrote:
>
>> > Is this code fundamentally broken?
>>
>> > class B
>> > {
>> > }
>>
>> > class D: public B
>> > {
>> > }
>>
>> > void proc(B* b)
>> > {
>> > D* d = dynamic_cast<D*>b;
>> > // do things to d
>> > }
>>
>> > void f()
>> > {
>> > B* b = new B;
>>
>> Do you mean:
>>
>> B* b = new D;
>
> er, no I don't think so. I intended to new the base class, cast to the
> derived class
> then delete the base class.
>
>> > proc (b);If b does not point to an object of type D, the
>> > dynamic_cast in proc should
>> barff.
>
> ?? I thought you could do this with a dynamic_cast?

There seems to be a missunderstanding about dynamic_cast<>. In your
scenario, D is derived from B. That means that every D* automatically
qualifies as a B*. Therefore, if you have a variable of type B*, its value
may actually point to a D object. Sometimes, you might know that the actual
pointee is of type D and you may want to access parts of the object that
are inaccessible through B. In this case, you can use a pointer cast
(static_cast<D*> or dynamic_cast<D*>) to tell the compiler that the B*
variable holds a value that actually is a D* and that the compiler should
not treat it as such.

Now, what happens if the B* variable does not actually hold a D* and you
cast? Well, if you use a static_cast<D*> you get undefined behavior. If you
use a dynamic_cast<D*> the conversion will yield a null pointer:

#include <typeinfo>
#include <iostream>
#include <iomanip>

struct B {

virtual ~B () {};

};

struct D : B {};

int main ( void ) {
try {
B* bp = new B;
D* dp = dynamic_cast<D*>( bp );
std::cout << std::boolalpha << ( bp == 0 ) << '\n';
std::cout << std::boolalpha << ( dp == 0 ) << '\n';
}
catch ( std::bad_cast & e ) {
std::cout << e.what() << '\n';
}
try {
B* bp = new B;
D* dp = static_cast<D*>( bp ); // UB: everything from now on is bogus
std::cout << std::boolalpha << ( bp == 0 ) << '\n';
std::cout << std::boolalpha << ( dp == 0 ) << '\n';
}
catch ( std::bad_cast & e ) {
std::cout << e.what() << '\n';
}
}

Either way, casting from B* to D* does not magically turn the pointee into a
D object if it wasn't one to begin with.


Best

Kai-Uwe Bux

On 24 Jan, 10:00, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
> Nick Keighleywrote:
> > On 24 Jan, 08:54, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
> >> Nick Keighleywrote:

if the quoting is messed up, google has just been "upgraded" again.
Do those people test anything?

> >> > Is this code fundamentally broken?
>
> >> > class B
> >> > {
> >> > }
>
> >> > class D: public B
> >> > {
> >> > }
>
> >> > void proc(B* b)
> >> > {
> >> > D* d = dynamic_cast<D*>b;
> >> > // do things to d
> >> > }
>
> >> > void f()
> >> > {
> >> > B* b = new B;
>
> >> Do you mean:
>
> >> B* b = new D;
>
> > er, no I don't think so. I intended to new the base class, cast to the
> > derived class then delete the base class.
>
> >> > proc (b);If b does not point to an object of type D, the
> >> > dynamic_cast in proc should barff.
>
> > ?? I thought you could do this with a dynamic_cast?There seems to be a missunderstanding about dynamic_cast<>.
>
> In your
> scenario, D is derived from B. That means that every D* automatically
> qualifies as a B*. Therefore, if you have a variable of type B*, its value
> may actually point to a D object. Sometimes, you might know that the actual
> pointee is of type D and you may want to access parts of the object that
> are inaccessible through B. In this case, you can use a pointer cast
> (static_cast<D*> or dynamic_cast<D*>) to tell the compiler that the B*
> variable holds a value that actually is a D* and that the compiler should
> not treat it as such.
>
> Now, what happens if the B* variable does not actually hold a D* and you
> cast? Well, if you use a static_cast<D*> you get undefined behavior. If you
> use a dynamic_cast<D*> the conversion will yield a null pointer:
>
> #include <typeinfo>
> #include <iostream>
> #include <iomanip>
>
> struct B {
>
> virtual ~B () {};
>
> };struct D : B {};
>
> int main ( void ) {
> try {
> B* bp = new B;
> D* dp = dynamic_cast<D*>( bp );
> std::cout << std::boolalpha << ( bp == 0 ) << '\n';
> std::cout << std::boolalpha << ( dp == 0 ) << '\n';
> }
> catch ( std::bad_cast & e ) {
> std::cout << e.what() << '\n';
> }
> try {
> B* bp = new B;
> D* dp = static_cast<D*>( bp ); // UB: everything from now on is bogus
> std::cout << std::boolalpha << ( bp == 0 ) << '\n';
> std::cout << std::boolalpha << ( dp == 0 ) << '\n';
> }
> catch ( std::bad_cast & e ) {
> std::cout << e.what() << '\n';
> }
>
> }
>
> Either way, casting from B* to D* does not magically turn the pointee into a
> D object if it wasn't one to begin with.

right. Brain fart. The real code did indeed new D.


class B
{
}

class D: public B
{
}


void proc(B* b)
{
D* d = dynamic_cast<D*>b;
// do things to d
}

void f()
{
B* b = new D;
proc (b);
delete b;
}


my only excuse (besides temporary insanity) is that the new was hidden
in a factory method.

thankyou for you patience.


--
Nick Keighley

Nick Keighley wrote:
> On 24 Jan, 10:00, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
>> Nick Keighleywrote:
>>> On 24 Jan, 08:54, Kai-Uwe Bux <jkherci...@gmx.net> wrote:
>>>> Nick Keighleywrote:
>
> if the quoting is messed up, google has just been "upgraded" again.
> Do those people test anything?
>
>>>>> Is this code fundamentally broken?
>>>>> class B
>>>>> {
>>>>> }
>>>>> class D: public B
>>>>> {
>>>>> }
>>>>> void proc(B* b)
>>>>> {
>>>>> D* d = dynamic_cast<D*>b;
>>>>> // do things to d
>>>>> }
>>>>> void f()
>>>>> {
>>>>> B* b = new B;
>>>> Do you mean:
>>>> B* b = new D;
>>> er, no I don't think so. I intended to new the base class, cast to the
>>> derived class then delete the base class.
>>>>> proc (b);If b does not point to an object of type D, the
>>>>> dynamic_cast in proc should barff.
>>> ?? I thought you could do this with a dynamic_cast?There seems to be a missunderstanding about dynamic_cast<>.
>> In your
>> scenario, D is derived from B. That means that every D* automatically
>> qualifies as a B*. Therefore, if you have a variable of type B*, its value
>> may actually point to a D object. Sometimes, you might know that the actual
>> pointee is of type D and you may want to access parts of the object that
>> are inaccessible through B. In this case, you can use a pointer cast
>> (static_cast<D*> or dynamic_cast<D*>) to tell the compiler that the B*
>> variable holds a value that actually is a D* and that the compiler should
>> not treat it as such.
>>
>> Now, what happens if the B* variable does not actually hold a D* and you
>> cast? Well, if you use a static_cast<D*> you get undefined behavior. If you
>> use a dynamic_cast<D*> the conversion will yield a null pointer:
>>
>> #include <typeinfo>
>> #include <iostream>
>> #include <iomanip>
>>
>> struct B {
>>
>> virtual ~B () {};
>>
>> };struct D : B {};
>>
>> int main ( void ) {
>> try {
>> B* bp = new B;
>> D* dp = dynamic_cast<D*>( bp );
>> std::cout << std::boolalpha << ( bp == 0 ) << '\n';
>> std::cout << std::boolalpha << ( dp == 0 ) << '\n';
>> }
>> catch ( std::bad_cast & e ) {
>> std::cout << e.what() << '\n';
>> }
>> try {
>> B* bp = new B;
>> D* dp = static_cast<D*>( bp ); // UB: everything from now on is bogus
>> std::cout << std::boolalpha << ( bp == 0 ) << '\n';
>> std::cout << std::boolalpha << ( dp == 0 ) << '\n';
>> }
>> catch ( std::bad_cast & e ) {
>> std::cout << e.what() << '\n';
>> }
>>
>> }
>>
>> Either way, casting from B* to D* does not magically turn the pointee into a
>> D object if it wasn't one to begin with.
>
> right. Brain fart. The real code did indeed new D.
>
>
> class B
> {
> }
>
> class D: public B
> {
> }
>
>
> void proc(B* b)
> {
> D* d = dynamic_cast<D*>b;
> // do things to d
> }
>
> void f()
> {
> B* b = new D;
> proc (b);
> delete b;
> }
>
>
> my only excuse (besides temporary insanity) is that the new was hidden
> in a factory method.
>
> thankyou for you patience.
>
>

Not sure if your question still stands, but the dynamic cast doesn't
really cause any problems here (it doesn't really "do" anything, in
fact, since b is a D). The only glaring pitfall in the code you've
shown is the lack of a virtual destructor for B.