[Ocs] Attica patches
Frederik Gladhorn
frederik at gladhorn.de
Fri Apr 8 15:12:03 PDT 2011
Hi Lazlo,
looks good to me :)
It would be great if you took the time to also extend the KDE plugin so that
there will be no bad surprises later when someone tries to use it.
Also this is quite a fundamental extension, so I would still like to bump the
version number and it would be nice to document that this is only available
starting from version x.y.z.
Thanks,
Frederik
On Friday 8. April 2011 22.24.40 Laszlo Papp wrote:
> On Fri, Apr 8, 2011 at 7:15 PM, Laszlo Papp <djszapi at archlinux.us> wrote:
> >> I can try to make this patch as soon as possible, any objection to ?
> >
> > http://djszapi.dyndns-home.com/platformdependent_v2.patch
> >
> > Please comment on it. Thank you in advance!
>
> Updated version: Reviewed by: David Faure (heretofore)
> http://djszapi.dyndns-home.com/0001-Implement-a-separate-PlatformDependentV
> 2-interface-p.patch
>
> The only small change I did compared to the previous version of this
> patch is that I implemented a separate platformdependent_v2.cpp
> because of the non-inline virtual destructor in that class. I have
> been told by David Faure as it is a good practice to do like that.
>
> The reason is the following behind that claim:
> Non-inline dtor for v2 would make things less fragile, here's why:
> The vtable (and RTTI for dynamic_cast) is generated in the .o that
> contains the implementation of the first virtual method of the class.
> If all methods are pure virtual --or inlined -- then it might not
> happen inside the base library which means then that it could happen
> twice in two derived classes in two libs based on the base lib -- and
> then dynamic_cast will fail and adding a non-inline dtor doesn't cost
> you that much :-)
>
> In more words, from the web: "Some compilers (read: gcc) look for the
> first-encountered non-inline virtual method body and use it to decide
> where to put the virtual method table. If you don't have any virtual
> methods with bodies in a .cpp file, the virtual method table is not
> created." So we really want one single vtable for the class and not
> one per user of the class.
>
> Any objection to this patch or can I push it ?
>
> Best Regards,
> Laszlo Papp
More information about the Ocs
mailing list