You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/05/20 19:04:57 UTC

svn commit: r946713 - in /subversion/trunk/subversion/libsvn_wc: adm_files.c adm_files.h update_editor.c

Author: julianfoad
Date: Thu May 20 17:04:56 2010
New Revision: 946713

URL: http://svn.apache.org/viewvc?rev=946713&view=rev
Log:
Move a function to where it can be shared, and give it a better name.

* subversion/libsvn_wc/adm_files.h,
  subversion/libsvn_wc/adm_files.c
  (svn_wc__get_ultimate_base_md5_checksum): New function, based on
    get_pristine_base_checksum().

* subversion/libsvn_wc/update_editor.c
  (get_pristine_base_checksum): Delete.
  (apply_textdelta): Use svn_wc__get_ultimate_base_md5_checksum() instead,
    and rename the checksum variables for clarity.

Modified:
    subversion/trunk/subversion/libsvn_wc/adm_files.c
    subversion/trunk/subversion/libsvn_wc/adm_files.h
    subversion/trunk/subversion/libsvn_wc/update_editor.c

Modified: subversion/trunk/subversion/libsvn_wc/adm_files.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_files.c?rev=946713&r1=946712&r2=946713&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_files.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_files.c Thu May 20 17:04:56 2010
@@ -493,6 +493,34 @@ svn_wc__get_pristine_contents(svn_stream
 
 
 svn_error_t *
+svn_wc__get_ultimate_base_md5_checksum(const svn_checksum_t **md5_checksum,
+                                       svn_wc__db_t *db,
+                                       const char *local_abspath,
+                                       apr_pool_t *result_pool,
+                                       apr_pool_t *scratch_pool)
+{
+  svn_error_t *err;
+
+  err = svn_wc__db_base_get_info(NULL, NULL, NULL, NULL, NULL, NULL,
+                                 NULL, NULL, NULL, NULL, NULL, md5_checksum,
+                                 NULL, NULL, NULL,
+                                 db, local_abspath,
+                                 scratch_pool, scratch_pool);
+  if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
+    {
+      svn_error_clear(err);
+      *md5_checksum = NULL;
+      return SVN_NO_ERROR;
+    }
+  if (*md5_checksum && (*md5_checksum)->kind != svn_checksum_md5)
+    SVN_ERR(svn_wc__db_pristine_get_md5(md5_checksum, db, local_abspath,
+                                        *md5_checksum,
+                                        scratch_pool, scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
 svn_wc__prop_path(const char **prop_path,
                   const char *path,
                   svn_wc__db_kind_t node_kind,

Modified: subversion/trunk/subversion/libsvn_wc/adm_files.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_files.h?rev=946713&r1=946712&r2=946713&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_files.h (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_files.h Thu May 20 17:04:56 2010
@@ -193,6 +193,16 @@ svn_wc__ultimate_base_text_path_to_read(
                                         apr_pool_t *result_pool,
                                         apr_pool_t *scratch_pool);
 
+/* Set *MD5_CHECKSUM to the MD-5 checksum of the BASE_NODE pristine text
+ * of LOCAL_ABSPATH in DB, or to NULL if it has no BASE_NODE.
+ * Allocate *MD5_CHECKSUM in RESULT_POOL. */
+svn_error_t *
+svn_wc__get_ultimate_base_md5_checksum(const svn_checksum_t **md5_checksum,
+                                       svn_wc__db_t *db,
+                                       const char *local_abspath,
+                                       apr_pool_t *result_pool,
+                                       apr_pool_t *scratch_pool);
+
 
 
 /*** Opening all kinds of adm files ***/

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=946713&r1=946712&r2=946713&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Thu May 20 17:04:56 2010
@@ -4017,46 +4017,10 @@ open_file(const char *path,
   return SVN_NO_ERROR;
 }
 
-/* Set *MD5_CHECKSUM to the MD-5 hex digest of the BASE_NODE pristine text
- * of LOCAL_ABSPATH in DB, or to NULL if it has no BASE_NODE or if that
- * checksum is not known.  Allocate *MD5_CHECKSUM in RESULT_POOL.
- * LOCAL_ABSPATH must be a file. */
-static svn_error_t *
-get_pristine_base_checksum(const char **md5_checksum,
-                           svn_wc__db_t *db,
-                           const char *local_abspath,
-                           apr_pool_t *result_pool,
-                           apr_pool_t *scratch_pool)
-{
-  svn_error_t *err;
-  const svn_checksum_t *checksum;
-
-  err = svn_wc__db_base_get_info(NULL, NULL, NULL, NULL, NULL, NULL,
-                                 NULL, NULL, NULL, NULL, NULL, &checksum,
-                                 NULL, NULL, NULL,
-                                 db, local_abspath,
-                                 scratch_pool, scratch_pool);
-  if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
-    {
-      svn_error_clear(err);
-      *md5_checksum = NULL;
-      return SVN_NO_ERROR;
-    }
-  /* ### SVN_EXPERIMENTAL_PRISTINE: may need to convert SHA-1 to MD-5. */
-  if (checksum && checksum->kind != svn_checksum_md5)
-    SVN_ERR(svn_wc__db_pristine_get_md5(&checksum, db, local_abspath,
-                                        checksum, scratch_pool, scratch_pool));
-
-  *md5_checksum = checksum ? svn_checksum_to_cstring(checksum, result_pool)
-                           : NULL;
-  return SVN_NO_ERROR;
-}
-
-
 /* An svn_delta_editor_t function. */
 static svn_error_t *
 apply_textdelta(void *file_baton,
-                const char *base_checksum,
+                const char *expected_base_checksum,
                 apr_pool_t *pool,
                 svn_txdelta_window_handler_t *handler,
                 void **handler_baton)
@@ -4065,7 +4029,7 @@ apply_textdelta(void *file_baton,
   apr_pool_t *handler_pool = svn_pool_create(fb->pool);
   struct handler_baton *hb = apr_pcalloc(handler_pool, sizeof(*hb));
   svn_error_t *err;
-  const char *checksum;
+  const char *recorded_base_checksum;
   svn_stream_t *source;
   svn_stream_t *target;
 
@@ -4082,23 +4046,28 @@ apply_textdelta(void *file_baton,
      text base hasn't been corrupted, and that its checksum
      matches the expected base checksum. */
 
-  /* The incoming delta is targeted against BASE_CHECKSUM. Make sure that
-     it matches our recorded checksum.  (In WC-1, we could not do this test
+  /* The incoming delta is targeted against EXPECTED_BASE_CHECKSUM. Find and
+     check our RECORDED_BASE_CHECKSUM.  (In WC-1, we could not do this test
      for replaced nodes because we didn't store the checksum of the "revert
      base".  In WC-NG, we do and we can.) */
-  SVN_ERR(get_pristine_base_checksum(&checksum,
-                                     fb->edit_baton->db, fb->local_abspath,
-                                     fb->pool, pool));
-  if (checksum && base_checksum
-      && strcmp(base_checksum, checksum) != 0)
-    {
+  {
+    const svn_checksum_t *checksum;
+
+    SVN_ERR(svn_wc__get_ultimate_base_md5_checksum(&checksum,
+                                                   fb->edit_baton->db,
+                                                   fb->local_abspath,
+                                                   pool, pool));
+    recorded_base_checksum
+      = checksum ? svn_checksum_to_cstring(checksum, pool) : NULL;
+    if (recorded_base_checksum && expected_base_checksum
+        && strcmp(expected_base_checksum, recorded_base_checksum) != 0)
       return svn_error_createf(SVN_ERR_WC_CORRUPT_TEXT_BASE, NULL,
                      _("Checksum mismatch for '%s':\n"
                        "   expected:  %s\n"
                        "   recorded:  %s\n"),
                      svn_dirent_local_style(fb->local_abspath, pool),
-                     base_checksum, checksum);
-    }
+                     expected_base_checksum, recorded_base_checksum);
+  }
 
   /* Open the text base for reading, unless this is an added file. */
 
@@ -4132,15 +4101,15 @@ apply_textdelta(void *file_baton,
         source = svn_stream_empty(handler_pool);
     }
 
-  /* If we don't have a local checksum, use the ra provided checksum */
-  if (!checksum)
-    checksum = base_checksum;
+  /* If we don't have a recorded checksum, use the ra provided checksum */
+  if (!recorded_base_checksum)
+    recorded_base_checksum = expected_base_checksum;
 
   /* Checksum the text base while applying deltas */
-  if (checksum)
+  if (recorded_base_checksum)
     {
       SVN_ERR(svn_checksum_parse_hex(&hb->expected_source_md5_checksum,
-                                     svn_checksum_md5, checksum,
+                                     svn_checksum_md5, recorded_base_checksum,
                                      handler_pool));
 
       /* Wrap stream and store reference to allow calculating the md5 */



Re: svn commit: r946713 - in /subversion/trunk/subversion/libsvn_wc: adm_files.c adm_files.h update_editor.c

Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-05-20, Greg Stein wrote:
> On Thu, May 20, 2010 at 13:04,  <ju...@apache.org> wrote:
> > Author: julianfoad
> > Date: Thu May 20 17:04:56 2010
> > New Revision: 946713
> >
> > URL: http://svn.apache.org/viewvc?rev=946713&view=rev
> > Log:
> > Move a function to where it can be shared, and give it a better name.
> >
> > * subversion/libsvn_wc/adm_files.h,
> >  subversion/libsvn_wc/adm_files.c
> >  (svn_wc__get_ultimate_base_md5_checksum): New function, based on
> 
> More naming terminology? When I see this, it makes me ask, "What does
> "ultimate" mean?" The docstring doesn't make it clear why this new
> term is present.
> 
> The underlying db function is svn_wc__db_base_get_info(), so why
> wouldn't this simply be svn_wc__base_get_md5_checksum() ?

Because I'm working on a WC-1 legacy code base in which the term "text
base" usually means something like

  "the copied base if the file is replaced with a copy,
   else nothing if replaced by a simple add,
   else the - uh, how shall we say it? - "ultimate" base, 

therefore I can't simply use the word "base" and expect that to be
clear.  I'm trying to change the names in two steps, from the old
(inconsistent and unclear) names, through some intermediate names of
which this is one, then to some final perfect naming.

I'll post separately about naming.

- Julian


Re: svn commit: r946713 - in /subversion/trunk/subversion/libsvn_wc: adm_files.c adm_files.h update_editor.c

Posted by Greg Stein <gs...@gmail.com>.
On Thu, May 20, 2010 at 13:04,  <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Thu May 20 17:04:56 2010
> New Revision: 946713
>
> URL: http://svn.apache.org/viewvc?rev=946713&view=rev
> Log:
> Move a function to where it can be shared, and give it a better name.
>
> * subversion/libsvn_wc/adm_files.h,
>  subversion/libsvn_wc/adm_files.c
>  (svn_wc__get_ultimate_base_md5_checksum): New function, based on

More naming terminology? When I see this, it makes me ask, "What does
"ultimate" mean?" The docstring doesn't make it clear why this new
term is present.

The underlying db function is svn_wc__db_base_get_info(), so why
wouldn't this simply be svn_wc__base_get_md5_checksum() ?

>...

Cheers,
-g