Review Request: Add an OpenConnect VPN plug-in

[Lamarque Vieira Souza]

> On June 27, 2011, 12:37 p.m., Lamarque Vieira Souza wrote:
> > vpnplugins/openconnect/openconnectwidget.cpp, line 50
> > <http://git.reviewboard.kde.org/r/101788/diff/1/?file=25396#file25396line50>
> >
> >     Do not do this. This class inherit from SettingWidget, which has a virtual destructor, which already deletes d_ptr. You are doing a double delete here, which will corrupt system memory. Just remove this line.
> 
> Ilia Kats wrote:
>     As far as I know, this would only apply if OpenconnectAuthWidgetPrivate would inherit from SettingWidgetPrivate, which it does not. All the other VPN plugins are also deleting their d-pointers.
> 
> Ilia Kats wrote:
>     OpenconnectSettingWidgetPrivate, sorry. Applies for the auth widget too, though.

So they are all double deleting. SettingWidget's destructor is virtual, if it was not then there would be no problem. Besides, SettingWidgetPrivate does not even have a destructor.


> On June 27, 2011, 12:37 p.m., Lamarque Vieira Souza wrote:
> > plasma_nm_version.h, line 3
> > <http://git.reviewboard.kde.org/r/101788/diff/1/?file=25381#file25381line3>
> >
> >     Have you edited this manually? This file is supposed to be edited automatically but I have not sent the edit script to anyone.
> 
> Ilia Kats wrote:
>     I am able to write my own scripts, you know. And no, I won't push it like that, but editing that part out of every version of every patch is just too much effort.

Yeah, but this is nm09 branch, not openconnect. Or are you using a temporary branch? If so then change your script to only touch this file when running from master or nm09 branches.


- Lamarque Vieira


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101788/#review4198
-----------------------------------------------------------


On June 27, 2011, 6:16 p.m., Ilia Kats wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101788/
> -----------------------------------------------------------
> 
> (Updated June 27, 2011, 6:16 p.m.)
> 
> 
> Review request for Network Management.
> 
> 
> Summary
> -------
> 
> Add an OpenConnect VPN plug-in to KDE NM. First time using threads, so I don't know if that's how it's supposed to be done, but it seems to be working.
> Also, I can't figure out  how to make the loginForm QGroupBox have a minimum height, but get bigger when widgets get added. If the minimum height is 0, then both the box and the dialog get resized, however this causes visual "interference" as the serverLogBox jumps up and down as the upper loginForm box gets resized. Setting the minimum height of the loginForm box to 100 causes it to stay at 100 and squeeze the added widgets to fit the size, instead of getting bigger. Any ideas?
> 
> This requires OpenConnect >= 3.03 to build. 3.03 is not yet released, but the important thing is commit 423eee0b51a204562d6f2ec67893133ebcf200d6 from OpenConnect git (http://git.infradead.org/users/dwmw2/openconnect.git/ )
> 
> 
> This addresses bug 226028.
>     http://bugs.kde.org/show_bug.cgi?id=226028
> 
> 
> Diffs
> -----
> 
>   libs/internals/settings/vpnsecrets.cpp 0d9e3f9 
>   libs/ui/connectionsecretsjob.cpp d791bb3 
>   libs/ui/vpnuiplugin.h 444ab2a 
>   libs/ui/vpnuiplugin.cpp d058a52 
>   plasma_nm_version.h b580c80 
>   vpnplugins/CMakeLists.txt 4706a61 
>   vpnplugins/openconnect/CMakeLists.txt PRE-CREATION 
>   vpnplugins/openconnect/FindOpenConnect.cmake PRE-CREATION 
>   vpnplugins/openconnect/networkmanagement_openconnectui.desktop PRE-CREATION 
>   vpnplugins/openconnect/nm-openconnect-service.h PRE-CREATION 
>   vpnplugins/openconnect/openconnectauth.h PRE-CREATION 
>   vpnplugins/openconnect/openconnectauth.cpp PRE-CREATION 
>   vpnplugins/openconnect/openconnectauth.ui PRE-CREATION 
>   vpnplugins/openconnect/openconnectauthworkerthread.h PRE-CREATION 
>   vpnplugins/openconnect/openconnectauthworkerthread.cpp PRE-CREATION 
>   vpnplugins/openconnect/openconnectprop.ui PRE-CREATION 
>   vpnplugins/openconnect/openconnectui.h PRE-CREATION 
>   vpnplugins/openconnect/openconnectui.cpp PRE-CREATION 
>   vpnplugins/openconnect/openconnectwidget.h PRE-CREATION 
>   vpnplugins/openconnect/openconnectwidget.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101788/diff
> 
> 
> Testing
> -------
> 
> Yes, see the bugzilla ticket.
> 
> 
> Screenshots
> -----------
> 
> 
>   http://git.reviewboard.kde.org/r/101788/s/191/
> 
> 
> Thanks,
> 
> Ilia
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110627/a155763a/attachment-0001.htm