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

[PATCH] Fix stack smashing bugs

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2005-09-03 17:39:34 CEST

Hi,

The patch I've sent to fix some bugs I've introduced with the new
progress callbacks has been applied but then reverted because it
introduced a circular dependency which broke the build on some
platforms. (Revs 16010, 16011 - reverted in 16017).

So here's a patch which avoids that circular dependency. This is done by
not using svn_ra_create_callbacks() in wrapper_template.h but just
allocating the svn_ra_callbacks2_t object directly. I've added a comment
in svn_ra_create_callbacks() to make sure that if that function is ever
changed where to also change the initialization of that object.

The second patch attached is to have progress information for svn://
access too. But it can only be applied after the fix has been applied.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.tigris.org

[[[
Followup to r15948. Fix stack smashing bugs and other smaller issues.

Patch by: Stefan Küng <tortoisesvn@gmail.com> with input from brane and
          danderson.

* subversion/include/svn_ra.h
  (svn_ra_progress_notify_func_t): Add an apr_pool_t to the callback
    prototype.
  (svn_ra_create_callbacks): New function to create and initialize the
    svn_ra_callbacks2_t object.
  (svn_ra_callbacks2_t): Add comment about svn_ra_create_callbacks().
  (svn_ra_callbacks_t): Change comment.

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_create_callbacks): New function to create and initialize
    the svn_ra_callbacks2_t object.
  (svn_ra_open): Use the new svn_ra_create_callbacks().

* subversion/libsvn_ra/wrapper_template.h
  (compat_open): Allocate memory for the svn_ra_callbacks2_t object
    from pool.

* subversion/libsvn_ra_dav/session.c
  (neon_progress_baton_t): New structure to pass to the neon callback.
  (ra_dav_neonprogress): Pass the new apr_pool_t param to the progress
    callback.
  (svn_ra_dav__open): Use the new neon_progress_baton_t struct to pass
    to the neon callback function.
]]]
Index: subversion/libsvn_ra/wrapper_template.h
===================================================================
--- subversion/libsvn_ra/wrapper_template.h (Revision 16034)
+++ subversion/libsvn_ra/wrapper_template.h (Arbeitskopie)
@@ -45,20 +45,22 @@
                                  apr_pool_t *pool)
 {
   svn_ra_session_t *sess = apr_pcalloc (pool, sizeof (svn_ra_session_t));
- svn_ra_callbacks2_t callbacks2;
+ svn_ra_callbacks2_t *callbacks2 = NULL;
   sess->vtable = &VTBL;
   sess->pool = pool;
 
- callbacks2.open_tmp_file = callbacks->open_tmp_file;
- callbacks2.auth_baton = callbacks->auth_baton;
- callbacks2.get_wc_prop = callbacks->get_wc_prop;
- callbacks2.set_wc_prop = callbacks->set_wc_prop;
- callbacks2.push_wc_prop = callbacks->push_wc_prop;
- callbacks2.invalidate_wc_props = callbacks->invalidate_wc_props;
- callbacks2.progress_func = NULL;
- callbacks2.progress_baton = NULL;
+ callbacks = apr_pcalloc (pool, sizeof (svn_ra_callbacks2_t));
 
- SVN_ERR (VTBL.open (sess, repos_URL, &callbacks2, callback_baton,
+ callbacks2->open_tmp_file = callbacks->open_tmp_file;
+ callbacks2->auth_baton = callbacks->auth_baton;
+ callbacks2->get_wc_prop = callbacks->get_wc_prop;
+ callbacks2->set_wc_prop = callbacks->set_wc_prop;
+ callbacks2->push_wc_prop = callbacks->push_wc_prop;
+ callbacks2->invalidate_wc_props = callbacks->invalidate_wc_props;
+ callbacks2->progress_func = NULL;
+ callbacks2->progress_baton = NULL;
+
+ SVN_ERR (VTBL.open (sess, repos_URL, callbacks2, callback_baton,
                       config, pool));
   *session_baton = sess;
   return SVN_NO_ERROR;
Index: subversion/libsvn_ra/ra_loader.c
===================================================================
--- subversion/libsvn_ra/ra_loader.c (Revision 16034)
+++ subversion/libsvn_ra/ra_loader.c (Arbeitskopie)
@@ -230,6 +230,21 @@
   return SVN_NO_ERROR;
 }
 
+svn_error_t *
+svn_ra_create_callbacks (svn_ra_callbacks2_t **callbacks,
+ apr_pool_t *pool)
+{
+ /* Note: if you change this function, you also must change the
+ allocation of the svn_ra_callbacks2_t objects in
+ libsvn_ra/wrapper_template:compat_open()
+
+ The reason that those functions don't use svn_ra_create_callbacks
+ is to avoid circular dependencies with include files.
+ */
+ *callbacks = apr_pcalloc (pool, sizeof (svn_ra_callbacks2_t));
+ return SVN_NO_ERROR;
+}
+
 svn_error_t *svn_ra_open2 (svn_ra_session_t **session_p,
                            const char *repos_URL,
                            const svn_ra_callbacks2_t *callbacks,
@@ -290,17 +305,18 @@
 {
   /* Deprecated function. Copy the contents of the svn_ra_callbacks_t
      to a new svn_ra_callbacks2_t and call svn_ra_open2(). */
- svn_ra_callbacks2_t callbacks2;
- callbacks2.open_tmp_file = callbacks->open_tmp_file;
- callbacks2.auth_baton = callbacks->auth_baton;
- callbacks2.get_wc_prop = callbacks->get_wc_prop;
- callbacks2.set_wc_prop = callbacks->set_wc_prop;
- callbacks2.push_wc_prop = callbacks->push_wc_prop;
- callbacks2.invalidate_wc_props = callbacks->invalidate_wc_props;
- callbacks2.progress_func = NULL;
- callbacks2.progress_baton = NULL;
+ svn_ra_callbacks2_t *callbacks2;
+ SVN_ERR (svn_ra_create_callbacks (&callbacks2, pool));
+ callbacks2->open_tmp_file = callbacks->open_tmp_file;
+ callbacks2->auth_baton = callbacks->auth_baton;
+ callbacks2->get_wc_prop = callbacks->get_wc_prop;
+ callbacks2->set_wc_prop = callbacks->set_wc_prop;
+ callbacks2->push_wc_prop = callbacks->push_wc_prop;
+ callbacks2->invalidate_wc_props = callbacks->invalidate_wc_props;
+ callbacks2->progress_func = NULL;
+ callbacks2->progress_baton = NULL;
   return svn_ra_open2 (session_p, repos_URL,
- &callbacks2, callback_baton,
+ callbacks2, callback_baton,
                        config, pool);
 }
 
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (Revision 16034)
+++ subversion/include/svn_ra.h (Arbeitskopie)
@@ -180,7 +180,8 @@
  */
 typedef void (*svn_ra_progress_notify_func_t) (apr_off_t progress,
                                                apr_off_t total,
- void * baton);
+ void *baton,
+ apr_pool_t *pool);
 
 
 /**
@@ -322,6 +323,10 @@
  * Each routine takes a @a callback_baton originally provided with the
  * vtable.
  *
+ * In order to ease future compatibility, clients must use
+ * svn_ra_create_callbacks() to allocate and initialize this structure
+ * instead of doing so themselves.
+ *
  * @since New in 1.3.
  */
 typedef struct svn_ra_callbacks2_t
@@ -371,52 +376,25 @@
   void *progress_baton;
 } svn_ra_callbacks2_t;
 
-/** A collection of callbacks implemented by libsvn_client which allows
- * an RA layer to "pull" information from the client application, or
- * possibly store information. libsvn_client passes this vtable to
- * @c RA->open().
+/** Similar to svn_ra_callbacks2_t, except that the progress
+ * notification function and baton is missing.
  *
- * Each routine takes a @a callback_baton originally provided with the
- * vtable.
- *
  * @deprecated Provided for backward compatibility with the 1.2 API.
  */
 typedef struct svn_ra_callbacks_t
 {
- /** Open a unique temporary file for writing in the working copy.
- * This file will be automatically deleted when @a fp is closed.
- */
   svn_error_t *(*open_tmp_file) (apr_file_t **fp,
                                  void *callback_baton,
                                  apr_pool_t *pool);
   
- /** An authentication baton, created by the application, which is
- * capable of retrieving all known types of credentials.
- */
   svn_auth_baton_t *auth_baton;
 
- /*** The following items may be set to NULL to disallow the RA layer
- to perform the respective operations of the vtable functions.
- Perhaps WC props are not defined or are in invalid for this
- session, or perhaps the commit operation this RA session will
- perform is a server-side only one that shouldn't do post-commit
- processing on a working copy path. ***/
-
- /** Fetch working copy properties.
- *
- *<pre> ### we might have a problem if the RA layer ever wants a property
- * ### that corresponds to a different revision of the file than
- * ### what is in the WC. we'll cross that bridge one day...</pre>
- */
   svn_ra_get_wc_prop_func_t get_wc_prop;
 
- /** Immediately set new values for working copy properties. */
   svn_ra_set_wc_prop_func_t set_wc_prop;
 
- /** Schedule new values for working copy properties. */
   svn_ra_push_wc_prop_func_t push_wc_prop;
 
- /** Invalidate working copy properties. */
   svn_ra_invalidate_wc_props_func_t invalidate_wc_props;
 
 } svn_ra_callbacks_t;
@@ -439,6 +417,19 @@
 svn_error_t *
 svn_ra_initialize (apr_pool_t *pool);
 
+/** Initialize a callback structure.
+* Set @a *callbacks to a ra callbacks object, allocated in @a pool.
+*
+* In order to easy future compatibility, clients must use this
+* function to allocate and initialize the @c svn_ra_callbacks2_t
+* structure rather than doing so themselves.
+*
+* @since New in 1.3.
+*/
+svn_error_t *
+svn_ra_create_callbacks (svn_ra_callbacks2_t **callbacks,
+ apr_pool_t *pool);
+
 /**
  * A repository access session. This object is used to perform requests
  * to a repository, identified by an URL.
Index: subversion/libsvn_ra_dav/session.c
===================================================================
--- subversion/libsvn_ra_dav/session.c (Revision 16034)
+++ subversion/libsvn_ra_dav/session.c (Arbeitskopie)
@@ -561,14 +561,23 @@
 #endif /* if/else SVN_NEON_0_25 */
 }
 
