You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2007/12/14 05:07:17 UTC

svn commit: r604097 - in /httpd/mod_ftp/trunk: include/mod_ftp.h modules/ftp/ftp_commands.c modules/ftp/ftp_connection.c

Author: wrowe
Date: Thu Dec 13 20:07:15 2007
New Revision: 604097

URL: http://svn.apache.org/viewvc?rev=604097&view=rev
Log:
Complete the memory scope refactoring; create an fc->login_pool which
is cleared upon each invocation of the USER cmd, so that logging in as
multiple users repeatedly doesn't result in the growth of the footprint.

Remaining c->pool references are exactly that; connection wide, one time
allocations which are not an issue.

Modified:
    httpd/mod_ftp/trunk/include/mod_ftp.h
    httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c
    httpd/mod_ftp/trunk/modules/ftp/ftp_connection.c

Modified: httpd/mod_ftp/trunk/include/mod_ftp.h
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/include/mod_ftp.h?rev=604097&r1=604096&r2=604097&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/include/mod_ftp.h (original)
+++ httpd/mod_ftp/trunk/include/mod_ftp.h Thu Dec 13 20:07:15 2007
@@ -274,6 +274,7 @@
     const char *response_notes; /* Notes for response handling. */
 
     /* User information */
+    apr_pool_t *login_pool;   /* Child of c->pool reset on every USER cmd */
     int logged_in;
     const char *user;
     const char *authorization;

Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c?rev=604097&r1=604096&r2=604097&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c (original)
+++ httpd/mod_ftp/trunk/modules/ftp/ftp_commands.c Thu Dec 13 20:07:15 2007
@@ -1032,7 +1032,7 @@
     r->the_request = apr_pstrdup(r->pool, "PASS xxx");
 
     userpass = apr_psprintf(r->pool, "%s:%s", fc->user, arg);
-    fc->authorization = apr_psprintf(c->pool, "Basic %s",
+    fc->authorization = apr_psprintf(fc->login_pool, "Basic %s",
                                      ap_pbase64encode(r->pool, userpass));
 
     /* This is normally set in the dispatcher, but since we just read
@@ -1046,7 +1046,7 @@
         ap_conf_vector_t *conf_vector;
         module *modp;
 
-        ftpserver = apr_pcalloc(c->pool, sizeof(*ftpserver));
+        ftpserver = apr_pcalloc(fc->login_pool, sizeof(*ftpserver));
         memcpy(ftpserver, c->base_server, sizeof(*ftpserver));
 
         /* We need to count the number of modules in order to know
@@ -1057,7 +1057,7 @@
          * to do it this way.
          */
         for (i = 0; ap_loaded_modules[i]; i++);
-        conf_vector = apr_pcalloc(c->pool, sizeof(void *) * i);
+        conf_vector = apr_pcalloc(fc->login_pool, sizeof(void *) * i);
 
         for (modp = ap_top_module; modp; modp = modp->next) {
             /* This is a hack.  Basically, to keep the user in thier
@@ -1072,7 +1072,7 @@
             if (modp == &core_module) {
                 core_server_config *core;
 
-                ftpcore = apr_pcalloc(c->pool, sizeof(*ftpcore));
+                ftpcore = apr_pcalloc(fc->login_pool, sizeof(*ftpcore));
                 core = 
                      ap_get_module_config(c->base_server->module_config,
                                           modp);
@@ -1108,9 +1108,9 @@
                           fsc->docrootenv);
         }
         else if (((rv = apr_filepath_merge(&tmppath, NULL, docroot, 
-                                         APR_FILEPATH_TRUENAME 
-                                       | APR_FILEPATH_NOTRELATIVE, 
-                                         c->pool)) != APR_SUCCESS)
+                                           APR_FILEPATH_TRUENAME |
+                                           APR_FILEPATH_NOTRELATIVE, 
+                                           fc->login_pool)) != APR_SUCCESS)
               || (rv = APR_ENOTDIR, !ap_is_directory(r->pool, tmppath))) {
             ap_log_perror(APLOG_MARK, APLOG_WARNING, rv,
                           r->pool,
@@ -1203,7 +1203,7 @@
         if (!userdir || (strncmp(userdir, fc->cwd, strlen(userdir)) != 0)) {
             goto pass_try_again;
         }
-        ftpcore->ap_document_root = apr_pstrcat(c->pool, 
+        ftpcore->ap_document_root = apr_pstrcat(fc->login_pool, 
                   ftpcore->ap_document_root, userdir, NULL);
         apr_cpystrn(fc->cwd, "/", APR_PATH_MAX + 1);
     }
@@ -2714,8 +2714,9 @@
         ftp_limitlogin_loggedout(fc->user, c, r->pool);
     }
     fc->logged_in        = 0;
-    
-    fc->user = apr_pstrdup(c->pool, arg);
+    apr_pool_clear(fc->login_pool);
+
+    fc->user = apr_pstrdup(fc->login_pool, arg);
 
     if ((fsc->options & FTP_OPT_REQUIRESSL) && !fc->is_secure) {
         fc->response_notes = apr_pstrdup(r->pool,
@@ -2729,7 +2730,8 @@
         fc->response_notes = apr_pstrdup(r->pool,
                                          "Guest login ok, type your email "
                                          "address as the password");
-        fc->user = apr_pstrdup(c->pool, "anonymous");	/* force this for limit control */
+	/* force this for limit control */
+        fc->user = apr_pstrdup(fc->login_pool, "anonymous");
     }
     else {
         fc->response_notes = apr_psprintf(r->pool, "Password required for %s",

Modified: httpd/mod_ftp/trunk/modules/ftp/ftp_connection.c
URL: http://svn.apache.org/viewvc/httpd/mod_ftp/trunk/modules/ftp/ftp_connection.c?rev=604097&r1=604096&r2=604097&view=diff
==============================================================================
--- httpd/mod_ftp/trunk/modules/ftp/ftp_connection.c (original)
+++ httpd/mod_ftp/trunk/modules/ftp/ftp_connection.c Thu Dec 13 20:07:15 2007
@@ -32,7 +32,7 @@
  *
  * Returns: nothing 
  */
-static apr_status_t initialize_ftp_connection(conn_rec *c, ftp_connection *fc)
+static void initialize_ftp_connection(conn_rec *c, ftp_connection *fc)
 {
     /* The ftp_connection structure is calloc'ed so only initalize
      * the members that we need to.
@@ -53,7 +53,8 @@
     fc->rename_from[0]   = '\0';
 
     fc->cntlsock  = ap_get_module_config(c->conn_config, &core_module);
-    return apr_pool_create(&fc->data_pool, c->pool);
+    apr_pool_create(&fc->login_pool, c->pool);
+    apr_pool_create(&fc->data_pool, c->pool);
 }
 
 /* ftp_ssl_init: Fakes a read on the SSL filters to force initialization.
@@ -172,12 +173,7 @@
      * Allocate for FTP connection structure, and initialize 
      */
     fc = apr_pcalloc(c->pool, sizeof(*fc));
-    if ((rv = initialize_ftp_connection(c, fc)) != APR_SUCCESS) {
-        /* Failure creating the data subpool */
-        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, c->base_server,
-                     "Failure initializing FTP connection pool.");
-        return OK;
-    }
+    initialize_ftp_connection(c, fc);
 
     /* Check for implicit security on the control connection */
     if (!fsc->implicit_ssl) {