On Wed, 2010-03-03 at 11:10 -0500, Greg Stein wrote:
> On Wed, Mar 3, 2010 at 08:48, Neels J Hofmeyr <neels_at_elego.de> wrote:
> >...
> >> If the caller creates the file, the caller chooses the file's name. We
> >> have to trust the caller to use the right kind of creation functions to
> >> give a safely unique name.
>
> libsvn_wc. no big deal.
I have just noticed that we already sort-of leak the temporary file name
in a public API, svn_wc_transmit_text_deltas3():
/** Send the local modifications for versioned file @a local_abspath (with
* matching @a file_baton) through @a editor, then close @a file_baton
* afterwards. Use @a scratch_pool for any temporary allocation.
*
* This process creates a copy of @a local_abspath with keywords and eol
* untranslated. If @a tempfile is non-NULL, set @a *tempfile to the
* absolute path to this copy, allocated in @a result_pool. Do not clean
* up the copy; caller can do that. If @a digest is non-NULL, put the MD5
* checksum of the temporary file into @a digest, which must point to @c
* APR_MD5_DIGESTSIZE bytes of storage. (The purpose of handing back the
* tmp copy is that it is usually about to become the new text base anyway,
* but the installation of the new text base is outside the scope of this
* function.)
... */
svn_wc_transmit_text_deltas3(const char **tempfile, ...)
It's used by svn_client__do_commit() which passes the tempfiles and
checksums up to its callers which are ...
svn_client_commit4() which just removes those files if it encounters
an error (it doesn't try to use them or pass them anywhere else);
libsvn_client:wc_to_repos_copy() which doesn't look at them.
That is the only usage in Subversion. Of course it may possibly be used
by other clients.
We can still implement this API compatibly, of course, even if we were
to want an implementation that ; it just means we have to produce a
temporary file even if we would not otherwise have done so. We could say
that it's none of the caller's business to look in that file and it
should be doing nothing other than deleting it, in which case we could
cheat and return something other than a true copy (an empty file? no
file?).
I don't think this is a critical problem, but it's worth being aware of.
I haven't checked whether there are any other leaks.
> >> If the caller creates the file, the pristine store can't keep track of
> >> what's in its temporary directory. Does it matter? Maybe not, yet.
>
> No.
Hmm.
> >> If the caller creates the file, the caller chooses the file's
> >> permissions. The pristine store loses the ability to control privacy.
> >> Does it matter? Maybe not, yet.
>
> bunk.
>
> install:
> move file
> chmod file
>
> we do this *today* ... there isn't anything new tech here.
OK, sure.
But as I said to Neels, this "write" API isn't on the critical path so
I'm now looking at *using* the one we have (even though it's not my
personal favourite).
- Julian
Received on 2010-03-03 18:32:35 CET