Re: svn commit: r9868 - trunk/subversion/libsvn_wc

jpieper@tigris.org writes:

> Author: jpieper
> Date: Sun May 23 15:52:47 2004
> New Revision: 9868

> Modified: trunk/subversion/libsvn_wc/log.c
> ==============================================================================
> --- trunk/subversion/libsvn_wc/log.c (original)
> +++ trunk/subversion/libsvn_wc/log.c Sun May 23 15:52:47 2004
> @@ -1274,6 +1274,9 @@
> char buf[BUFSIZ];
> apr_size_t buf_len;
> apr_file_t *f = NULL;
> + const char *logfile_path;
> + int log_number;
> + apr_pool_t *iterpool = svn_pool_create (pool);
>
> /* kff todo: use the tag-making functions here, now. */
> const char *log_start
> @@ -1293,27 +1296,45 @@
> a ghost open tag. */
> SVN_ERR (svn_xml_parse (parser, log_start, strlen (log_start), 0));
>
> - /* Parse the log file's contents. */
> - SVN_ERR_W (svn_wc__open_adm_file (&f, svn_wc_adm_access_path (adm_access),
> - SVN_WC__ADM_LOG, APR_READ, pool),
> - _("Couldn't open log"));
> -
> - do {
> - buf_len = sizeof (buf);
> -
> - err = svn_io_file_read (f, buf, &buf_len, pool);
> - if (err && !APR_STATUS_IS_EOF(err->apr_err))
> - return svn_error_createf
> - (err->apr_err, err,
> - _("Error reading administrative log file in '%s'"),
> - svn_wc_adm_access_path (adm_access));
> -
> - SVN_ERR (svn_xml_parse (parser, buf, buf_len, 0));
> -
> - } while (! err);
> -
> - svn_error_clear (err);
> - SVN_ERR (svn_io_file_close (f, pool));
> + for (log_number = 0; ; log_number++)
> + {
> + svn_pool_clear (iterpool);
> + logfile_path = apr_psprintf (iterpool, SVN_WC__ADM_LOG "%s",
> + (log_number == 0) ? ""
> + : apr_psprintf (pool, ".%d", log_number));
> + /* Parse the log file's contents. */
> + err = svn_wc__open_adm_file (&f, svn_wc_adm_access_path (adm_access),
> + logfile_path, APR_READ, iterpool);
> + if (err)
> + {
> + if (APR_STATUS_IS_ENOENT (err->apr_err))
> + {
> + svn_error_clear (err);
> + break;
> + }
> + else
> + {
> + SVN_ERR_W (err, _("Couldn't open log"));
> + }
> + }
> +
> + do {
> + buf_len = sizeof (buf);
> +
> + err = svn_io_file_read (f, buf, &buf_len, iterpool);
> + if (err && !APR_STATUS_IS_EOF(err->apr_err))
> + return svn_error_createf
> + (err->apr_err, err,
> + _("Error reading administrative log file in '%s'"),
> + svn_wc_adm_access_path (adm_access));
> +
> + SVN_ERR (svn_xml_parse (parser, buf, buf_len, 0));

This will leak err if err != NULL and svn_xml_parse returns an error.
Yes, you copied it, but it's still wrong.

> +
> + } while (! err);
> +
> + svn_error_clear (err);
> + SVN_ERR (svn_io_file_close (f, iterpool));
> + }
>
>
> /* Pacify Expat with a pointless closing element tag. */
> @@ -1336,9 +1357,18 @@
> }
> else
> {
> - /* No 'killme'? Remove the logfile; its commands have been executed. */
> - SVN_ERR (svn_wc__remove_adm_file (svn_wc_adm_access_path (adm_access),
> - pool, SVN_WC__ADM_LOG, NULL));
> + for (log_number--; log_number >= 0; log_number--)
> + {
> + svn_pool_clear (iterpool);
> + logfile_path = apr_psprintf (iterpool, SVN_WC__ADM_LOG "%s",
> + (log_number == 0) ? ""
> + : apr_psprintf (pool, ".%d",
> + log_number));

I see this bit of code a few times, perhaps factor into something like

             logfile_path = svn_wc__logfile_path (log_number, iterpool);

It would also fix the bug where you used pool rather than iterpool.

> +
> + /* No 'killme'? Remove the logfile; its commands have been executed. */
> + SVN_ERR (svn_wc__remove_adm_file (svn_wc_adm_access_path (adm_access),
> + iterpool, logfile_path, NULL));
> + }
> }
>
> return SVN_NO_ERROR;
>
> Modified: trunk/subversion/libsvn_wc/update_editor.c
> ==============================================================================
> --- trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ trunk/subversion/libsvn_wc/update_editor.c Sun May 23 15:52:47 2004
> @@ -141,6 +141,9 @@
> /* The bump information for this directory. */
> struct bump_dir_info *bump_info;
>
> + /* The current log file number. */
> + int log_number;
> +
> /* The pool in which this baton itself is allocated. */
> apr_pool_t *pool;
> };
> @@ -211,6 +214,52 @@
> return entry->url;
> }
>
> +/* An APR pool cleanup handler. This runs the log file for a
> + directory baton. */
> +static apr_status_t
> +cleanup_dir_baton (void *dir_baton)
> +{
> + struct dir_baton *db = dir_baton;
> + svn_error_t *err;
> + apr_status_t apr_err;
> + svn_wc_adm_access_t *adm_access;
> +
> + /* If there are no log files to write, return immediately. */
> + if (db->log_number == 0)
> + return APR_SUCCESS;
> +
> + err = svn_wc_adm_retrieve (&adm_access, db->edit_baton->adm_access,
> + db->path, apr_pool_parent_get (db->pool));
> +
> + if (err)
> + {
> + apr_err = err->apr_err;
> + svn_error_clear (err);
> + return apr_err;
> + }

You could make this shorter by replacing
     if (err)
       {
         ...
       }
with
     if (! err)

> +
> + err = svn_wc__run_log (adm_access, NULL, apr_pool_parent_get (db->pool));
> +
> + if (err)
> + {
> + apr_err = err->apr_err;
> + svn_error_clear (err);
> + return apr_err;
> + }
> +
> + return APR_SUCCESS;
> +}
> +

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org