[[[
Plug a memory leak by avoiding repeated failing attempts at apr_xlate setup.

Review by: jerenkrantz
Suggested by: jerenkrantz
              dberlin
              ghudson
              brane
              striker

* subversion/libsvn_subr/utf.c
  (enotimpl_handle_node): New file-static variable.
  (get_xlate_handle_node): Use enotimpl_handle_node to determine
  whether to even bother calling apr_xlate_open again.  Update and fix
  doc string, which was wrong before anyway.
]]]


Index: subversion/libsvn_subr/utf.c
===================================================================
--- subversion/libsvn_subr/utf.c	(revision 15373)
+++ subversion/libsvn_subr/utf.c	(working copy)
@@ -68,6 +68,17 @@
    memory leak. */
 static apr_hash_t *xlate_handle_hash = NULL;
 
+/* Non-NULL iff apr_xlate_open() ever returned APR_ENOTIMPL.  Since
+   there would be no point ever calling apr_xlate_open() again in this
+   process, we can save ourself some setup work (including memory
+   allocations) by caching a handle_node to return in that case;
+   enotimpl_handle_node->handle will be NULL, of course.
+   apr_xlate_open() could return APR_ENOTIMPL because APR_HAS_XLATE is
+   not defined, or for some other reason.  For our purposes it doesn't
+   matter, we just want to remember that it is not implemented for
+   this process. */
+static xlate_handle_node_t *enotimpl_handle_node = NULL;
+
 /* Clean up the xlate handle cache. */
 static apr_status_t
 xlate_cleanup (void *arg)
@@ -122,12 +133,13 @@
     }
 }
 
-/* Return an apr_xlate handle for converting from FROMPAGE to
-   TOPAGE. Create one if it doesn't exist in USERDATA_KEY. If
-   unable to find a handle, or unable to create one because
-   apr_xlate_open returned APR_EINVAL, then set *RET to null and
-   return SVN_NO_ERROR; if fail for some other reason, return
-   error. */
+/* Set *RET to an apr_xlate handle for converting from FROMPAGE to
+   TOPAGE, creating the handle if it doesn't exist in USERDATA_KEY.
+   If unable to find a handle, or unable to create one because
+   apr_xlate_open returned APR_EINVAL or APR_ENOTIMPL, then
+   *RET->handle will be NULL (and in the ENOTIMPL case, allocate *RET
+   itself in a child of the top-level pool, not in POOL).  If fail for
+   any other reason, return error. */
 static svn_error_t *
 get_xlate_handle_node (xlate_handle_node_t **ret,
                        const char *topage, const char *frompage,
@@ -199,6 +211,14 @@
   if (userdata_key && xlate_handle_hash)
     pool = apr_hash_pool_get (xlate_handle_hash);
 
+  /* If we failed due to ENOTIMPL before, then we'd certainly fail
+     now, so just return without trying. */
+  if (enotimpl_handle_node)
+    {
+      *ret = enotimpl_handle_node;
+      return SVN_NO_ERROR;
+    }
+  
   /* Try to create a handle. */
   *ret = apr_palloc (pool, sizeof(xlate_handle_node_t));
   apr_err = apr_xlate_open (&(*ret)->handle, topage, frompage, pool);
@@ -229,6 +249,19 @@
 
   if (APR_STATUS_IS_EINVAL (apr_err) || APR_STATUS_IS_ENOTIMPL (apr_err))
     {
+      if (APR_STATUS_IS_ENOTIMPL (apr_err))
+        {
+          /* Remember for next time that we needn't bother trying.
+             Use the top-level pool because we have no lifetime
+             guarantees about our local pool parameter. */
+          enotimpl_handle_node = apr_palloc (svn_pool_create (NULL),
+                                             sizeof (*enotimpl_handle_node));
+          enotimpl_handle_node->handle = NULL;
+          enotimpl_handle_node->frompage = NULL;
+          enotimpl_handle_node->topage = NULL;
+          enotimpl_handle_node->next = NULL;
+        }
+
       (*ret)->handle = NULL;
       return SVN_NO_ERROR;
     }