> On March 27, 2016, 10:03 a.m., David Faure wrote: > > Given how the socket API works, you should only call error() after a call > > that returns false (e.g. waitForConnected, etc.). As you found out, calling > > error() at random points in time doesn't give useful information, you get > > the last error that was ever set, even after 10 successful calls following > > the call that had an error. > > > > Calling setError ourselves is a workaround, which doesn't fit with the Qt > > socket API (there is no "No error" error code). > > UnknownSocketError *means* error, it doesn't mean no error. > > > > It seems to me (from the description, I didn't read the code attentively) > > that the right solution is to actually disconnect after a > > RemoteHostClosedError. Then the state won't be ConnectedState anymore, and > > isConnected() will work as intended :-) > > Albert Astals Cid wrote: > But it won't, even if you disconnect, the error will still be > RemoteHostClosedError given that Qt does not reset the error after > disconnectFromHost (since there's basically no way to mark "no error"), so if > we reconnect after disconnection we will still have RemoteHostClosedError if > we ask for the error even if it's not true for this session. Of you mean this > in connection with the above hack? > > Andreas Hartmetz wrote: > Yeah, what happens here is working around two API mistakes in QTcpSocket: > that there is no proper "no error" code, and that the error isn't cleared > when starting a new connection. Apart from settings that may reasonably be > persistent like e.g. proxy or buffer size settings, a new connection should > behave exactly the same as deleting and recreating the socket. Otherwise the > API effectively requires you to delete and recreate a socket to get a clean > state, which isn't very nice in such a widely used API. (I sometimes do this > in internal interfaces, it's a nice technique to avoid errors in reset and > clean up code paths - but it causes a little overhead and more work for > consumers) > Since the check here is for a specific error code, UnknownError works as > a code that isn't that specific one. And if we hack the error code reset, we > have what we need. I consider that self defense against bad API, not a real > hack.
My suggestion is about using the API the way it was intended to be used: never calling error() unless a QAbstractSocket method returns false. So definitely not calling error() in isConnected(), but only after specific API calls that return false. Albert: if we disconnect, then state() == ConnectedState will be false, and therefore isConnected will return false, as intended. No need to call error() in that method. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127501/#review94042 ----------------------------------------------------------- On March 26, 2016, 5:29 p.m., Albert Astals Cid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127501/ > ----------------------------------------------------------- > > (Updated March 26, 2016, 5:29 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > ------- > > Qt sockets returns ConnectedState even when error is set to > RemoteHostClosedError after a write operation. > > So make ::isConnected also check for RemoteHostClosedError. > > This has the consequence that we have to create/delete the socket since Qt > sockets have no way to reset their error state so if we want to reuse the > slave to connect again it will never work with the same socket since it will > always say RemoteHostClosedError. > > I need this for https://git.reviewboard.kde.org/r/127502/ > > > Diffs > ----- > > src/core/tcpslavebase.cpp b9be69d > > Diff: https://git.reviewboard.kde.org/r/127501/diff/ > > > Testing > ------- > > Seesms to work fine and the pop3 bugfix i have also works fine. > > > Thanks, > > Albert Astals Cid > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
