On 29 Jun 2020, at 11:41, Ruediger Pluem <rpl...@apache.org> wrote:
>> + etag = apr_palloc(r->pool, weak_len + sizeof("\"\"") +
>> + 4*(APR_SHA1_DIGESTSIZE/3) + vlv_len + 4);
>
> Using apr_base64_encode_len in the formula above would make it easier to
> understand and read IMHO.
It would also mean it could no longer be optimised to a constant. What side do
we want to fall on?
>> +
>> + apr_sha1_init(&context);
>> + nbytes = sizeof(buf);
>> + while (apr_file_read(fd, buf, &nbytes) == APR_SUCCESS) {
>
> I assume that the code has been taken from ap_md5digest, but
> if we have MMAP available and if we have not disabled MMAP we use MMAP to
> read the contents of the file
> if sendfile is not possible (e.g. SSL connections).
> Wouldn't using MMAP more performant here especially in case of larger files?
It makes sense, but I would want to change something like this separately.
>> + apr_sha1_update_binary(&context, buf, nbytes);
>> + nbytes = sizeof(buf);
>> + }
>> + apr_file_seek(fd, APR_SET, &offset);
>
> Why do we always reset the file pointer to 0? Why don't we get the actual
> file pointer of fd before we do the reading
> and restore it afterwards?
I am guessing that the why is lost in the mists of time. As a guess, it avoids
an additional call.
Using the actual file pointer is cleaner, as there is a guarantee that the
function does not have any side effects.
http://svn.apache.org/viewvc?rev=1879332&view=rev
<http://svn.apache.org/viewvc?rev=1879332&view=rev>
Regards,
Graham
—
