On 21 April 2014 19:49, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> wrote:
> On Mon, Apr 21, 2014 at 9:37 AM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>>
>> On 20 April 2014 22:38, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
>> wrote:
>> >
>> > On Sat, Apr 19, 2014 at 10:32 PM, Ivan Zhakov <ivan_at_visualsvn.com>
>> > wrote:
>> >>
>> >> On 25 October 2013 15:12, <stefan2_at_apache.org> wrote:
>> >> > Author: stefan2
>> >> > Date: Fri Oct 25 11:12:27 2013
>> >> > New Revision: 1535686
>> >> >
[...]
>> >> > +/* Determine the checksum for the SIZE bytes of data starting at
>> >> > START
>> >> > + * in FILE and return the result in *FNV1_CHECKSUM.
>> >> > + * Use POOL for tempoary allocations.
>> >> > + */
>> >> > +static svn_error_t *
>> >> > +fnv1a_checksum_on_file_range(apr_uint32_t *fnv1_checksum,
>> >> > + apr_file_t *file,
>> >> > + apr_off_t start,
>> >> > + apr_off_t size,
>> >> > + apr_pool_t *pool)
>> >> > +{
>> >> > + char buffer[4096];
>> >> > +
>> >> Why you're using non-standard sized buffer for IO operations on stack?
>> >> It should be apr_palloc(SVN__STREAM_CHUNK_SIZE). Is not it?
>> >
>> >
>> > No need to use dynamically alloc the buffer
>> > but not using SVN__STREAM_CHUNK_SIZE
>> > as clearly an oversight.
>> >
>> Your reply doesn't have any arguments why no need to use buffer from
>> pool like we do in all other places.
>
>
> You now have the chance to make your statement
> actually true. Please fix the following occurrences
> and check their callers for proper pool usage:
>
> trunk/subversion/libsvn_delta/text_delta.c:923
> trunk/subversion/libsvn_subr/io.c:805
>
Hi Stefan,
Well, I didn't notice that places. Sorry for confusing you.
It's good time to make consistent in either way.
Where temporary read buffers should be allocated: on stack or in heap
(pool)? I'm fine with both approaches, but I would like to make it
consistent. What do you think?
--
Ivan Zhakov
Received on 2014-04-21 19:05:52 CEST