PATCH: kio_http [IMPORTANT]

[Waldo Bastian]

> Hi,
> 
> The following is a patch for kio_http and it addresses the following issues.  
> All of which I myself have encountered...
> 
> 1st section:
>
> Removes a meaningless do loop which was appropriate (I added it :)) when we 
> were directly accessing the system ::read call.  However, since we now call 
> TCPSlaveBase::read either needs to be moved there or completely 
> discarded, no ?

It is still needed. Moving it to TCPSlaveBase::read might be ok though. There 
is no urgent need for that though, so it should happen after 3.1

> 2nd section:
> 
> Fixes a rather nasty bug where we force a resume download even when the 
> server does not support it.  To compound the problem we correctly signal the
> calling job that the server does not support resumption.

That is this part of the fix?

   // Inform the job that we can indeed resume...
-  if ( bCanResume && m_request.offset )
+  if ( bCanResume && m_request.offset > 0 )
     canResume();
+  else
+    m_request.offset = 0;

The change in the "if ( bCanResume && m_request.offset )" is only cosmetical, 
right? The else part looks ok.

> 3rd section:
> 
> Fixes two problems:
> 
> First is the issue of using the negation (not) operator with signed long/int 
> variables in conditional statements. It is my understanding (or rather was 
> mentioned to me) that doing can also cause undefined conditions and in
> general a bad idea besides making it to understand the intention of the 
> coder.

I don't think this is very important.

> The following part:
> 
> -  if (bytesReceived <= 0)
> -     return -1; // Error: connection lost
> +  if (bytesReceived < 0)
> +     return -1; // ERROR: Connection lost...
> 
> fixes a long running problem I have had with downloading the config file off 
> of my router's web interface.  I always get a "Connection broken" error 
> message even though the entire file was correctly downloaded.  Why on earth 
> do we consider 0 byte returns as being error conditions ????

When the webserver says it is going to send 100 bytes and then disconnects 
after 90 it is an error. Note that this is readLimited() and that is used 
when the size of data is known. Can you send me a copy of the http log for 
such a download?

I have committed the 
+  else
+    m_request.offset = 0;
part of your patch.

Cheers,
Waldo