Justin Erenkrantz wrote:
> On Dec 9, 2007 2:42 AM, Stefan Küng <tortoisesvn@gmail.com> wrote:
>> Well, I used the same approach as svn_wc_set_adm_dir() : that function
>> doesn't really need a pool argument either. IIRC, the discussion back
>> then was that all functions should take a pool argument, even if not
>> currently used so it can be extended more easily in the future.
>
> Well, used in the manner of this patch, a pool would be unsafe.
True. I've removed the pool argument.
>> AFAIK, there are not that many requests done for an operation. Which
>> means it shouldn't be a big problem?
>
> No - it is a big problem. There's at least 2x-per-file HTTP requests
> on an update (GET & PROPFIND).
>
> So, yah, that can be a lot of requests - so allocating from the
> session pool is badness - especially when we're dealing with a string
> that shouldn't change after initialization (based on the docstring).
I see. I only tested this with neon when I checked how much that code
part was called. That's why I didn't notice that. But it worked when I
ran some command with serf outside the debugger, so I assumed it was ok.
I've changed the patch to add a 'const char *useragent' to the
connection struct.
New patch attached.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
[[[
Allow the user-agent string sent over http to be defined by the client.
* subversion/include/svn_ra.h
(svn_ra_set_client_namestring): new function.
(svn_ra_get_client_namestring): new function.
* subversion/libsvn_ra/clientstring.c : new file containing helper
functions
* subversion/libsvn_ra_neon/session.c
(svn_ra_neon__open) : use svn_ra_get_client_namestring to
create the useragent string.
* subversion/libsvn_ra_serf/ra_serf.h : add an entry for the useragent
string to the connection struct.
* subversion/libsvn_ra_serf/propfind_buckets.c
(become_request) : use the useragent string stored in the connection.
* subversion/libsvn_ra_serf/util.c
(svn_ra_serf__conn_setup) : create the useragent string and store it
in the connection struct.
(svn_ra_serf__setup_serf_req) : use the useragent string of the
connection.
]]]
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 28326)
+++ subversion/include/svn_ra.h (working copy)
@@ -40,7 +40,28 @@
/* Misc. declarations */
+
/**
+ * Use @a name for the client string for http headers.
+ *
+ * @note This function changes global (per-process) state and must be
+ * called in a single-threaded context during the initialization of a
+ * Subversion client.
+ *
+ * @since New in 1.5.
+ */
+svn_error_t *
+svn_ra_set_client_namestring(const char *name);
+
+/**
+ * returns the svn client name string used in http headers
+ *
+ * @since New in 1.5.
+ */
+const char * svn_ra_get_client_namestring(void);
+
+
+/**
* Get libsvn_ra version information.
*
* @since New in 1.1.
Index: subversion/libsvn_ra/clientstring.c
===================================================================
--- subversion/libsvn_ra/clientstring.c (revision 0)
+++ subversion/libsvn_ra/clientstring.c (revision 0)
@@ -0,0 +1,55 @@
+/*
+ * clientstring.c: client string handling for http headers.
+ *
+ * ====================================================================
+ * Copyright (c) 2007 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+/* ==================================================================== */
+
+/*** Includes. ***/
+#include <apr_pools.h>
+#include <apr_strings.h>
+
+#include "svn_error.h"
+#include "svn_error_codes.h"
+#include "svn_ra.h"
+
+
+/* The default client name string */
+static const char default_ra_client_name[] = "SVN";
+
+/* The client name string that is actually used. */
+static const char *ra_client_name = default_ra_client_name;
+
+
+
+const char *
+svn_ra_get_client_namestring()
+{
+ return ra_client_name;
+}
+
+
+svn_error_t *
+svn_ra_set_client_namestring(const char *name)
+{
+ apr_pool_t *rootpool;
+ if (apr_pool_create(&rootpool, NULL))
+ abort();
+
+ ra_client_name = apr_pstrdup(rootpool, name);
+
+ return SVN_NO_ERROR;
+}
Property changes on: subversion\libsvn_ra\clientstring.c
___________________________________________________________________
Added: svn:eol-style
+ native
Index: subversion/libsvn_ra_neon/session.c
===================================================================
--- subversion/libsvn_ra_neon/session.c (revision 28326)
+++ subversion/libsvn_ra_neon/session.c (working copy)
@@ -821,7 +821,15 @@
unsigned int neon_auth_types = 0;
neonprogress_baton_t *neonprogress_baton =
apr_pcalloc(pool, sizeof(*neonprogress_baton));
+ const char * useragent = NULL;
+ useragent = apr_pstrcat(pool,
+ "SVN/",
+ SVN_VERSION,
+ "/",
+ svn_ra_get_client_namestring(pool),
+ NULL);
+
/* Sanity check the URI */
SVN_ERR(parse_url(uri, repos_URL));
@@ -930,8 +938,8 @@
ne_set_read_timeout(sess2, timeout);
}
- ne_set_useragent(sess, "SVN/" SVN_VERSION);
- ne_set_useragent(sess2, "SVN/" SVN_VERSION);
+ ne_set_useragent(sess, useragent);
+ ne_set_useragent(sess2, useragent);
/* clean up trailing slashes from the URL */
len = strlen(uri->path);
Index: subversion/libsvn_ra_serf/propfind_buckets.c
===================================================================
--- subversion/libsvn_ra_serf/propfind_buckets.c (revision 28326)
+++ subversion/libsvn_ra_serf/propfind_buckets.c (working copy)
@@ -158,7 +158,7 @@
hdrs_bkt = serf_bucket_request_get_headers(bucket);
serf_bucket_headers_setn(hdrs_bkt, "Host", ctx->conn->hostinfo);
- serf_bucket_headers_setn(hdrs_bkt, "User-Agent", USER_AGENT);
+ serf_bucket_headers_setn(hdrs_bkt, "User-Agent", ctx->conn->useragent);
if (ctx->conn->using_compression == TRUE)
{
serf_bucket_headers_setn(hdrs_bkt, "Accept-Encoding", "gzip");
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 28326)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -105,6 +105,9 @@
/* Current authorization value used for the proxy server; may be NULL */
char *proxy_auth_value;
+ /* user agent string */
+ const char *useragent;
+
} svn_ra_serf__connection_t;
/*
Index: subversion/libsvn_ra_serf/util.c
===================================================================
--- subversion/libsvn_ra_serf/util.c (revision 28326)
+++ subversion/libsvn_ra_serf/util.c (working copy)
@@ -49,6 +49,13 @@
serf_bucket_t *bucket;
svn_ra_serf__connection_t *conn = baton;
+ conn->useragent = apr_pstrcat(pool,
+ "SVN/",
+ SVN_VERSION,
+ "/",
+ svn_ra_get_client_namestring(pool),
+ NULL);
+
bucket = serf_bucket_socket_create(sock, conn->bkt_alloc);
if (conn->using_ssl)
{
@@ -271,7 +278,7 @@
hdrs_bkt = serf_bucket_request_get_headers(*req_bkt);
serf_bucket_headers_setn(hdrs_bkt, "Host", conn->hostinfo);
- serf_bucket_headers_setn(hdrs_bkt, "User-Agent", USER_AGENT);
+ serf_bucket_headers_setn(hdrs_bkt, "User-Agent", conn->useragent);
if (content_type)
{
serf_bucket_headers_setn(hdrs_bkt, "Content-Type", content_type);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Dec 9 14:26:38 2007