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

Re: r1673875: Provide a central and complete implementation for handling FSFS quirks...

From: Julian Foad <julianfoad_at_gmail.com>
Date: Sat, 18 Apr 2015 12:35:54 +0100

Stefan Fuhrmann wrote:
>> @@ -1386,11 +1386,11 @@ svn_fs_fs__file_length(svn_filesize_t *l
[...]
>> - A plain representation may specify its EXPANDED LENGTH as "0"
>> + A plain representation may specify its EXPANDED_SIZE as "0"
[...]
>
> That should be implicitly fixed be r1674400.
> It removed all the local handling for special case.

Thanks. All of r1674400 looks good to me.

Here are a few more suggestions from further reading of the surrounding code.

[[[
Index: subversion/libsvn_fs_fs/cached_data.c
===================================================================
--- subversion/libsvn_fs_fs/cached_data.c (revision 1674452)
+++ subversion/libsvn_fs_fs/cached_data.c (working copy)
@@ -1725,7 +1725,7 @@ get_combined_window(svn_stringbuf_t **re
 }

 /* Returns whether or not the expanded fulltext of the file is cachable
- * based on its size SIZE. The decision depends on the cache used by RB.
+ * based on its size SIZE. The decision depends on the cache used by FFD.
  */
 static svn_boolean_t
 fulltext_size_is_cachable(fs_fs_data_t *ffd, svn_filesize_t size)
Index: subversion/libsvn_fs_fs/transaction.c
===================================================================
--- subversion/libsvn_fs_fs/transaction.c (revision 1674452)
+++ subversion/libsvn_fs_fs/transaction.c (working copy)
@@ -2116,7 +2116,7 @@ rep_write_get_baton(struct rep_write_bat
 }

 /* For REP->SHA1_CHECKSUM, try to find an already existing representation
- in FS and return it in *OUT_REP. If no such representation exists or
+ in FS and return it in *OLD_REP. If no such representation exists or
    if rep sharing has been disabled for FS, NULL will be returned. Since
    there may be new duplicate representations within the same uncommitted
    revision, those can be passed in REPS_HASH (maps a sha1 digest onto
@@ -2142,7 +2142,7 @@ get_shared_rep(representation_t **old_re

   /* Check and see if we already have a representation somewhere that's
      identical to the one we just wrote out. Start with the hash lookup
- because it is cheepest. */
+ because it is cheapest. */
   if (reps_hash)
     *old_rep = apr_hash_get(reps_hash,
                             rep->sha1_digest,
@@ -2197,6 +2197,8 @@ get_shared_rep(representation_t **old_re
       /* in our txn, is there a rep file named with the wanted SHA1?
          If so, read it and use that rep.
        */
+ /* ### Would it be faster (and so better) to just try reading it,
+ and handle ENOENT, instead of first checking for presence? */
       SVN_ERR(svn_io_check_path(file_name, &kind, scratch_pool));
       if (kind == svn_node_file)
         {
@@ -2214,6 +2216,7 @@ get_shared_rep(representation_t **old_re
   /* A simple guard against general rep-cache induced corruption. */
   if ((*old_rep)->expanded_size != rep->expanded_size)
     {
+ /* ### Shouldn't this be a hard error? At least warn or log it. */
       *old_rep = NULL;
     }
   else
]]]

- Julian
Received on 2015-04-18 13:36:44 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.