Review Request 111636: Port away from kde_file.h in AuthInfo (KIO)

[David Gil Oliva]

> On July 23, 2013, 11:19 a.m., David Faure wrote:
> > staging/kio/src/core/authinfo.cpp, line 523
> > <http://git.reviewboard.kde.org/r/111636/diff/1/?file=172718#file172718line523>
> >
> >     Making a member variable read from a function-local iodevice sounds dangerous to me. If you use d->fstream from a method not called by this one, you get a crash.
> >     
> >     Maybe it would be better to pass the stream to the 2 methods that need it, rather than having it as a member variable? Or making file a member too.
> >     Well, I guess that's not needed, it just seems fragile to have a member that is only usable when called from within one method.
> >     
> >     ==
> >     
> >     A 2nd issue: what should be the encoding of the QTextStream? i.e. which encoding does the file use?

I (think that I) fix it in the next patch, putting in NetRCPrivate those methods that don't have to be public (getMachinePart(), getMacdefPart() and extract() ), and leaving public only lookup(), reload() and parse(). I even thought about putting parse() in NetRCPrivate, but I left it protected in NetRC.

This way, I think there's no danger. The usual way of calling this class is calling lookup(), which calls parse() (and it calls the rest of the methods).

==

2nd issue: I don't know what the encoding of the QTextStream should be. 


- David


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


On July 21, 2013, 11:41 p.m., David Gil Oliva wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111636/
> -----------------------------------------------------------
> 
> (Updated July 21, 2013, 11:41 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Port away from kde_file.h in AuthInfo (KIO)
> 
> I have tried not to touch much code, but I finally rewrote some parts to make them easier to understand.
> 
> 
> Diffs
> -----
> 
>   staging/kio/src/core/authinfo.h d6415b2f2e9ccec7c3e046f569fb44dbbc879d6b 
>   staging/kio/src/core/authinfo.cpp 65ebacf84e989f19f1b896c596a6b24185c67447 
> 
> Diff: http://git.reviewboard.kde.org/r/111636/diff/
> 
> 
> Testing
> -------
> 
> It builds. I have tested the part of the code not related to loginMap with a little program and .netrc sample files, to check whether it correctly parses the information.
> 
> 
> Thanks,
> 
> David Gil Oliva
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130818/714deffe/attachment.html>