[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] renaming the function as per convention

From: Karl Fogel <kfogel_at_google.com>
Date: 2006-08-31 23:58:47 CEST

Kamesh Jayachandran <kamesh@collab.net> writes:
> Can someone look in to this?

Done in r21327 :-). Note how the log message differs -- when
replacing function A with B, write it like this:

  * path/to/file
    (B): Rename to A.

(Except I said "replaces", slightly less accurate, but allowed me to
fit more on the line...)

-K

> 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

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 31 23:59:55 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.