Review Request 115499: Season of KDE Project - KRFB

[Amandeep Singh]

> On Feb. 17, 2014, 9:32 p.m., George Kiagiadakis wrote:
> > krfb/invitationsrfbclient.cpp, lines 111-145
> > <https://git.reviewboard.kde.org/r/115499/diff/1/?file=242050#file242050line111>
> >
> >     I don't like very much that this code gets duplicated from RfbClient here. Why do you need this here?

This was done to change the order of events during prompting the user. Previously, any un-authenticated client would first prompt the user for accepting/rejecting. By this change, the client remains a pending one, until it passes authentication, and can prompt user & get registered with RfbServerManager only after authentication is complete.

As soon as the client reaches state RFB_INITIALIZATION, processNewClient() is called to prompt the host-user to accept/reject (if askOnConnect is true). 


> On Feb. 17, 2014, 9:32 p.m., George Kiagiadakis wrote:
> > libvncserver/sockets.c, line 111
> > <https://git.reviewboard.kde.org/r/115499/diff/1/?file=242080#file242080line111>
> >
> >     Is this really needed? What does it do?  For the same reason (libvncserver code), we should avoid it.

We would need the change if we wish to support repeated start/stop of the InvitationsRfbServer by the user, via the UI.

It makes sense for the function to return without doing anything, only when the sockets are already initialized (RFB_SOCKET_READY).
As the very next line shows, socketState is set to RFB_SOCKET_READY when rfbInitSockets is called first time properly & sockets are initialized.

Previously it's just checking if socket state is not RFB_SOCKET_INIT. This is also case when sockets are in shut down state, which causes problem, when user tries to start server again.


On Feb. 17, 2014, 9:32 p.m., Amandeep Singh wrote:
> > Some more issues that I have noticed:
> > 
> > * The password edit button inititially says "Edit" with letters, but if you click on it it becomes a floppy icon and when you save the password it becomes the "document-edit" icon. For consistency, it should also initially have the "document-edit" icon instead of saying "Edit".
> > 
> > * I cannot enable the rfb server at all. It always returns an error, not sure why.
> > 
> > * krfb crashes on exit inside ~InvitationsRfbServer. In the backtrace it looks like the destructor is being recursively called from inside the destructor. I haven't understood why yet.
> >

The Icon is set in the UI file to get set on start up. Not sure why you're experiencing the issue with the Icon. Works perfect at two machines I tested.

Also about the crash in InvitationsRfbServer's destructor, is it reproducable? Can you please tell the steps that lead to crash.
I have also fixed another crash regarding closing Krfb during pending connections, please see if you still get the crash. 


- Amandeep


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115499/#review50035
-----------------------------------------------------------


On Feb. 18, 2014, 2:43 p.m., Amandeep Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115499/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2014, 2:43 p.m.)
> 
> 
> Review request for Telepathy and George Kiagiadakis.
> 
> 
> Repository: krfb
> 
> 
> Description
> -------
> 
> Season of KDE Project - KRFB
> 
> Status Quo
> -----------
> Currently KRFB works on an invite-only model, where the host user needs to create an invite via the Invitation Dialog.
> This provides the connection details (address and password), which must then be communicated to the remote user.
> No information about telepathy contacts with whom desktop can be shared is present in the KRFB Dialog.
> The implementation of RFB also lacks support of clipboard operations like cut and paste of text.
> 
> Changes
> -------
> Krfb modified to move away from invite-model, and modeled as a service that can be started/stopped.
> Bidirectional text-clipboard operations are supported.
> Telepathy contacts with rfb capability shown inside main window, with an option to share desktop.
> Desktop sharing password visible in main window, with option to change it.
> Unattended Access allowed with separate password.
> Fixed bugs in libvncserver responsible for KRFB crashing.
> Krfb now runs as KUniqueApplication.
> 
> All changes are present in the following clone of KRFB
> 
> Repository
> ----------
> http://quickgit.kde.org/?p=clones%2Fkrfb%2Famandeepsingh%2Fkrfb-sok.git
> 
> 
> Diffs
> -----
> 
>   krfb/invitationsrfbclient.cpp bd5cff8 
>   krfb/invitationsrfbserver.cpp 47ed934 
>   krfb/mainwindow.cpp PRE-CREATION 
>   krfb/rfbclient.h 7b46d3f 
>   krfb/rfbclient.cpp 9ab16eb 
>   krfb/rfbserver.cpp cd5d6c0 
>   krfb/tubesrfbserver.cpp 575c3cd 
>   krfb/ui/mainwidget.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/115499/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Amandeep Singh
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140218/c1135237/attachment-0001.html>