+typedef struct neonprogress_baton_t
+{
+ void *progress_baton;
+ svn_ra_progress_notify_func_t progress_func;
+ apr_pool_t *pool;
+} neonprogress_baton_t;
+
 static void
-ra_dav_neonprogress(void * baton, off_t progress, off_t total)
+ra_dav_neonprogress(void *baton, off_t progress, off_t total)
 {
- /* Important: don't change the svn_ra_callbacks2_t struct here! */
- const svn_ra_callbacks2_t * callbacks = (svn_ra_callbacks2_t *)baton;
- if (callbacks->progress_func)
+ const neonprogress_baton_t *neonprogress_baton =
+ (neonprogress_baton_t *)baton;
+ if (neonprogress_baton->progress_func)
     {
- callbacks->progress_func(progress, total, callbacks->progress_baton);
+ neonprogress_baton->progress_func(progress, total,
+ neonprogress_baton->progress_baton,
+ neonprogress_baton->pool);
     }
 }
 
@@ -593,6 +602,8 @@
   svn_boolean_t compression;
   svn_config_t *cfg;
   const char *server_group;
+ neonprogress_baton_t *neonprogress_baton =
+ apr_pcalloc(pool, sizeof(neonprogress_baton_t));
 
   /* Sanity check the URI */
   if (ne_uri_parse(repos_URL, &uri)
@@ -802,13 +813,11 @@
           ne_ssl_trust_default_ca(sess2);
         }
     }
