You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Fuhrmann <eq...@web.de> on 2012/04/27 05:39:05 UTC
Re: svn commit: r1325899 - in /subversion/trunk/subversion/libsvn_ra_svn:
marshal.c ra_svn.h
Am 26.04.2012 11:05, schrieb Daniel Shahaf:
> stefan2@apache.org wrote on Fri, Apr 13, 2012 at 18:36:46 -0000:
>> Author: stefan2
>> Date: Fri Apr 13 18:36:46 2012
>> New Revision: 1325899
>>
>> URL: http://svn.apache.org/viewvc?rev=1325899&view=rev
>> Log:
>> Various performance improvements to the ra_svn marshaller.
>> The total effect is roughly a duplication in throughput.
>>
>> /* --- WRITING DATA ITEMS --- */
>>
>> +static svn_error_t *write_number(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
>> + apr_uint64_t number, char follow)
>> +{
>> + apr_size_t written;
>> +
>> + if (conn->write_pos + SVN_INT64_BUFFER_SIZE>= sizeof(conn->write_buf))
> Off-by-one? You don't check for room for the character FOLLOW.
Nope. SVN_INT64_BUFFER_SIZE already includes
room for a terminating char and we will simply
overwrite the NUL that svn__ui64toa appends:
>> + SVN_ERR(writebuf_flush(conn, pool));
>> +
>> + written = svn__ui64toa(conn->write_buf + conn->write_pos, number);
>> + conn->write_buf[conn->write_pos + written] = follow;
>> + conn->write_pos += written + 1;
>> +
>> + return SVN_NO_ERROR;
>> +}
-- Stefan^2.
Re: svn commit: r1325899 - in
/subversion/trunk/subversion/libsvn_ra_svn: marshal.c ra_svn.h
Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Fri, Apr 27, 2012 at 05:39:05 +0200:
> Am 26.04.2012 11:05, schrieb Daniel Shahaf:
>> stefan2@apache.org wrote on Fri, Apr 13, 2012 at 18:36:46 -0000:
>>> Author: stefan2
>>> Date: Fri Apr 13 18:36:46 2012
>>> New Revision: 1325899
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1325899&view=rev
>>> Log:
>>> Various performance improvements to the ra_svn marshaller.
>>> The total effect is roughly a duplication in throughput.
>>>
>>> /* --- WRITING DATA ITEMS --- */
>>>
>>> +static svn_error_t *write_number(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
>>> + apr_uint64_t number, char follow)
>>> +{
>>> + apr_size_t written;
>>> +
>>> + if (conn->write_pos + SVN_INT64_BUFFER_SIZE>= sizeof(conn->write_buf))
>> Off-by-one? You don't check for room for the character FOLLOW.
>
> Nope. SVN_INT64_BUFFER_SIZE already includes
> room for a terminating char and we will simply
> overwrite the NUL that svn__ui64toa appends:
Worth a comment explaining that? Relying on SVN_INT64_BUFFER_SIZE
including room for a NUL --- which is not needed in this context --- is
of course correct, but could be confusing.
>>> + SVN_ERR(writebuf_flush(conn, pool));
>>> +
>>> + written = svn__ui64toa(conn->write_buf + conn->write_pos, number);
>>> + conn->write_buf[conn->write_pos + written] = follow;
>>> + conn->write_pos += written + 1;
>>> +
>>> + return SVN_NO_ERROR;
>>> +}
> -- Stefan^2.