Discussion:
[networkmanagement/nm09] libs/service: fix kwallet
Lamarque Vieira Souza
2011-05-23 21:53:41 UTC
Permalink
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
Loading...