On Tue, May 07, 2019 at 04:58:32PM +0200, SZEDER Gábor wrote:
> 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
Yeah, this reproduces easily. The problem is that fread_buffer()
modifies "size" (if there are not enough bytes in the buffer to read),
so we cannot just assume it is "eltsize * nmemb" anymore.
I.e., we need to squash in:
diff --git a/http.c b/http.c
index 8dbc91f607..27aa0a3192 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 nmemb;
+ return size / eltsize;
}
#ifndef NO_CURL_IOCTL
The other conversions all look correct (there's a similar case in
fwrite_sha1_file, but it already does the division correctly).
-Peff
