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

Re: [PATCH]remove adm_access batons in svn_wc_translated_file3()

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 11 Aug 2009 11:31:29 +0100

On Mon, 2009-08-10, Hyrum K. Wright wrote:
> On Aug 10, 2009, at 8:08 PM, HuiHuang wrote:
> >>> * Output files are created in the temp file area belonging to
> >>> - * @a versioned_file. By default they will be deleted at pool cleanup.
> >>> + * @a versioned_abspath. By default they will be deleted at scratch_pool cleanup.
> >>> *
> >>> * If @c SVN_WC_TRANSLATE_NO_OUTPUT_CLEANUP is specified, the default
> >>> - * pool cleanup handler to remove @a *xlated_path is not registered.
> >>> + * result_pool cleanup handler to remove @a *xlated_path is not registered.
> >
> >> The first of these two paragraphs refers to "scratch_pool cleanup" and
> >> the second to "result_pool cleanup". Shouldn't they both refer to the
> >> same pool?
> >
> > Originally the function use a single pool for memory allocation
> > while now we use result_pool
> > for output variables and scratch_pool for the temp.
> >
> > The temp files are allocated from scratch_pool and xlated_path is
> > allocated from result_pool,
> > that is why there is difference.
> >
> > That is what I think about it, is that right?
>
> Are they both output values? I believe so, and if that's the case
> then they should both go in result_pool.

This function creates only ONE output file, which is the file named by
*xlated_path. (The plural wording, "Output files are created..." is
misleading and should be changed to "The output file is created...".)

Both of those paragraphs in the doc string are talking about this same
file. Therefore they MUST talk about the same pool. (And they should be
joined together as a single paragraph.)

So here is a re-write of that part of the doc string:

  * The output file is created in the temp file area belonging to
  * @a versioned_abspath. By default it will be deleted at result_pool
  * cleanup. If @a flags includes @c SVN_WC_TRANSLATE_NO_OUTPUT_CLEANUP,
  * the default result_pool cleanup handler to remove @a *xlated_path is
  * not registered.

> (This is actually a bit tricky, since the file isn't strictly an
> output parameter, but it is instead a tangible side-effect of the
> function, which we want to persist for some amount of time. Stuff
> allocated in scratch_pool isn't guaranteed to exist beyond the scope
> of the function.)

I would say the generated file, and its name, together are the output of
this function. The result_pool is intended to be used for storing
results which have to be allocated somewhere in order to survive beyond
the end of the function. Although we most often use pools just for
storing data in memory, it is perfectly valid to use the pool also to
control the persistence of a file that has been created as an output of
the function.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2382419
Received on 2009-08-11 12:31:59 CEST

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