-
- /* Set the neon callback to make it call the svn_progress_notify_func_t
- * Note that ne_set_progress() takes a non-const baton as the third param.
- * Since we don't change the callback struct but only use the non-const
- * notification callback items of that struct, it's safe to cast */
- ne_set_progress(sess, ra_dav_neonprogress, (void*)callbacks);
- ne_set_progress(sess2, ra_dav_neonprogress, (void*)callbacks);
+ neonprogress_baton->pool = pool;
+ neonprogress_baton->progress_baton = callbacks->progress_baton;
+ neonprogress_baton->progress_func = callbacks->progress_func;
+ ne_set_progress(sess, ra_dav_neonprogress, (void*)neonprogress_baton);
+ ne_set_progress(sess2, ra_dav_neonprogress, (void*)neonprogress_baton);
   session->priv = ras;
 
   return SVN_NO_ERROR;

[[[
Add progress notification for ra_svn. This is for issue #901.

Patch by: Stefan Küng <tortoisesvn@gmail.com>

* subversion/libsvn_ra_svn/client.c (ra_svn_open): Initialize the
  progress callback function for svn connections.
* subversion/libsvn_ra_svn/marshal.c
  (writebuf_output): call the progress notification callback.
  (readbuf_input): Take an additional pool param and call the progress
  notification callback.
  (readbuff_fill, readbuf_read, readbuf_skip_leading_garbage,
  svn_ra_svn_skip_leading_garbage):
  Pass an additional pool param to readbuf_input.
* subversion/libsvn_ra_svn/ra_svn.h: include svn_ra.h explicitely
  (svn_ra_svn_conn_st): Add params for the progress notifications to
  the struct.
]]]
Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (Revision 15964)
+++ subversion/libsvn_ra_svn/client.c (Arbeitskopie)
@@ -644,6 +644,11 @@
       conn = svn_ra_svn_create_conn(sock, NULL, NULL, pool);
     }
 
+ conn->progress_func = callbacks->progress_func;
+ conn->progress_baton = callbacks->progress_baton;
+ conn->progress_progress = 0;
+ conn->progress_total = -1;
+
   /* Read server's greeting. */
   SVN_ERR(svn_ra_svn_read_cmd_response(conn, pool, "nnll", &minver, &maxver,
                                        &mechlist, &caplist));
