isn't this dangerous?

Hi,

I found this code in lib/interfaces/iplugin.cpp:


IPlugin::IPlugin( const KComponentData &instance,
QObject *parent )
        : QObject( parent ),
        KXMLGUIClient(), d( new IPluginPrivate )
{
    d->core =
static_cast<KDevelop::ICore*>(parent);
    setComponentData( instance );
}


and I wonder why we need the static_cast here. I think it
would be saver to 
change the constructer to receive a KDevelop::ICore* instead
of a QParent*

Or is there a reason why we can not do this?

Jens

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On Friday 20 April 2007 05:59, Jens Herden wrote:
> Hi,
>
> I found this code in lib/interfaces/iplugin.cpp:
>
>
> IPlugin::IPlugin( const KComponentData &instance,
QObject *parent )
>
>         : QObject( parent ),
>
>         KXMLGUIClient(), d( new IPluginPrivate )
> {
>     d->core =
static_cast<KDevelop::ICore*>(parent);
>     setComponentData( instance );
> }
>
>
> and I wonder why we need the static_cast here. I think
it would be saver to
> change the constructer to receive a KDevelop::ICore*
instead of a QParent*
>
> Or is there a reason why we can not do this?
>
> Jens

It would be safer. I don't know why we couldn't do this.
Hopefully, Alexander 
or Andreas can explain better
-- 
Matt

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 20.04.07 07:27:21, Matt Rogers wrote:
> On Friday 20 April 2007 05:59, Jens Herden wrote:
> > Hi,
> >
> > I found this code in lib/interfaces/iplugin.cpp:
> >
> >
> > IPlugin::IPlugin( const KComponentData
&instance, QObject *parent )
> >
> >         : QObject( parent ),
> >
> >         KXMLGUIClient(), d( new IPluginPrivate )
> > {
> >     d->core =
static_cast<KDevelop::ICore*>(parent);
> >     setComponentData( instance );
> > }
> >
> >
> > and I wonder why we need the static_cast here. I
think it would be saver to
> > change the constructer to receive a
KDevelop::ICore* instead of a QParent*
> >
> > Or is there a reason why we can not do this?
> >
> > Jens
> 
> It would be safer. I don't know why we couldn't do
this. Hopefully, Alexander 
> or Andreas can explain better

Because the instances are not created by us. We can only
give a QObject
to KServiceTypeTrader::createInstanceFromQuery as parent. 

Andreas

-- 
You will meet an important person who will help you advance
professionally.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On Friday 20 April 2007, Andreas Pakulat wrote:
> Because the instances are not created by us. We can
only give a
> QObject to KServiceTypeTrader::createInstanceFromQuery
as parent.

Well, but shouldn't we do a dynamic_cast at least?

Andras

-- 
Quanta Plus developer - http://quanta.kdewebdev.o
rg
K Desktop Environment - http://www.kde.org

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 20.04.07 19:57:27, Andras Mantia wrote:
> On Friday 20 April 2007, Andreas Pakulat wrote:
> > Because the instances are not created by us. We
can only give a
> > QObject to
KServiceTypeTrader::createInstanceFromQuery as parent.
> 
> Well, but shouldn't we do a dynamic_cast at least?

Well, if the dynamic_cast fails the plugin won't work and
from the
plugin destructor I don't think we can abort the plugin
creation (or can
we?).

Somehow I feel we should provide our own factory
implementation for
kdevelop plugins, which checks the QObject* parameter using
dynamic_cast
and returns 0 if its not Core/ICore.

Andreas

-- 
Is that really YOU that is reading this?

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 4/20/07, Jens Herden <jenskdewebdev.org> wrote:
> IPlugin::IPlugin( const KComponentData &instance,
QObject *parent )
>         : QObject( parent ),
>         KXMLGUIClient(), d( new IPluginPrivate )
> {
>     d->core =
static_cast<KDevelop::ICore*>(parent);
>     setComponentData( instance );
> }
>
>
> and I wonder why we need the static_cast here. I think
it would be saver to
> change the constructer to receive a KDevelop::ICore*
instead of a QParent*

I don't think we can change the constructor because we use
KLibLoader::createInstance (called by
KServiceTypeTrader::createInstanceFromQuery which we use in
plugin
loader) to instantiate plugins. createInstance sends
QObject* as a
parent argument to the plugin's constructor, so it looks
like we're
required to have this signature for the plugin's constructor
with
QObject *parent there.

In our case static_cast is ok because plugin controller
never sends
anything other than KDevelop::ICore pointer as an object.

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On Friday 20 April 2007, Andreas Pakulat wrote:
> Well, if the dynamic_cast fails the plugin won't work
and from the
> plugin destructor I don't think we can abort the plugin
creation (or
> can we?).

I don't know, but having a null pointer (for d->core)
instead of an 
invalid pointer is still better, IMO. If it ever happens
that parent is 
not a KDevelop::ICore, for null pointer it will crash at
first access, 
but with the invalid pointer who knows what will happen. 

Andras

-- 
Quanta Plus developer - http://quanta.kdewebdev.o
rg
K Desktop Environment - http://www.kde.org

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On 4/20/07, Andras Mantia <amantiakde.org> wrote:
> On Friday 20 April 2007, Andreas Pakulat wrote:
> > Well, if the dynamic_cast fails the plugin won't
work and from the
> > plugin destructor I don't think we can abort the
plugin creation (or
> > can we?).
> I don't know, but having a null pointer (for
d->core) instead of an
> invalid pointer is still better, IMO. If it ever
happens that parent is
> not a KDevelop::ICore, for null pointer it will crash
at first access,
> but with the invalid pointer who knows what will
happen.
Can anybody please explain what's wrong with static cast
here?

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On Friday 20 April 2007, Alexander Dymo wrote:
> In our case static_cast is ok because plugin controller
never sends
> anything other than KDevelop::ICore pointer as an
object.

In that case of course it is fine to have the static_cast,
but it was 
unclear (for me, and probably for Jens as well), what kind
of pointer 
can you get there.

Andras

-- 
Quanta Plus developer - http://quanta.kdewebdev.o
rg
K Desktop Environment - http://www.kde.org

_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel

On Apr 21, 2007, at 12:41 AM, Andras Mantia wrote:

> On Friday 20 April 2007, Alexander Dymo wrote:
>> In our case static_cast is ok because plugin
controller never sends
>> anything other than KDevelop::ICore pointer as an
object.
>
> In that case of course it is fine to have the
static_cast, but it was
> unclear (for me, and probably for Jens as well), what
kind of pointer
> can you get there.
>
> Andras
>

We should probably add a comment there so that people can
read the  
code and know it's safe. Any takers?

Matt



_______________________________________________
KDevelop-devel mailing list
KDevelop-develkdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinf
o/kdevelop-devel