kdereview: ksmtp

[Daniel Vrátil]

Hi Rolf,

thanks for the review, sorry it took me so long to get to you.

On Tuesday, May 16, 2017 8:05:17 PM CEST Rolf Eike Beer wrote:
> Am Donnerstag, 11. Mai 2017, 17:03:01 schrieb Daniel Vrátil:
> > Hi,
> > 
> > please review ksmtp, which is now in kdereview.
> 
> -the CMakeLists.txt has a mix of spaces inside () or not

Fixed

> 
> -in loginjob, line 173, you check for code 25. Should this be 250? Or is
> that 25*? Where is ServerResponse actually defined, I only see the header.

ServerResponse is defined in sessionthread.cpp for some reason. 
ServerResponse::isCode() indeed checks the prefix of the response, so 
isCode(25) returns true for any 25x return code.

> -does that support pipelining? I don't see any sync points, so I guess not.

Not yet, but it's next on the todo list.

> -there is a longstanding bug in KMail that it violates the RfC when it has a
> problem with authentication (e.g. password rejected), that is does not
> properly QUIT the SMTP session, but just closes the socket. Is that
> properly handled?

It is properly handled now. Calling Session::quit() does not close the 
connection but sends QUIT command and only closes the connection after a 
response arrives from the server (unless the server closes it first of course). 
It's now up to the user to ensure that the Session object is not destroyed 
until the state changes to Disconnected after calling Session::quit().

Dan

> 
> Greetings,
> 
> Eike


-- 
Daniel Vrátil
www.dvratil.cz | dvratil at kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20170531/dc384207/attachment.sig>