KWallet and DeviceStore sync patch

Timo Hoenig thoenig at suse.de
Mon Jul 3 10:45:08 CEST 2006


Hi Valentine,

On Thu, 2006-06-29 at 13:19 +0600, Valentine Sinitsyn wrote:

> Attached to this letter is much improved version of my
> KWallet-integration-and-DeviceStore-KStorage-synchronization patch
> against kdereview/knetworkmanager I've sent to Timo some weeks ago.
> Although many ideas are already implemented in knm-make-it-cool
> branch, I'd like to highlight several points:
> 
> * Bugfixes to my code committed to knm-make-it-cool branch:
>    - Network::isModified() should return "true" if Network object is
>    modified itself or it is associated with modified Encryption
>    object. That's because we never persist Encryption object alone,
>    only on par with network and the code like "if (net->isModified())
>    {net->persist()}" must handle the situation when, say, only password
>    was changed.
>    - Encryption::setSecrets() should mark Encryption object as _dirty
>    only if it is not of type EncryptionWPAEnterprise, since we do not
>    store secrets ("identityPassword" and "certPrivatePassword") for
>    this type of Encryption. BTW: is this (i.e. not storing identity
>    and cert passwords) the right thing to do?

This should be a bug.  I'll investigate.

>    - Tray::slotNMConnected() is not the right place to persist Network
>    on successful connection since NetworkManager emit updateNetwork
>    signal for this purpose. This signal is handled (ultimately) in
>    KStorage::updateNetwork().
>    - There are some checks like _secret.isEmpty() in Encryption
>    classes. They are not correct (or at least they does not work for
>    me). Instead, one should check for _secret["password"].isEmpty() -
>    it also makes code cleaner.

Correct.

>    - Network::Network() constructors: it is wrong to initialize
>    _hw_addresses (QStringList) with "" (i.e. adding empty string
>    to the beginning of the list) - this leads to
>    "empty" branches in "Show Networks" dialog for instance. What we
>    need is empty _hw_addresses list which is filled later.

Correct.

> * DeviceStore/KStorage synchronization is stated to be implemented in
> knm-make-it-cool network branch but I kindly ask you to look at my
> implementation of it. The changes required are fairy minimal (there is
> no need for Network pseudo-copy constructors or initFoundNetwork()
> methods) and it fits perfectly to aforementioned updateNetwork signal.

I will leave this up to Will.

> Hope this patch (or some parts of it) will be useful for knm-make-it-cool or
> kdereview/knetworkmanager branches.

You can expect most to be applied to the kdereview/knetworkmanager soon.

Thanks a lot for your efforts.

> Regards,
> Valentine

   Timo



More information about the kde-networkmanager mailing list