Index: subversion/libsvn_ra_svn/marshal.c
===================================================================
--- subversion/libsvn_ra_svn/marshal.c (Revision 15964)
+++ subversion/libsvn_ra_svn/marshal.c (Arbeitskopie)
@@ -156,6 +156,12 @@
         status = apr_socket_send(conn->sock, data, &count);
       else
         status = apr_file_write(conn->out_file, data, &count);
+ conn->progress_progress += count;
+ if (conn->progress_func)
+ {
+ conn->progress_func(conn->progress_progress, conn->progress_total,
+ pool, conn->progress_baton);
+ }
       if (status)
         return svn_error_wrap_apr(status, _("Can't write to connection"));
       if (count == 0)
@@ -236,7 +242,7 @@
 
 /* Read data from socket or input file as appropriate. */
 static svn_error_t *readbuf_input(svn_ra_svn_conn_t *conn, char *data,
- apr_size_t *len)
+ apr_size_t *len, apr_pool_t *pool)
 {
   apr_status_t status;
 
@@ -247,6 +253,12 @@
     status = apr_socket_recv(conn->sock, data, len);
   else
     status = apr_file_read(conn->in_file, data, len);
+ conn->progress_progress += *len;
+ if (conn->progress_func)
+ {
+ conn->progress_func(conn->progress_progress, conn->progress_total,
+ pool, conn->progress_baton);
+ }
   if (conn->sock && conn->block_handler)
     apr_socket_timeout_set(conn->sock, 0);
   if (status && !APR_STATUS_IS_EOF(status))
@@ -265,7 +277,7 @@
   assert(conn->read_ptr == conn->read_end);
   SVN_ERR(writebuf_flush(conn, pool));
   len = sizeof(conn->read_buf);
- SVN_ERR(readbuf_input(conn, conn->read_buf, &len));
+ SVN_ERR(readbuf_input(conn, conn->read_buf, &len, pool));
   conn->read_ptr = conn->read_buf;
   conn->read_end = conn->read_buf + len;
   return SVN_NO_ERROR;
@@ -304,7 +316,7 @@
     {
       SVN_ERR(writebuf_flush(conn, pool));
       count = end - data;
- SVN_ERR(readbuf_input(conn, data, &count));
+ SVN_ERR(readbuf_input(conn, data, &count, pool));
       data += count;
     }
 
@@ -319,7 +331,8 @@
   return SVN_NO_ERROR;
 }
 
-static svn_error_t *readbuf_skip_leading_garbage(svn_ra_svn_conn_t *conn)
+static svn_error_t *readbuf_skip_leading_garbage(svn_ra_svn_conn_t *conn,
+ apr_pool_t *pool)
 {
   char buf[256]; /* Must be smaller than sizeof(conn->read_buf) - 1. */
   const char *p, *end;
@@ -331,7 +344,7 @@
     {
       /* Read some data directly from the connection input source. */
       len = sizeof(buf);
- SVN_ERR(readbuf_input(conn, buf, &len));
+ SVN_ERR(readbuf_input(conn, buf, &len, pool));
       end = buf + len;
 
       /* Scan the data for '(' WS with a very simple state machine. */
@@ -619,7 +632,7 @@
 svn_error_t *svn_ra_svn_skip_leading_garbage(svn_ra_svn_conn_t *conn,
                                              apr_pool_t *pool)
 {
- return readbuf_skip_leading_garbage(conn);
+ return readbuf_skip_leading_garbage(conn, pool);
 }
 
 /* --- READING AND PARSING TUPLES --- */
Index: subversion/libsvn_ra_svn/ra_svn.h
===================================================================
--- subversion/libsvn_ra_svn/ra_svn.h (Revision 15964)
+++ subversion/libsvn_ra_svn/ra_svn.h (Arbeitskopie)
@@ -29,6 +29,7 @@
 #include <apr_file_io.h>
 #include <apr_thread_proc.h>
 #include <svn_ra_svn.h>
+#include "svn_ra.h"
 
 /* Handler for blocked writes. */
 typedef svn_error_t *(*ra_svn_block_handler_t)(svn_ra_svn_conn_t *conn,
@@ -53,6 +54,10 @@
   ra_svn_block_handler_t block_handler;
   void *block_baton;
   apr_hash_t *capabilities;
+ svn_ra_progress_notify_func_t progress_func;
+ void *progress_baton;
+ apr_off_t progress_progress;
+ apr_off_t progress_total;
   apr_pool_t *pool;
 };
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Sep 3 17:40:28 2005

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