Re: Review Request 127501: Improve TCPSlaveBase::isConnected
> 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.
>
> David Faure wrote:
> 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.
>
> Andreas Hartmetz wrote:
> There does seem to be a way to check whether the socket error is for the
> current connection, sort of... check if the errorString() is empty. That one
> is cleared on close() and AFAICS also set each time a socket error occurs. It
> is also never set on its own - every error is a socket error in
> QAbstractSocket.
> So QAbstractSocket does support lazy error checks, it's just a bit
> strange.
>
> Albert Astals Cid wrote:
> Andreas, that doesn't work, see the code of QIODevice::errorString
>
> if (d->errorString.isEmpty()) {
> #ifdef QT_NO_QOBJECT
> return QLatin1String(QT_TRANSLATE_NOOP(QIODevice, "Unknown
> error"));
> #else
> return tr("Unknown error");
> #endif
> }
>
> Albert Astals Cid wrote:
> > 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.
>
> You can not do that, there's no API call that will return false in this
> case, socket.write returns a valid number of bytes, waitForBytesWritten
> returns true, but if you ask for the error after that, it has been set to
> RemoteHostClosedError, you may argue that is a bug in Qt and that calling
> waitForBytesWritten should properly return false if the calls to
> QAbstractSocketPrivate::canReadNotification,
> QAbstractSocketPrivate::readFromSocket and QNativeSocketEngine::read end up
> setting an error.
>
> > 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.
>
> Yes, we could do that and it would fix isConnected for the first time,
> the problem is that if after you try to reuse the socket, error() will still
> be set and as there's no way to unset it (or check we have to read it, unless
> as said on the paragraph above, we consider the Qt behaviour a bug and fix it)
>
> David Faure wrote:
> Isn't a signal emitted then, when the
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127501/ --- (Updated March 31, 2016, 8:01 p.m.) Status -- This change has been discarded. 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
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
> 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.
>
> David Faure wrote:
> 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.
>
> Andreas Hartmetz wrote:
> There does seem to be a way to check whether the socket error is for the
> current connection, sort of... check if the errorString() is empty. That one
> is cleared on close() and AFAICS also set each time a socket error occurs. It
> is also never set on its own - every error is a socket error in
> QAbstractSocket.
> So QAbstractSocket does support lazy error checks, it's just a bit
> strange.
>
> Albert Astals Cid wrote:
> Andreas, that doesn't work, see the code of QIODevice::errorString
>
> if (d->errorString.isEmpty()) {
> #ifdef QT_NO_QOBJECT
> return QLatin1String(QT_TRANSLATE_NOOP(QIODevice, "Unknown
> error"));
> #else
> return tr("Unknown error");
> #endif
> }
>
> Albert Astals Cid wrote:
> > 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.
>
> You can not do that, there's no API call that will return false in this
> case, socket.write returns a valid number of bytes, waitForBytesWritten
> returns true, but if you ask for the error after that, it has been set to
> RemoteHostClosedError, you may argue that is a bug in Qt and that calling
> waitForBytesWritten should properly return false if the calls to
> QAbstractSocketPrivate::canReadNotification,
> QAbstractSocketPrivate::readFromSocket and QNativeSocketEngine::read end up
> setting an error.
>
> > 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.
>
> Yes, we could do that and it would fix isConnected for the first time,
> the problem is that if after you try to reuse the socket, error() will still
> be set and as there's no way to unset it (or check we have to read it, unless
> as said on the paragraph above, we consider the Qt behaviour a bug and fix it)
Isn't a signal emitted then, when the socket detects that the remote
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
> 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.
>
> David Faure wrote:
> 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.
>
> Andreas Hartmetz wrote:
> There does seem to be a way to check whether the socket error is for the
> current connection, sort of... check if the errorString() is empty. That one
> is cleared on close() and AFAICS also set each time a socket error occurs. It
> is also never set on its own - every error is a socket error in
> QAbstractSocket.
> So QAbstractSocket does support lazy error checks, it's just a bit
> strange.
>
> Albert Astals Cid wrote:
> Andreas, that doesn't work, see the code of QIODevice::errorString
>
> if (d->errorString.isEmpty()) {
> #ifdef QT_NO_QOBJECT
> return QLatin1String(QT_TRANSLATE_NOOP(QIODevice, "Unknown
> error"));
> #else
> return tr("Unknown error");
> #endif
> }
> 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.
You can not do that, there's no API call that will return false in this case,
socket.write returns a valid number of bytes, waitForBytesWritten returns true,
but if you ask for the error after that, it has been set to
RemoteHostClosedError, you may argue that is a bug in Qt and that calling
waitForBytesWritten should properly return false if the calls to
QAbstractSocketPrivate::canReadNotification,
QAbstractSocketPrivate::readFromSocket and QNativeSocketEngine::read end up
setting an error.
> 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.
Yes, we could do that and it would fix isConnected for the first time, the
problem is that if after you try to reuse the socket, error() will still be set
and as there's no way to unset it (or check we have to read it, unless as said
on the paragraph above, we consider the Qt behaviour a bug and fix it)
- Albert
---
This is an automatically generated e-mail. To reply, visit:
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
> 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.
>
> David Faure wrote:
> 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.
>
> Andreas Hartmetz wrote:
> There does seem to be a way to check whether the socket error is for the
> current connection, sort of... check if the errorString() is empty. That one
> is cleared on close() and AFAICS also set each time a socket error occurs. It
> is also never set on its own - every error is a socket error in
> QAbstractSocket.
> So QAbstractSocket does support lazy error checks, it's just a bit
> strange.
Andreas, that doesn't work, see the code of QIODevice::errorString
if (d->errorString.isEmpty()) {
#ifdef QT_NO_QOBJECT
return QLatin1String(QT_TRANSLATE_NOOP(QIODevice, "Unknown error"));
#else
return tr("Unknown error");
#endif
}
- Albert
---
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
>
>
___
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
> 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
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
> 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? 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. - Andreas --- 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
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
> 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 :-) 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? - Albert --- 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
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127501/#review94042 --- 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 :-) - David Faure 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
Re: Review Request 127501: Improve TCPSlaveBase::isConnected
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127501/#review94033
---
Ow. That's a pretty bad problem.
I suggest a less intrusive (code churn and regression potential) solution:
Since QAbstractSocket *does* have setError() as a protected method, you can
still call it:
// never instantiated
class QAbstractSocketHack: public QAbstractSocket {
public:
void setError(SocketError e) Q_DECL_OVERRIDE {
QAbstractSocket::setError(e); }
};
// in socket reset code
static_cast(>socket)->setError(QAbstractSocket::UnknownSocketError);
- Andreas Hartmetz
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
