You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/07/06 21:54:25 UTC
svn commit: r1358385 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Author: stefan2
Date: Fri Jul 6 19:54:25 2012
New Revision: 1358385
URL: http://svn.apache.org/viewvc?rev=1358385&view=rev
Log:
FSFS refactoring. Replace the while-macro code for recoverable
file errors with proper functions.
* subversion/libsvn_fs_fs/fs_fs.c
(RETRY_RECOVERABLE, IGNORE_RECOVERABLE, RECOVERABLE_RETRY_TEST,
RECOVERABLE_RETRY_NEXT, CURRENT_BUF_LEN): drop
(RECOVERABLE_RETRY_COUNT): re-introduce; adapt commentary
(try_stringbuf_from_file, read_content): new functions
(read_current): replace by read_content
(get_youngest, get_next_revision_ids): adapt callers
(revision_proplist, get_and_increment_txn_key_body): simplify
by using the new functions
Modified:
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1358385&r1=1358384&r2=1358385&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jul 6 19:54:25 2012
@@ -1407,7 +1407,7 @@ svn_fs_fs__upgrade(svn_fs_t *fs, apr_poo
}
-/* SVN_ERR-like macros for dealing with recoverable errors on mutable files
+/* Functions for dealing with recoverable errors on mutable files
*
* Revprops, current, and txn-current files are mutable; that is, they
* change as part of normal fsfs operation, in constrat to revs files, or
@@ -1440,98 +1440,75 @@ svn_fs_fs__upgrade(svn_fs_t *fs, apr_poo
*
** Solution
*
- * Wrap opens and reads of such files with RETRY_RECOVERABLE and
- * closes with IGNORE_RECOVERABLE. Call these macros within a loop of
- * RECOVERABLE_RETRY_COUNT iterations (though, realistically, the
- * second try will succeed). Make sure you put a break statement
- * after the close, at the end of your loop. Immediately after your
- * loop, return err if err.
+ * Try open and read of such files in try_stringbuf_from_file(). Call
+ * this function within a loop of RECOVERABLE_RETRY_COUNT iterations
+ * (though, realistically, the second try will succeed).
+ */
+
+#define RECOVERABLE_RETRY_COUNT 10
+
+/* Read the file at PATH and return its content in *CONTENT. *CONTENT will
+ * not be modified unless the whole file was read successfully.
*
- * You must initialize err to SVN_NO_ERROR and filehandle to NULL, as
- * these macros do not.
+ * ESTALE, EIO and ENOENT will not cause this function to return an error
+ * unless LAST_ATTEMPT has been set. If MISSING is not NULL, indicate
+ * missing files (ENOENT) there.
+ *
+ * Use POOL for allocations.
*/
+static svn_error_t *
+try_stringbuf_from_file(svn_stringbuf_t **content,
+ svn_boolean_t *missing,
+ const char *path,
+ svn_boolean_t last_attempt,
+ apr_pool_t *pool)
+{
+ svn_error_t *err = svn_stringbuf_from_file2(content, path, pool);
+ if (missing)
+ *missing = FALSE;
+ if (err)
+ {
+ if (APR_STATUS_IS_ENOENT(err->apr_err))
+ {
+ if (!last_attempt)
+ {
+ svn_error_clear(err);
+ if (missing)
+ *missing = TRUE;
+ return SVN_NO_ERROR;
+ }
+ }
#ifdef ESTALE
-/* Do not use do-while due to the embedded 'continue'. */
-#define RETRY_RECOVERABLE(err, filehandle, expr) \
- if (1) { \
- svn_error_clear(err); \
- err = (expr); \
- if (err) \
- { \
- apr_status_t _e = APR_TO_OS_ERROR(err->apr_err); \
- if ((_e == ESTALE) || (_e == EIO) || (_e == ENOENT)) { \
- if (NULL != filehandle) \
- (void)apr_file_close(filehandle); \
- continue; \
- } \
- return svn_error_trace(err); \
- } \
- } else
-#define IGNORE_RECOVERABLE(err, expr) \
- if (1) { \
- svn_error_clear(err); \
- err = (expr); \
- if (err) \
- { \
- apr_status_t _e = APR_TO_OS_ERROR(err->apr_err); \
- if ((_e != ESTALE) && (_e != EIO)) \
- return svn_error_trace(err); \
- } \
- } else
-#define RECOVERABLE_RETRY_TEST \
- i < 10
-#define RECOVERABLE_RETRY_NEXT \
- i++
-#else
-#define RETRY_RECOVERABLE(err, filehandle, expr) SVN_ERR(expr)
-#define IGNORE_RECOVERABLE(err, expr) SVN_ERR(expr)
-#define RECOVERABLE_RETRY_TEST
-#define RECOVERABLE_RETRY_NEXT
+ else if (APR_TO_OS_ERROR(err->apr_err) == ESTALE
+ || APR_TO_OS_ERROR(err->apr_err) == EIO)
+ {
+ if (!last_attempt)
+ {
+ svn_error_clear(err);
+ return SVN_NO_ERROR;
+ }
+ }
+ }
#endif
-/* Long enough to hold: "<svn_revnum_t> <node id> <copy id>\0"
- * 19 bytes for svn_revnum_t (room for 32 or 64 bit values)
- * + 2 spaces
- * + 26 bytes for each id (these are actually unbounded, so we just
- * have to pick something; 2^64 is 13 bytes in base-36)
- * + 1 terminating null
- */
-#define CURRENT_BUF_LEN 48
+ return svn_error_trace(err);
+}
/* Read the 'current' file FNAME and store the contents in *BUF.
Allocations are performed in POOL. */
static svn_error_t *
-read_current(const char *fname, char **buf, apr_pool_t *pool)
+read_content(svn_stringbuf_t **content, const char *fname, apr_pool_t *pool)
{
- apr_file_t *revision_file = NULL;
- apr_size_t len;
int i;
- svn_error_t *err = SVN_NO_ERROR;
- apr_pool_t *iterpool;
+ *content = NULL;
- *buf = apr_palloc(pool, CURRENT_BUF_LEN);
- iterpool = svn_pool_create(pool);
- for (i = 0; RECOVERABLE_RETRY_TEST; RECOVERABLE_RETRY_NEXT)
- {
- svn_pool_clear(iterpool);
-
- RETRY_RECOVERABLE(err, revision_file,
- svn_io_file_open(&revision_file, fname,
- APR_READ | APR_BUFFERED,
- APR_OS_DEFAULT, iterpool));
-
- len = CURRENT_BUF_LEN;
- RETRY_RECOVERABLE(err, revision_file,
- svn_io_read_length_line(revision_file,
- *buf, &len, iterpool));
- IGNORE_RECOVERABLE(err, svn_io_file_close(revision_file, iterpool));
-
- break;
- }
- svn_pool_destroy(iterpool);
+ for (i = 0; !*content && (i < RECOVERABLE_RETRY_COUNT); ++i)
+ SVN_ERR(try_stringbuf_from_file(content, NULL,
+ fname, i + 1 < RECOVERABLE_RETRY_COUNT,
+ pool));
- return svn_error_trace(err);
+ return SVN_NO_ERROR;
}
/* Find the youngest revision in a repository at path FS_PATH and
@@ -1542,12 +1519,11 @@ get_youngest(svn_revnum_t *youngest_p,
const char *fs_path,
apr_pool_t *pool)
{
- char *buf;
-
- SVN_ERR(read_current(svn_dirent_join(fs_path, PATH_CURRENT, pool),
- &buf, pool));
+ svn_stringbuf_t *buf;
+ SVN_ERR(read_content(&buf, svn_dirent_join(fs_path, PATH_CURRENT, pool),
+ pool));
- *youngest_p = SVN_STR_TO_REV(buf);
+ *youngest_p = SVN_STR_TO_REV(buf->data);
return SVN_NO_ERROR;
}
@@ -3231,55 +3207,31 @@ revision_proplist(apr_hash_t **proplist_
/* if (1); null condition for easier merging to revprop-packing */
{
- apr_file_t *revprop_file = NULL;
- svn_error_t *err = SVN_NO_ERROR;
+ svn_stringbuf_t *content = NULL;
+ svn_boolean_t missing = FALSE;
int i;
- apr_pool_t *iterpool;
+ apr_pool_t *iterpool = svn_pool_create(pool);
- proplist = apr_hash_make(pool);
- iterpool = svn_pool_create(pool);
- for (i = 0; RECOVERABLE_RETRY_TEST; RECOVERABLE_RETRY_NEXT)
+ /* Don't retry if the file is missing, i.e. the rev doesn't exist. */
+ for (i = 0; i < RECOVERABLE_RETRY_COUNT && !missing && !content; ++i)
{
svn_pool_clear(iterpool);
+ SVN_ERR(try_stringbuf_from_file(&content,
+ &missing,
+ path_revprops(fs, rev, iterpool),
+ i+1 < RECOVERABLE_RETRY_COUNT,
+ iterpool));
+ }
- /* Clear err here rather than after finding a recoverable error so
- * we can return that error on the last iteration of the loop. */
- svn_error_clear(err);
- err = svn_io_file_open(&revprop_file, path_revprops(fs, rev,
- iterpool),
- APR_READ | APR_BUFFERED, APR_OS_DEFAULT,
- iterpool);
- if (err)
- {
- if (APR_STATUS_IS_ENOENT(err->apr_err))
- {
- svn_error_clear(err);
- return svn_error_createf(SVN_ERR_FS_NO_SUCH_REVISION, NULL,
- _("No such revision %ld"), rev);
- }
-#ifdef ESTALE
- else if (APR_TO_OS_ERROR(err->apr_err) == ESTALE
- || APR_TO_OS_ERROR(err->apr_err) == EIO
- || APR_TO_OS_ERROR(err->apr_err) == ENOENT)
- continue;
-#endif
- return svn_error_trace(err);
- }
-
- SVN_ERR(svn_hash__clear(proplist, iterpool));
- RETRY_RECOVERABLE(err, revprop_file,
- svn_hash_read2(proplist,
- svn_stream_from_aprfile2(
- revprop_file, TRUE, iterpool),
- SVN_HASH_TERMINATOR, pool));
-
- IGNORE_RECOVERABLE(err, svn_io_file_close(revprop_file, iterpool));
+ if (missing)
+ return svn_error_createf(SVN_ERR_FS_NO_SUCH_REVISION, NULL,
+ _("No such revision %ld"), rev);
- break;
- }
+ proplist = apr_hash_make(pool);
+ SVN_ERR(svn_hash_read2(proplist,
+ svn_stream_from_stringbuf(content, iterpool),
+ SVN_HASH_TERMINATOR, pool));
- if (err)
- return svn_error_trace(err);
svn_pool_destroy(iterpool);
}
@@ -5065,37 +5017,16 @@ get_and_increment_txn_key_body(void *bat
apr_file_t *txn_current_file = NULL;
const char *tmp_filename;
char next_txn_id[MAX_KEY_SIZE+3];
- svn_error_t *err = SVN_NO_ERROR;
- apr_pool_t *iterpool;
apr_size_t len;
int i;
- cb->txn_id = apr_palloc(cb->pool, MAX_KEY_SIZE);
-
- iterpool = svn_pool_create(pool);
- for (i = 0; RECOVERABLE_RETRY_TEST; RECOVERABLE_RETRY_NEXT)
- {
- svn_pool_clear(iterpool);
-
- RETRY_RECOVERABLE(err, txn_current_file,
- svn_io_file_open(&txn_current_file,
- txn_current_filename,
- APR_READ | APR_BUFFERED,
- APR_OS_DEFAULT, iterpool));
- len = MAX_KEY_SIZE;
- RETRY_RECOVERABLE(err, txn_current_file,
- svn_io_read_length_line(txn_current_file,
- cb->txn_id,
- &len,
- iterpool));
- IGNORE_RECOVERABLE(err, svn_io_file_close(txn_current_file,
- iterpool));
-
- break;
- }
- SVN_ERR(err);
+ svn_stringbuf_t *buf;
+ SVN_ERR(read_content(&buf, txn_current_filename, pool));
- svn_pool_destroy(iterpool);
+ /* remove trailing newlines */
+ svn_stringbuf_strip_whitespace(buf);
+ cb->txn_id = buf->data;
+ len = buf->len;
/* Increment the key and add a trailing \n to the string so the
txn-current file has a newline in it. */
@@ -6183,8 +6114,10 @@ get_next_revision_ids(const char **node_
{
char *buf;
char *str;
+ svn_stringbuf_t *content;
- SVN_ERR(read_current(svn_fs_fs__path_current(fs, pool), &buf, pool));
+ SVN_ERR(read_content(&content, svn_fs_fs__path_current(fs, pool), pool));
+ buf = content->data;
str = svn_cstring_tokenize(" ", &buf);
if (! str)