Author: kotkov
Date: Tue Oct 23 11:36:24 2018
New Revision: 1844634
URL: http://svn.apache.org/viewvc?rev=1844634&view=rev
Log:
Following up on r1836306, provide an alternative fix for allowing things
like "svn commit -F <(echo foo)" that doesn't require a more expensive
apr_file_info_get(APR_FINFO_SIZE | APR_FINFO_TYPE) syscall in the
low-level and commonly used function.
Instead of additionally blacklisting APR_PIPE handles, rework the code to
work under the conditions where `finfo.size` may be wrong. That is, try to
read one more byte than necessary, expect to see an EOF, but if we don't
(e.g., for pipes), fall through to reading the remaining part of the file
chunk-by-chunk.
This approach should work correctly under all possible cases where `finfo.
size` is incorrect, and also allows to just ask for APR_FINFO_SIZE, which
is generally cheaper. At least on Windows, this uses GetFileSizeEx() and
results in a single syscall.
* subversion/libsvn_subr/io.c
(stringbuf_from_aprfile): Replace the additional blacklist condition for
APR_PIPEs with the approach described above.
Modified:
subversion/trunk/subversion/libsvn_subr/io.c
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1844634&r1=1844633&r2=1844634&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Tue Oct 23 11:36:24 2018
@@ -2547,27 +2547,36 @@ stringbuf_from_aprfile(svn_stringbuf_t *
apr_finfo_t finfo = { 0 };
/* In some cases we get size 0 and no error for non files, so we
- also check for the name. (= cached in apr_file_t) and for FIFOs */
- if (! apr_file_info_get(&finfo, APR_FINFO_SIZE | APR_FINFO_TYPE, file)
- && finfo.fname && finfo.filetype != APR_PIPE)
+ also check for the name. (= cached in apr_file_t) */
+ if (! apr_file_info_get(&finfo, APR_FINFO_SIZE, file) && finfo.fname)
{
- /* we've got the file length. Now, read it in one go. */
+ /* In general, there is no guarantee that the given file size is
+ correct, for instance, because the underlying handle could be
+ pointing to a pipe. We don't know that in advance, so attempt
+ to read *one more* byte than necessary. If we get an EOF, then
+ we're done and we have succesfully avoided reading the file chunk-
+ by-chunk. If we don't, we fall through and do so to read the
+ remaining part of the file. */
svn_boolean_t eof;
- res_initial_len = (apr_size_t)finfo.size;
+ res_initial_len = (apr_size_t)finfo.size + 1;
res = svn_stringbuf_create_ensure(res_initial_len, pool);
SVN_ERR(svn_io_file_read_full2(file, res->data,
res_initial_len, &res->len,
&eof, pool));
res->data[res->len] = 0;
- *result = res;
- return SVN_NO_ERROR;
+ if (eof)
+ {
+ *result = res;
+ return SVN_NO_ERROR;
+ }
}
}
/* XXX: We should check the incoming data for being of type binary. */
buf = apr_palloc(pool, SVN__STREAM_CHUNK_SIZE);
- res = svn_stringbuf_create_ensure(res_initial_len, pool);
+ if (!res)
+ res = svn_stringbuf_create_ensure(res_initial_len, pool);
/* apr_file_read will not return data and eof in the same call. So this loop
* is safe from missing read data. */
