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
