Re: [PATCH] fix for issue #2556: support working copies on the root of a (virtual) drive

> > Reading it Garrett's way: we may want to rev the APIs just because of the
> pool requirement. But, when the pool being
> > passed in is NULL (as the old api compat routines would), we'd need to
> create our own pool with svn_pool_create(NULL);
> > something along these lines:
>
> Included a slightly simpler form of your example.

Apart from this little nit:

+ if ((status == APR_SUCCESS ||
+ status == APR_EINCOMPLETE) &&
+ rel_path[0] == '\0')
+ {
+ result = TRUE;
+ goto cleanup;
+ }
+
+ cleanup:
+ if (!pool)
+ apr_pool_destroy(strpool);
+ return result;
+}

Which is shorter written as

+ if ((status == APR_SUCCESS ||
+ status == APR_EINCOMPLETE) &&
+ rel_path[0] == '\0')
+ result = TRUE;
+
+ cleanup:
+ if (!pool)
+ apr_pool_destroy(strpool);
+ return result;
+}

I'm +1 on committing this. Note that that's only based on code review:
I have no means of testing it here, but judging from the review, it's
good.

> I didn't rev any of the functions in path.c nor deleted is_canonical. This
> can be done, it'll improve the memory usage slightly.

That, and in the common case (where the revved api would be used), you
wouldn't need to resort to allocating a global pool, so you wouldn't
need to resort to obtaining a global mutex, which should be faster
too. The fallback scenario would be to obtain the global mutex (and
thus be slow and less memory-efficient).

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 22 23:39:03 2006