[networkmanagement/nm09] libs/service: fix kwallet

Lamarque Vieira Souza lamarque at gmail.com
Mon May 23 23:53:41 CEST 2011


	Hi Ilia, please add the next secrets agent's patches to 
git.reviewboard.kde.org. There have been some turbulances (like crashes,  
memory corruption, reverted commits) that could have been avoided using 
reviewboard. The secrets agent implementation seems to be more complex than we 
have thought.

	About my commit you reverted:

1. I did not make  the "delayed DBus reply" work synchronized, I just followed 
Qt's documentation about how delayed reply should be implemented: 

http://doc.trolltech.com/latest/qdbusdeclaringslots.html#delayed-replies

	The way you did you are not warning the requester that the reply is going 
to be delayed. Quoting the documentation: 

"The use of QDBusConnection::sessionBus().send(data->reply) is needed to 
explicitly inform the caller that the response will be delayed. In this case, 
the return value is unimportant; we return an arbitrary value to satisfy the 
compiler."

2. Since the return value is unimportant you do not need to create a local 
variable (QVariantMapMap map) to return it, you can just return the connection 
parameter, which is the same type.

3. In the foreach loop you test if m_secretsProvider is null for each element, 
you can test it only once since m_secretsProvider is not touched inside the 
loop.

4. I do not like the NMDBusSecretAgent::deleteSavedConnection approach to 
delete a parameter emited from another object. That can lead to dandling 
pointers and we already have plenty of problems with dangling pointers. This 
is another reason to asking for comments in reviewboard about how to proceed. 
As a note I do not know exactly how we should proceed (yet) either.

	Thanks for all the work you are doing to implement the secrets agent, it 
is crucial for us it works.

-- 
Lamarque V. Souza
http://www.geographicguide.com/brazil.htm
Linux User #57137 - http://counter.li.org/
http://planetkde.org/pt-br
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110523/ff645ee2/attachment.htm 


More information about the kde-networkmanager mailing list