Can someone look in to this?
With regards
Kamesh Jayachandran
Kamesh Jayachandran wrote:
> Malcolm Rowe wrote:
>> On Mon, Aug 28, 2006 at 01:57:32PM +0530, Kamesh Jayachandran wrote:
>>
>>> static function is not supposed to have 'svn_' prefix.
>>> We use '__' in function names that is used by other 'C' files
>>> within the same library.
>>>
>>> * subversion/libsvn_diff/diff4.c
>>> (svn_diff__adjust): Renamed to _diff_adjust.
>>> (svn_diff_diff4): Calls _diff_adjust.
>>>
>>>
>>
>> I don't think we use leading underscores anywhere else; why not
>> 'diff_adjust' (or even 'adjust_diff', since that presumably is a better
>> description of what it's doing).
>>
>>
> I am fine with 'adjust_diff'.
>
>>>
>>> static APR_INLINE void
>>> -svn_diff__adjust(svn_diff_t *diff, svn_diff_t *adjust)
>>> +_diff_adjust(svn_diff_t *diff, svn_diff_t *adjust)
>>> {
>>>
>>
>> Incidentally, isn't this function a bit large to be inlined?
>>
> Yes.
> Find the attached patch.
>
> With regards
> Kamesh Jayachandran
>
> [[[
> Patch by: Kamesh Jayachandran <kamesh@collab.net>
> Reviewed by: Malcolm Rowe <malcolm-svn-dev@farside.org.uk>
>
> static function is not supposed to have 'svn_' prefix.
> We use '__' in function names that is used by other 'C' files
> within the same library.
>
> * subversion/libsvn_diff/diff4.c
> (svn_diff__adjust):
> Renamed to adjust_diff.
> Removed the 'APR_INLINE' attribute for this function.
> (svn_diff_diff4): Calls adjust_diff.
> ]]]
>
> ------------------------------------------------------------------------
>
> Index: subversion/libsvn_diff/diff4.c
> ===================================================================
> --- subversion/libsvn_diff/diff4.c (revision 21287)
> +++ subversion/libsvn_diff/diff4.c (working copy)
> @@ -102,8 +102,8 @@
> */
>
>
> -static APR_INLINE void
> -svn_diff__adjust(svn_diff_t *diff, svn_diff_t *adjust)
> +static void
> +adjust_diff(svn_diff_t *diff, svn_diff_t *adjust)
> {
> svn_diff_t *hunk;
> apr_off_t range_start;
> @@ -240,7 +240,7 @@
> */
> lcs_adjust = svn_diff__lcs(position_list[3], position_list[2], subpool3);
> diff_adjust = svn_diff__diff(lcs_adjust, 1, 1, FALSE, subpool3);
> - svn_diff__adjust(diff_ol, diff_adjust);
> + adjust_diff(diff_ol, diff_adjust);
>
> svn_pool_clear(subpool3);
>
> @@ -249,7 +249,7 @@
> */
> lcs_adjust = svn_diff__lcs(position_list[1], position_list[3], subpool3);
> diff_adjust = svn_diff__diff(lcs_adjust, 1, 1, FALSE, subpool3);
> - svn_diff__adjust(diff_ol, diff_adjust);
> + adjust_diff(diff_ol, diff_adjust);
>
> /* Get rid of the position lists for original and ancestor, and delete
> * our scratchpool.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 31 11:37:09 2006