>
>> --- subversion/libsvn_ra_dav/mergeinfo.c (revision 22315)
>> +++ subversion/libsvn_ra_dav/mergeinfo.c (working copy)
>> @@ -127,6 +127,8 @@
>> {
>> struct mergeinfo_baton *mb = baton;
>> apr_size_t nlen = len;
>> + char tmpc;
>> + char *mutable_cdata = (char*)cdata;
>>
>> switch (state)
>> {
>> @@ -134,7 +136,13 @@
>> mb->curr_path = apr_pstrndup(mb->pool, cdata, nlen);
>> break;
>> case ELEM_merge_info_info:
>> - mb->curr_info = apr_pstrndup(mb->pool, cdata, nlen);
>> + /* if the data is having multiple lines we need to concatenate the
>> + * current data with the previous one.
>> + */
>> + tmpc = mutable_cdata[nlen];
>>
>
> This will index off the end of the "mutable_cdata" string, which is
> only "len" character long, rather than the "len + 1" characters this
> would require.
>
It is 'len' not 'len + 1'.
Say for data
'/trunk/abc:1-15....something_else_we_should_not_bother........................'
len is 16(including the new line).
so setting cdata[16] = '\0' sets the overwrites a char next to '\n'.
> Looks like an attempt to avoid copying "cdata"...
>
Yes.
>
>> + mutable_cdata[len]='\0';
>>
>
> We did it again, only this time we clobbered something else's memory.
> This time, we used "len" rather than "nlen" as our offset variable.
> Which we use doesn't really matter, but we should be consistent.
>
> Spacing is also missing from around the "=" character.
>
>
Sorry for the confusion.
>> + mb->curr_info = apr_pstrcat(mb->pool, mb->curr_info, cdata, NULL);
>>
>
> What happens when mb->curr_info hasn't been set yet?
>
Yes realised the same and sent the followup patch on the same. Anyway
you fixed the same.
>
>> + mutable_cdata[nlen] = tmpc;
>> break;
>>
>
> Now we attempt to restore the data we previously clobbered, again
> changing the offset variable we're using.
>
Yes I agree that 'inconsistent variable usage is bad'.
Are you ok if I give a patch which uses a 'memory clobbering+restoration
approach' with consistent 'len or nlen not both'?
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 21 07:26:03 2006