Re: [PATCH] Make fread/fwrite-like functions in http.c more like fread/fwrite.

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Wed, May 01, 2019 at 05:56:35PM +0900, Mike Hommey wrote:
> The fread/fwrite-like functions in http.c, namely fread_buffer,
> fwrite_buffer, fwrite_null, fwrite_sha1_file all return the
> multiplication of the size and number of items they are being given.
> 
> Practically speaking, it doesn't matter, because in all contexts where
> those functions are used, size is 1.
> 
> But those functions being similar to fread and fwrite (the curl API is
> designed around being able to use fread and fwrite directly), it might
> be preferable to make them behave like fread and fwrite, which, from
> the fread/fwrite manual page, is:
>    On  success, fread() and fwrite() return the number of items read
>    or written.  This number equals the number of bytes transferred
>    only when size is 1.  If an error occurs, or the end of the file
>    is reached, the return value is a short item count (or zero).

This patch breaks the test 'push to remote repository with packed
refs' in 't5540-http-push-webdav.sh':

  https://travis-ci.org/git/git/jobs/529223857#L2603

That test makes Apache spin like crazy at 100% CPU usage for about
30secs, after which, according to 'error.log':

  [Tue May 07 14:50:55.555166 2019] [mpm_prefork:notice] [pid 12638]
AH00169: caught SIGTERM, shutting down


> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>  http.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/http.c b/http.c
> index 98fb06df0b..8dbc91f607 100644
> --- a/http.c
> +++ b/http.c
> @@ -176,7 +176,7 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
>  	memcpy(ptr, buffer->buf.buf + buffer->posn, size);
>  	buffer->posn += size;
>  
> -	return size;
> +	return nmemb;
>  }
>  
>  #ifndef NO_CURL_IOCTL
> @@ -204,12 +204,12 @@ size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
>  	struct strbuf *buffer = buffer_;
>  
>  	strbuf_add(buffer, ptr, size);
> -	return size;
> +	return nmemb;
>  }
>  
>  size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf)
>  {
> -	return eltsize * nmemb;
> +	return nmemb;
>  }
>  
>  static void closedown_active_slot(struct active_request_slot *slot)
> @@ -2319,14 +2319,14 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,
>  			BUG("curl_easy_getinfo for HTTP code failed: %s",
>  				curl_easy_strerror(c));
>  		if (slot->http_code >= 300)
> -			return size;
> +			return nmemb;
>  	}
>  
>  	do {
>  		ssize_t retval = xwrite(freq->localfile,
>  					(char *) ptr + posn, size - posn);
>  		if (retval < 0)
> -			return posn;
> +			return posn / eltsize;
>  		posn += retval;
>  	} while (posn < size);
>  
> @@ -2339,7 +2339,7 @@ static size_t fwrite_sha1_file(char *ptr, size_t eltsize, size_t nmemb,
>  		the_hash_algo->update_fn(&freq->c, expn,
>  					 sizeof(expn) - freq->stream.avail_out);
>  	} while (freq->stream.avail_in && freq->zret == Z_OK);
> -	return size;
> +	return nmemb;
>  }
>  
>  struct http_object_request *new_http_object_request(const char *base_url,
> -- 
> 2.21.0
>