Review Request 110328: Add config option to silently create initial password-less wallet

Lamarque Souza lamarque at kde.org
Mon May 6 18:11:42 BST 2013



> On May 6, 2013, 4:40 p.m., Lamarque Souza wrote:
> > kwalletd/kwalletd.h, line 226
> > <http://git.reviewboard.kde.org/r/110328/diff/1/?file=142372#file142372line226>
> >
> >     Why split this feature into three different review requests? One of your requests [2] is just one line change and the other [1] is not visible from reviewboard. I think you should merge those two requests below with this one.
> >     
> >     [1] https://git.reviewboard.kde.org/r/110330
> >     [2] https://git.reviewboard.kde.org/r/110331
> 
> Eike Hein wrote:
>     110328 and 110330 are for independent features that need to be reviewed independently, and 110331 is for a different repository (kwallet.git vs kde-runtime.git).

Ok then, I mistakenly read the group field as repository in 110331. reviewboard still cannot show the diff in 110330.


> On May 6, 2013, 4:40 p.m., Lamarque Souza wrote:
> > kwalletd/kwalletd.cpp, line 506
> > <http://git.reviewboard.kde.org/r/110328/diff/1/?file=142373#file142373line506>
> >
> >     password is a QString, right? Then you should use password.clear() here instead of assigning an empty QString. That avoids creating a temporary QString.
> 
> Eike Hein wrote:
>     If you look at the code later, it cares whether password is null or empty via an isNull() check. An empty QString is different from a null one.

I know they are different. I thought QString::clear() would not call any constructor at all, which is not true. I wrote that based on http://techbase.kde.org/Development/Tutorials/Code_Checking#Module.3Dnullstrassign. It seems we should update techbase then.


- Lamarque


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


On May 6, 2013, 4:48 p.m., Eike Hein wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110328/
> -----------------------------------------------------------
> 
> (Updated May 6, 2013, 4:48 p.m.)
> 
> 
> Review request for KDE Runtime and Harald Sitter.
> 
> 
> Description
> -------
> 
> This patch adds a UI-less config option to kwalletd that makes it create the initial local wallet silently with an empty password instead of prompting the user to enter one.
> 
> It's a change desired by downstream consumers Kubuntu and Netrunner, and perhaps others, and recreates a modification they used to carry for KDE 3. Their goal is to make KWallet mostly invisible to the user during routine operations, but still have users benefit from encrypted password storage behind the scenes.
> 
> As such the config option is intended to be set by distributions. The new behavior is disabled by default.
> 
> In the interest of keeping the delta between upstream and downstream as small as possible I'd say it makes sense to pick this up.
> 
> 
> Diffs
> -----
> 
>   kwalletd/kwalletd.h e8e74c3 
>   kwalletd/kwalletd.cpp fa9fc11 
> 
> Diff: http://git.reviewboard.kde.org/r/110328/diff/
> 
> 
> Testing
> -------
> 
> Test package for Kubuntu by Harald Sitter, operation verified at runtime.
> 
> 
> Thanks,
> 
> Eike Hein
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130506/7e6c6a16/attachment.htm>


More information about the kde-core-devel mailing list