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

Re: svn commit: r924722 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/switch.c libsvn_client/update.c libsvn_wc/deprecated.c libsvn_wc/update_editor.c

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 18 Mar 2010 20:21:02 -0400

On Thu, Mar 18, 2010 at 19:54, Julian Foad <julian.foad_at_wandisco.com> wrote:
> On Thu, 2010-03-18, Greg Stein wrote:
>> On Thu, Mar 18, 2010 at 07:34, <julianfoad_at_apache.org> wrote:
>> > Author: julianfoad
>> > Date: Thu Mar 18 11:34:45 2010
>> > New Revision: 924722
>> >
>> > URL: http://svn.apache.org/viewvc?rev=924722&view=rev
>> > Log:
>> > Clean up two revved WC API function signatures: move the wc_ctx parameter
>> > before the first of the main input parameters, to keep those together and
>> > to minimize the difference from the previous version. Do some doc string
>> > clean-ups too.
>> >
>> > * subversion/include/svn_wc.h
>> > (svn_wc_get_status_editor5): Fix a parameter name typo. Note doc string
>> > out-of-dateness.
>> > (svn_wc_crawl_revisions5): Note doc string out-of-dateness.
>> > (svn_wc_get_update_editor4): Swap revision and wc_ctx parameters.
>> > (svn_wc_get_switch_editor4): Swap revision and wc_ctx parameters. Refer to
>> > svn_wc_get_update_editor4's doc string for most parameters to make it
>> > easy for the reader to see that there are no differences.
>>
>> Why swap these parameters? REVISION (or TARGET_REVISION) is an OUT
>> parameter. We always place OUT parameters first, and then the WC_CTX,
>> then the other params.
>
> Bert discussed this with me on IRC too. My log message said why, but
> actually there's more to it. I hadn't been able to discern a reason why
> the rev-number param was moved away (relative to the previous version of
> the API) from the other target parameters in the first place. After
> this commit, Bert said he'd done it for the reason you state - output
> params come before input params. But this parameter isn't an output
> from this function, at least not in the usual sense. It's an address
> given to this function, an address for this function to store away but
> definitely not to read from or write to. A different function will
> later retrieve that address and write a revision number to it.
>
> In that sense, the pointer is an input to this function, whereas in
> another sense we could call it an "output pointer" meaning it's a
> pointer that will only ever be written through (not read from), but it
> is neither a classical "input pointer parameter" nor a classical "output
> pointer parameter" for this function. Indeed, the code sequence in the
> present only caller of svn_wc_get_update_editor4() depends on this
> function call NOT overwriting the pointed-to revision number.
>
> I don't know of a precedent or rule for this specific situation so I
> feel either way is OK. Bert said he was happy to call it an input
> parameter. If you want to declare that this kind of parameter should be
> grouped with classical output pointer parameters, I would be fine with
> that decision and would happily change it.

Conceptually, it is still an output parameter. You are saying "put the
result HERE." That's an output parameter.

Sure, it takes a while to get the value; you have to drive the editor.
But that's just timing. The concept is still an OUT param.

I certainly have no problem with a redesign of the dataflow, but for
now... I see it as an OUT parameter.

(note that we have some similar-style params with some
checksum-computing functions, too)

Cheers,
-g
Received on 2010-03-19 01:21:28 CET

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