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 2013/07/24 15:24:44 UTC
svn commit: r1506545 [1/2] - in
/subversion/branches/fsfs-improvements/subversion: include/ libsvn_fs_fs/
Author: stefan2
Date: Wed Jul 24 13:24:44 2013
New Revision: 1506545
URL: http://svn.apache.org/r1506545
Log:
On the fsfs-improvements branch: Finally switch to a new FSFS ID API.
This replaces the previous string-based API with one that represents
IDs as quadruples of <revision,sub-id> pairs of integers. Thus, it
now matches the implicit usage of the node_id, branch_id, txn_id and
rev_offset parts of those IDs.
The patch does four things. First, it replaces the current API in
id.*. Second, it must use the new svn_fs_fs__id_part_t * instead of
const char * in all FSFS code. In some places we have to add a level
of indirection as key parts can now be put on stack instead of being
pool-allocated but we always pass them along through pointers.
Third, structs using a txn ID will use the new data type as well -
requiring more code to be updated. Lastly, we have to update code
that (de-)serializes IDs or handles ID assignment and increments.
* subversion/include/svn_error_codes.h
(SVN_ERR_FS_MALFORMED_TXN_ID): new error code
* subversion/libsvn_fs_fs/id.h
(svn_fs_fs__id_part_t): declare type for ID elements
(svn_fs_fs__id_txn_parse,
svn_fs_fs__id_rev_offset,
svn_fs_fs__id_part_compare): declare new API functions
(svn_fs_fs__id_part_eq,
svn_fs_fs__id_txn_used,
svn_fs_fs__id_txn_reset,
svn_fs_fs__id_txn_unparse,
svn_fs_fs__id_node_id,
svn_fs_fs__id_copy_id,
svn_fs_fs__id_rev,
svn_fs_fs__id_compare,
svn_fs_fs__id_txn_create): change signatures
* subversion/libsvn_fs_fs/id.c
(id_private_t): replace by ...
(fs_fs__id_t): ... this one; declare it such that we can just cast
between svn_fs_id_t and our internal type
(part_parse,
txn_id_parse): new external string-rep -> internal rep conversion utils
(svn_fs_fs__id_part_is_root):
(svn_fs_fs__id_part_eq,
svn_fs_fs__id_txn_used,
svn_fs_fs__id_txn_reset,
svn_fs_fs__id_txn_unparse): update signatures and code
(svn_fs_fs__id_txn_parse): implement new API function
(svn_fs_fs__id_node_id,
svn_fs_fs__id_copy_id,
svn_fs_fs__id_txn_id,
svn_fs_fs__id_rev,
svn_fs_fs__id_offset,
svn_fs_fs__id_is_txn,
svn_fs_fs__id_eq): update signature and reimplement trivially
(svn_fs_fs__id_part_compare): implement new API function
(svn_fs_fs__id_unparse,
svn_fs_fs__id_check_related,
svn_fs_fs__id_txn_create_root,
svn_fs_fs__id_txn_create,
svn_fs_fs__id_rev_create,
svn_fs_fs__id_copy,
svn_fs_fs__id_parse): update signature and reimplement
(serialize_id_private,
deserialize_id_private): drop
(svn_fs_fs__id_serialize,
svn_fs_fs__id_deserialize): greatly simplify
* subversion/libsvn_fs_fs/fs.h
(fs_fs_shared_txn_data_t,
representation_t): use the new ID part type for TXN IDs
* subversion/libsvn_fs_fs/cached_data.c
(open_and_seek_transaction): add indirection
* subversion/libsvn_fs_fs/dag.h
(svn_fs_fs__dag_txn_root,
svn_fs_fs__dag_clone_root,
svn_fs_fs__dag_set_entry,
svn_fs_fs__dag_clone_child,
svn_fs_fs__dag_delete,
svn_fs_fs__dag_make_dir,
svn_fs_fs__dag_make_file,
svn_fs_fs__dag_copy): use new ID part type in signature
* subversion/libsvn_fs_fs/dag.c
(set_entry,
make_entry,
svn_fs_fs__dag_set_entry,
svn_fs_fs__dag_txn_root,
svn_fs_fs__dag_txn_base_root,
svn_fs_fs__dag_clone_child,
svn_fs_fs__dag_clone_root,
svn_fs_fs__dag_delete,
svn_fs_fs__dag_make_file,
svn_fs_fs__dag_make_dir): use new ID part type in signature
(svn_fs_fs__dag_copy): use new ID part type in signature and code
* subversion/libsvn_fs_fs/fs_fs.h
(svn_fs_fs__set_node_origin,
svn_fs_fs__get_node_origin): use new ID part type in signature
* subversion/libsvn_fs_fs/fs_fs.c
(svn_fs_fs__set_node_origin): use new ID part type in signature
(svn_fs_fs__get_node_origin,
set_node_origins_for_file): ditto; convert string <-> ID part
Note, these are old-style IDs with rev:=0
* subversion/libsvn_fs_fs/hotcopy.c
(hotcopy_update_current): replace the ID part types with ints
Note, these are old-style IDs with rev:=0
* subversion/libsvn_fs_fs/low_level.c
(read_rep_offsets): add indirection
* subversion/libsvn_fs_fs/recovery.h
(svn_fs_fs__find_max_ids): replace the ID part types with ints
Note, these are old-style IDs with rev:=0
* subversion/libsvn_fs_fs/recovery.c
(recover_find_max_ids,
svn_fs_fs__find_max_ids): update / simplify using new ID API
(recover_body): ditto; we don't need the root node id here anymore
* subversion/libsvn_fs_fs/temp_serializer.c
(serialize_representation,
deserialize_representation): no sub-struct handling for id parts needed
* subversion/libsvn_fs_fs/transaction.h
(svn_fs_fs__txn_get_id,
svn_fs_fs__txn_changes_fetch,
svn_fs_fs__get_txn,
svn_fs_fs__reserve_copy_id,
svn_fs_fs__create_node,
svn_fs_fs__set_entry,
svn_fs_fs__add_change,
svn_fs_fs__create_successor,
svn_fs_fs__get_txn_ids): use new ID part type in signature
* subversion/libsvn_fs_fs/transaction.c
(fs_txn_data_t,
unlock_proto_rev_baton,
get_writable_proto_rev_baton,
get_and_increment_txn_key_baton,
path_txn_sha1,
path_txn_changes,
path_txn_props,
path_txn_next_ids,
purge_shared_txn,
svn_fs_fs__txn_changes_fetch,
svn_fs_fs__get_txn,
svn_fs_fs__add_change,
svn_fs_fs__create_successor,
write_final_changed_path_info,
verify_locks,
svn_fs_fs__get_txn_ids,
svn_fs_fs__txn_get_id,
free_shared_txn,
get_txn_proplist,
get_shared_txn,
unlock_proto_rev_body,
get_writable_proto_rev_body,
unlock_proto_rev,
unlock_proto_rev_list_locked,
get_writable_proto_rev,
svn_fs_fs__set_entry,
purge_shared_txn_body,
commit_body,
create_new_txn_noderev_from_rev,
svn_fs_fs__create_txn,
svn_fs_fs__change_txn_props,
get_shared_rep,
rep_write_contents_close,
svn_fs_fs__create_node,
svn_fs_fs__set_proplist,
svn_fs_fs__open_txn): update part ID types, their assignment
and indirection level
(get_and_increment_txn_key_body): trivial increment; update parser & writer
(create_txn_dir,
create_txn_dir_pre_1_5): update signature and txn ID initialization
(write_next_ids): update signature and IDs serialization
(read_next_ids): update signature and IDs parsing
(get_new_txn_node_id,
svn_fs_fs__reserve_copy_id,
write_final_current): update signature, ID init and increment code
(svn_fs_fs__purge_txn): convert svn_fs API parameter into internal ID type,
update parameter indirection
(set_uniquifier): need to convert uniquifier from int -> string
(get_next_revision_ids): update signature and parser
(get_final_id): update signature and switch implementation to ints
(write_final_rev): update signature, local variable type, ID initialization
and parameter indirection
* subversion/libsvn_fs_fs/tree.c
(fs_txn_root_data_t): replace txn id string with proper part ID
(root_txn_id): update signature; update implementation
(get_copy_inheritance,
fs_change_node_prop,
merge_changes,
fs_make_dir,
fs_delete_node,
copy_helper,
fs_make_file,
apply_textdelta,
apply_text): update local variable types
(make_path_mutable): ditto; update assignment
(add_change,
merge): update signature
(fs_node_origin_rev): simplify using the new ID API guarantees
(make_txn_root): update signature and struct member initialization
(svn_fs_fs__verify_root): update parameter indirection
* subversion/libsvn_fs_fs/util.h
(svn_fs_fs__path_txn_dir,
svn_fs_fs__path_txn_proto_rev,
svn_fs_fs__path_txn_proto_rev_lock,
svn_fs_fs__path_node_origin): use new ID part type in signature
svn_fs_fs__write_current): use int counters in signature
* subversion/libsvn_fs_fs/util.c
(combine_txn_id_string): update signature and convert id->string
(svn_fs_fs__path_txn_dir,
svn_fs_fs__path_txn_proto_rev,
svn_fs_fs__path_txn_proto_rev_lock): update signature
(svn_fs_fs__path_txn_node_rev,
svn_fs_fs__path_node_origin,
svn_fs_fs__write_current): update signature; convert int -> base36
Modified:
subversion/branches/fsfs-improvements/subversion/include/svn_error_codes.h
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/cached_data.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.h
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.h
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/hotcopy.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.h
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.h
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/transaction.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/transaction.h
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/tree.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/util.c
subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/util.h
Modified: subversion/branches/fsfs-improvements/subversion/include/svn_error_codes.h
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/include/svn_error_codes.h?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/include/svn_error_codes.h (original)
+++ subversion/branches/fsfs-improvements/subversion/include/svn_error_codes.h Wed Jul 24 13:24:44 2013
@@ -806,6 +806,11 @@ SVN_ERROR_START
SVN_ERR_FS_CATEGORY_START + 52,
"Could not initialize the revprop caching infrastructure.")
+ /** @since New in 1.9. */
+ SVN_ERRDEF(SVN_ERR_FS_MALFORMED_TXN_ID,
+ SVN_ERR_FS_CATEGORY_START + 53,
+ "Malformed transaction ID string.")
+
/* repos errors */
SVN_ERRDEF(SVN_ERR_REPOS_LOCKED,
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/cached_data.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/cached_data.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/cached_data.c Wed Jul 24 13:24:44 2013
@@ -202,7 +202,7 @@ open_and_seek_transaction(apr_file_t **f
apr_file_t *rev_file;
SVN_ERR(svn_io_file_open(&rev_file,
- svn_fs_fs__path_txn_proto_rev(fs, rep->txn_id,
+ svn_fs_fs__path_txn_proto_rev(fs, &rep->txn_id,
pool),
APR_READ | APR_BUFFERED, APR_OS_DEFAULT, pool));
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.c Wed Jul 24 13:24:44 2013
@@ -31,7 +31,6 @@
#include "cached_data.h"
#include "dag.h"
#include "fs.h"
-#include "key-gen.h"
#include "fs_fs.h"
#include "id.h"
#include "transaction.h"
@@ -334,7 +333,7 @@ set_entry(dag_node_t *parent,
const char *name,
const svn_fs_id_t *id,
svn_node_kind_t kind,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
node_revision_t *parent_noderev;
@@ -361,7 +360,7 @@ make_entry(dag_node_t **child_p,
const char *parent_path,
const char *name,
svn_boolean_t is_dir,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
const svn_fs_id_t *new_node_id;
@@ -454,7 +453,7 @@ svn_fs_fs__dag_set_entry(dag_node_t *nod
const char *entry_name,
const svn_fs_id_t *id,
svn_node_kind_t kind,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
/* Check it's a directory. */
@@ -618,7 +617,7 @@ svn_fs_fs__dag_revision_root(dag_node_t
svn_error_t *
svn_fs_fs__dag_txn_root(dag_node_t **node_p,
svn_fs_t *fs,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
const svn_fs_id_t *root_id, *ignored;
@@ -631,7 +630,7 @@ svn_fs_fs__dag_txn_root(dag_node_t **nod
svn_error_t *
svn_fs_fs__dag_txn_base_root(dag_node_t **node_p,
svn_fs_t *fs,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
const svn_fs_id_t *base_root_id, *ignored;
@@ -646,8 +645,8 @@ svn_fs_fs__dag_clone_child(dag_node_t **
dag_node_t *parent,
const char *parent_path,
const char *name,
- const char *copy_id,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *copy_id,
+ const svn_fs_fs__id_part_t *txn_id,
svn_boolean_t is_parent_copyroot,
apr_pool_t *pool)
{
@@ -720,7 +719,7 @@ svn_fs_fs__dag_clone_child(dag_node_t **
svn_error_t *
svn_fs_fs__dag_clone_root(dag_node_t **root_p,
svn_fs_t *fs,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
const svn_fs_id_t *base_root_id, *root_id;
@@ -747,7 +746,7 @@ svn_fs_fs__dag_clone_root(dag_node_t **r
svn_error_t *
svn_fs_fs__dag_delete(dag_node_t *parent,
const char *name,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
node_revision_t *parent_noderev;
@@ -871,7 +870,7 @@ svn_fs_fs__dag_make_file(dag_node_t **ch
dag_node_t *parent,
const char *parent_path,
const char *name,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
/* Call our little helper function */
@@ -884,7 +883,7 @@ svn_fs_fs__dag_make_dir(dag_node_t **chi
dag_node_t *parent,
const char *parent_path,
const char *name,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
/* Call our little helper function */
@@ -1189,7 +1188,7 @@ svn_fs_fs__dag_copy(dag_node_t *to_node,
svn_boolean_t preserve_history,
svn_revnum_t from_rev,
const char *from_path,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
const svn_fs_id_t *id;
@@ -1197,7 +1196,7 @@ svn_fs_fs__dag_copy(dag_node_t *to_node,
if (preserve_history)
{
node_revision_t *from_noderev, *to_noderev;
- const char *copy_id;
+ svn_fs_fs__id_part_t copy_id;
const svn_fs_id_t *src_id = svn_fs_fs__dag_get_id(from_node);
svn_fs_t *fs = svn_fs_fs__dag_get_fs(from_node);
@@ -1223,7 +1222,7 @@ svn_fs_fs__dag_copy(dag_node_t *to_node,
to_noderev->copyroot_path = NULL;
SVN_ERR(svn_fs_fs__create_successor(&id, fs, src_id, to_noderev,
- copy_id, txn_id, pool));
+ ©_id, txn_id, pool));
}
else /* don't preserve history */
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.h
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.h?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.h (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/dag.h Wed Jul 24 13:24:44 2013
@@ -27,6 +27,8 @@
#include "svn_delta.h"
#include "private/svn_cache.h"
+#include "id.h"
+
#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */
@@ -224,7 +226,7 @@ svn_error_t *svn_fs_fs__dag_revision_roo
for a transaction, call svn_fs_fs__dag_clone_root. */
svn_error_t *svn_fs_fs__dag_txn_root(dag_node_t **node_p,
svn_fs_t *fs,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
@@ -232,7 +234,7 @@ svn_error_t *svn_fs_fs__dag_txn_root(dag
allocating from POOL. Allocate the node in TRAIL->pool. */
svn_error_t *svn_fs_fs__dag_txn_base_root(dag_node_t **node_p,
svn_fs_t *fs,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
@@ -242,7 +244,7 @@ svn_error_t *svn_fs_fs__dag_txn_base_roo
root directory clone. Allocate *ROOT_P in POOL. */
svn_error_t *svn_fs_fs__dag_clone_root(dag_node_t **root_p,
svn_fs_t *fs,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
@@ -294,7 +296,7 @@ svn_error_t *svn_fs_fs__dag_set_entry(da
const char *entry_name,
const svn_fs_id_t *id,
svn_node_kind_t kind,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
@@ -321,8 +323,8 @@ svn_error_t *svn_fs_fs__dag_clone_child(
dag_node_t *parent,
const char *parent_path,
const char *name,
- const char *copy_id,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *copy_id,
+ const svn_fs_fs__id_part_t *txn_id,
svn_boolean_t is_parent_copyroot,
apr_pool_t *pool);
@@ -341,7 +343,7 @@ svn_error_t *svn_fs_fs__dag_clone_child(
*/
svn_error_t *svn_fs_fs__dag_delete(dag_node_t *parent,
const char *name,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
@@ -384,7 +386,7 @@ svn_error_t *svn_fs_fs__dag_make_dir(dag
dag_node_t *parent,
const char *parent_path,
const char *name,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
@@ -495,7 +497,7 @@ svn_error_t *svn_fs_fs__dag_make_file(da
dag_node_t *parent,
const char *parent_path,
const char *name,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
@@ -522,7 +524,7 @@ svn_error_t *svn_fs_fs__dag_copy(dag_nod
svn_boolean_t preserve_history,
svn_revnum_t from_rev,
const char *from_path,
- const char *txn_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs.h Wed Jul 24 13:24:44 2013
@@ -159,13 +159,8 @@ typedef struct fs_fs_shared_txn_data_t
transaction. */
struct fs_fs_shared_txn_data_t *next;
- /* This transaction's ID. For repositories whose format is less
- than SVN_FS_FS__MIN_TXN_CURRENT_FORMAT, the ID is in the form
- <rev>-<uniqueifier>, where <uniqueifier> runs from 0-99999 (see
- create_txn_dir_pre_1_5() in fs_fs.c). For newer repositories,
- the form is <rev>-<200 digit base 36 number> (see
- create_txn_dir() in fs_fs.c). */
- char txn_id[SVN_FS__TXN_MAX_LEN+1];
+ /* ID of this transaction. */
+ svn_fs_fs__id_part_t txn_id;
/* Whether the transaction's prototype revision file is locked for
writing by any thread in this process (including the current
@@ -431,7 +426,7 @@ typedef struct representation_t
svn_filesize_t expanded_size;
/* Is this representation a transaction? */
- const char *txn_id;
+ svn_fs_fs__id_part_t txn_id;
/* For rep-sharing, we need a way of uniquifying node-revs which share the
same representation (see svn_fs_fs__noderev_same_rep_key() ). So, we
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.c Wed Jul 24 13:24:44 2013
@@ -55,7 +55,6 @@
#include "fs.h"
#include "tree.h"
#include "lock.h"
-#include "key-gen.h"
#include "fs_fs.h"
#include "id.h"
#include "low_level.h"
@@ -1202,7 +1201,7 @@ get_node_origins_from_file(svn_fs_t *fs,
svn_error_t *
svn_fs_fs__get_node_origin(const svn_fs_id_t **origin_id,
svn_fs_t *fs,
- const char *node_id,
+ const svn_fs_fs__id_part_t *node_id,
apr_pool_t *pool)
{
apr_hash_t *node_origins;
@@ -1214,8 +1213,8 @@ svn_fs_fs__get_node_origin(const svn_fs_
pool));
if (node_origins)
{
- const char *node_id_ptr = node_id;
- apr_size_t len = strlen(node_id_ptr);
+ char node_id_ptr[SVN_INT64_BUFFER_SIZE];
+ apr_size_t len = svn__ui64tobase36(node_id_ptr, node_id->number);
svn_string_t *origin_id_str
= apr_hash_get(node_origins, node_id_ptr, len);
@@ -1232,7 +1231,7 @@ svn_fs_fs__get_node_origin(const svn_fs_
static svn_error_t *
set_node_origins_for_file(svn_fs_t *fs,
const char *node_origins_path,
- const char *node_id,
+ const svn_fs_fs__id_part_t *node_id,
svn_string_t *node_rev_id,
apr_pool_t *pool)
{
@@ -1241,8 +1240,9 @@ set_node_origins_for_file(svn_fs_t *fs,
apr_hash_t *origins_hash;
svn_string_t *old_node_rev_id;
- const char *node_id_ptr = node_id;
- apr_size_t len = strlen(node_id_ptr);
+ /* the hash serialization functions require strings as keys */
+ char node_id_ptr[SVN_INT64_BUFFER_SIZE];
+ apr_size_t len = svn__ui64tobase36(node_id_ptr, node_id->number);
SVN_ERR(svn_fs_fs__ensure_dir_exists(svn_dirent_join(fs->path,
PATH_NODE_ORIGINS_DIR,
@@ -1289,7 +1289,7 @@ set_node_origins_for_file(svn_fs_t *fs,
svn_error_t *
svn_fs_fs__set_node_origin(svn_fs_t *fs,
- const char *node_id,
+ const svn_fs_fs__id_part_t *node_id,
const svn_fs_id_t *node_rev_id,
apr_pool_t *pool)
{
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.h
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.h?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.h (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/fs_fs.h Wed Jul 24 13:24:44 2013
@@ -168,7 +168,7 @@ svn_error_t *svn_fs_fs__ensure_dir_exist
*/
svn_error_t *
svn_fs_fs__set_node_origin(svn_fs_t *fs,
- const char *node_id,
+ const svn_fs_fs__id_part_t *node_id,
const svn_fs_id_t *node_rev_id,
apr_pool_t *pool);
@@ -182,7 +182,7 @@ svn_fs_fs__set_node_origin(svn_fs_t *fs,
svn_error_t *
svn_fs_fs__get_node_origin(const svn_fs_id_t **origin_id,
svn_fs_t *fs,
- const char *node_id,
+ const svn_fs_fs__id_part_t *node_id,
apr_pool_t *pool);
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/hotcopy.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/hotcopy.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/hotcopy.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/hotcopy.c Wed Jul 24 13:24:44 2013
@@ -25,7 +25,6 @@
#include "fs_fs.h"
#include "hotcopy.h"
-#include "key-gen.h"
#include "util.h"
#include "recovery.h"
#include "revprops.h"
@@ -357,8 +356,8 @@ hotcopy_update_current(svn_revnum_t *dst
svn_revnum_t new_youngest,
apr_pool_t *scratch_pool)
{
- char next_node_id[MAX_KEY_SIZE] = "0";
- char next_copy_id[MAX_KEY_SIZE] = "0";
+ apr_uint64_t next_node_id = 0;
+ apr_uint64_t next_copy_id = 0;
fs_fs_data_t *dst_ffd = dst_fs->fsap_data;
if (*dst_youngest >= new_youngest)
@@ -367,7 +366,7 @@ hotcopy_update_current(svn_revnum_t *dst
/* If necessary, get new current next_node and next_copy IDs. */
if (dst_ffd->format < SVN_FS_FS__MIN_NO_GLOBAL_IDS_FORMAT)
SVN_ERR(svn_fs_fs__find_max_ids(dst_fs, new_youngest,
- next_node_id, next_copy_id,
+ &next_node_id, &next_copy_id,
scratch_pool));
/* Update 'current'. */
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.c Wed Jul 24 13:24:44 2013
@@ -24,134 +24,253 @@
#include <stdlib.h>
#include "id.h"
+
#include "../libsvn_fs/fs-loader.h"
#include "private/svn_temp_serializer.h"
#include "private/svn_string_private.h"
-typedef struct id_private_t {
- const char *node_id;
- const char *copy_id;
- const char *txn_id;
- svn_revnum_t rev;
- apr_off_t offset;
-} id_private_t;
+typedef struct fs_fs__id_t
+{
+ /* API visible part */
+ svn_fs_id_t generic_id;
+
+ /* private members */
+ struct
+ {
+ svn_fs_fs__id_part_t node_id;
+ svn_fs_fs__id_part_t copy_id;
+ svn_fs_fs__id_part_t txn_id;
+ svn_fs_fs__id_part_t rev_offset;
+ } private_id;
+} fs_fs__id_t;
+
+
+
+/* Parse the NUL-terminated ID part at DATA and write the result into *PART.
+ * Return TRUE if no errors were detected. */
+static svn_boolean_t
+part_parse(svn_fs_fs__id_part_t *part,
+ const char *data)
+{
+ /* special case: ID inside some transaction */
+ if (data[0] == '_')
+ {
+ part->revision = SVN_INVALID_REVNUM;
+ part->number = svn__base36toui64(&data, data + 1);
+ return *data == '\0';
+ }
+
+ /* special case: 0 / default ID */
+ if (data[0] == '0' && data[1] == '\0')
+ {
+ part->revision = 0;
+ part->number = 0;
+ return TRUE;
+ }
+
+ /* read old style / new style ID */
+ part->number = svn__base36toui64(&data, data);
+ if (data[0] != '-')
+ {
+ part->revision = 0;
+ return *data == '\0';
+ }
+
+ part->revision = SVN_STR_TO_REV(++data);
+
+ return TRUE;
+}
+
+/* Parse the transaction id in DATA and store the result in *TXN_ID.
+ * Return FALSE if there was some problem.
+ */
+static svn_boolean_t
+txn_id_parse(svn_fs_fs__id_part_t *txn_id,
+ const char *data)
+{
+ txn_id->revision = SVN_STR_TO_REV(data);
+ data = strchr(data, '-');
+ if (data == NULL)
+ return FALSE;
+
+ txn_id->number = svn__base36toui64(&data, ++data);
+ return *data == '\0';
+}
+
+/* Write the textual representation of *PART into P and return a pointer
+ * to the first position behind that string.
+ */
+static char *
+unparse_id_part(char *p,
+ const svn_fs_fs__id_part_t *part)
+{
+ if (SVN_IS_VALID_REVNUM(part->revision))
+ {
+ /* ordinary old style / new style ID */
+ p += svn__ui64tobase36(p, part->number);
+ if (part->revision > 0)
+ {
+ *(p++) = '-';
+ p += svn__i64toa(p, part->revision);
+ }
+ }
+ else
+ {
+ /* in txn: mark with "_" prefix */
+ *(p++) = '_';
+ p += svn__ui64tobase36(p, part->number);
+ }
+
+ *(p++) = '.';
+
+ return p;
+}
/* Operations on ID parts */
svn_boolean_t
-svn_fs_fs__id_part_is_root(const char * part)
+svn_fs_fs__id_part_is_root(const svn_fs_fs__id_part_t* part)
{
- return strcmp(part, "0") == 0;
+ return part->revision == 0 && part->number == 0;
}
svn_boolean_t
-svn_fs_fs__id_part_eq(const char *lhs,
- const char *rhs)
+svn_fs_fs__id_part_eq(const svn_fs_fs__id_part_t *lhs,
+ const svn_fs_fs__id_part_t *rhs)
{
- return strcmp(lhs, rhs) == 0;
+ return lhs->revision == rhs->revision && lhs->number == rhs->number;
}
svn_boolean_t
-svn_fs_fs__id_txn_used(const char * const *txn_id)
+svn_fs_fs__id_txn_used(const svn_fs_fs__id_part_t *txn_id)
{
- return *txn_id != NULL;
+ return SVN_IS_VALID_REVNUM(txn_id->revision) || (txn_id->number != 0);
}
void
-svn_fs_fs__id_txn_reset(const char **txn_id)
+svn_fs_fs__id_txn_reset(svn_fs_fs__id_part_t *txn_id)
+{
+ txn_id->revision = SVN_INVALID_REVNUM;
+ txn_id->number = 0;
+}
+
+svn_error_t *
+svn_fs_fs__id_txn_parse(svn_fs_fs__id_part_t *txn_id,
+ const char *data)
{
- *txn_id = NULL;
+ if (! txn_id_parse(txn_id, data))
+ return svn_error_createf(SVN_ERR_FS_MALFORMED_TXN_ID, NULL,
+ "malformed txn id '%s'", data);
+
+ return SVN_NO_ERROR;
}
const char *
-svn_fs_fs__id_txn_unparse(const char * const *txn_id,
+svn_fs_fs__id_txn_unparse(const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
- return apr_pstrdup(pool, *txn_id);
+ char string[2 * SVN_INT64_BUFFER_SIZE + 1];
+ char *p = string;
+
+ p += svn__i64toa(p, txn_id->revision);
+ *(p++) = '-';
+ p += svn__ui64tobase36(p, txn_id->number);
+
+ return apr_pstrmemdup(pool, string, p - string);
}
/* Accessing ID Pieces. */
-const char *
-svn_fs_fs__id_node_id(const svn_fs_id_t *id)
+const svn_fs_fs__id_part_t *
+svn_fs_fs__id_node_id(const svn_fs_id_t *fs_id)
{
- id_private_t *pvt = id->fsap_data;
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
- return pvt->node_id;
+ return &id->private_id.node_id;
}
-const char *
-svn_fs_fs__id_copy_id(const svn_fs_id_t *id)
+const svn_fs_fs__id_part_t *
+svn_fs_fs__id_copy_id(const svn_fs_id_t *fs_id)
{
- id_private_t *pvt = id->fsap_data;
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
- return pvt->copy_id;
+ return &id->private_id.copy_id;
}
-const char *
-svn_fs_fs__id_txn_id(const svn_fs_id_t *id)
+const svn_fs_fs__id_part_t *
+svn_fs_fs__id_txn_id(const svn_fs_id_t *fs_id)
{
- id_private_t *pvt = id->fsap_data;
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
- return pvt->txn_id;
+ return &id->private_id.txn_id;
}
-svn_revnum_t
-svn_fs_fs__id_rev(const svn_fs_id_t *id)
+const svn_fs_fs__id_part_t *
+svn_fs_fs__id_rev_offset(const svn_fs_id_t *fs_id)
{
- id_private_t *pvt = id->fsap_data;
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
- return pvt->rev;
+ return &id->private_id.rev_offset;
}
+svn_revnum_t
+svn_fs_fs__id_rev(const svn_fs_id_t *fs_id)
+{
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
+
+ return id->private_id.rev_offset.revision;
+}
-apr_off_t
-svn_fs_fs__id_offset(const svn_fs_id_t *id)
+apr_uint64_t
+svn_fs_fs__id_offset(const svn_fs_id_t *fs_id)
{
- id_private_t *pvt = id->fsap_data;
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
- return pvt->offset;
+ return id->private_id.rev_offset.number;
}
svn_boolean_t
-svn_fs_fs__id_is_txn(const svn_fs_id_t *id)
+svn_fs_fs__id_is_txn(const svn_fs_id_t *fs_id)
{
- id_private_t *pvt = id->fsap_data;
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
- return pvt->txn_id != NULL;
+ return svn_fs_fs__id_txn_used(&id->private_id.txn_id);
}
svn_string_t *
-svn_fs_fs__id_unparse(const svn_fs_id_t *id,
+svn_fs_fs__id_unparse(const svn_fs_id_t *fs_id,
apr_pool_t *pool)
{
- id_private_t *pvt = id->fsap_data;
+ char string[6 * SVN_INT64_BUFFER_SIZE + 10];
+ fs_fs__id_t *id = (fs_fs__id_t *)fs_id;
- if ((! pvt->txn_id))
- {
- char rev_string[SVN_INT64_BUFFER_SIZE];
- char offset_string[SVN_INT64_BUFFER_SIZE];
+ char *p = unparse_id_part(string, &id->private_id.node_id);
+ p = unparse_id_part(p, &id->private_id.copy_id);
- svn__i64toa(rev_string, pvt->rev);
- svn__i64toa(offset_string, pvt->offset);
- return svn_string_createf(pool, "%s.%s.r%s/%s",
- pvt->node_id, pvt->copy_id,
- rev_string, offset_string);
+ if (svn_fs_fs__id_txn_used(&id->private_id.txn_id))
+ {
+ *(p++) = 't';
+ p += svn__i64toa(p, id->private_id.txn_id.revision);
+ *(p++) = '-';
+ p += svn__ui64tobase36(p, id->private_id.txn_id.number);
}
else
{
- return svn_string_createf(pool, "%s.%s.t%s",
- pvt->node_id, pvt->copy_id,
- pvt->txn_id);
+ *(p++) = 'r';
+ p += svn__i64toa(p, id->private_id.rev_offset.revision);
+ *(p++) = '/';
+ p += svn__i64toa(p, id->private_id.rev_offset.number);
}
+
+ return svn_string_ncreate(string, p - string, pool);
}
@@ -161,23 +280,14 @@ svn_boolean_t
svn_fs_fs__id_eq(const svn_fs_id_t *a,
const svn_fs_id_t *b)
{
- id_private_t *pvta = a->fsap_data, *pvtb = b->fsap_data;
+ fs_fs__id_t *id_a = (fs_fs__id_t *)a;
+ fs_fs__id_t *id_b = (fs_fs__id_t *)b;
if (a == b)
return TRUE;
- if (strcmp(pvta->node_id, pvtb->node_id) != 0)
- return FALSE;
- if (strcmp(pvta->copy_id, pvtb->copy_id) != 0)
- return FALSE;
- if ((pvta->txn_id == NULL) != (pvtb->txn_id == NULL))
- return FALSE;
- if (pvta->txn_id && pvtb->txn_id && strcmp(pvta->txn_id, pvtb->txn_id) != 0)
- return FALSE;
- if (pvta->rev != pvtb->rev)
- return FALSE;
- if (pvta->offset != pvtb->offset)
- return FALSE;
- return TRUE;
+
+ return memcmp(&id_a->private_id, &id_b->private_id,
+ sizeof(id_a->private_id)) == 0;
}
@@ -185,20 +295,24 @@ svn_boolean_t
svn_fs_fs__id_check_related(const svn_fs_id_t *a,
const svn_fs_id_t *b)
{
- id_private_t *pvta = a->fsap_data, *pvtb = b->fsap_data;
+ fs_fs__id_t *id_a = (fs_fs__id_t *)a;
+ fs_fs__id_t *id_b = (fs_fs__id_t *)b;
if (a == b)
return TRUE;
+
/* If both node_ids start with _ and they have differing transaction
IDs, then it is impossible for them to be related. */
- if (pvta->node_id[0] == '_')
+ if (id_a->private_id.node_id.revision == SVN_INVALID_REVNUM)
{
- if (pvta->txn_id && pvtb->txn_id &&
- (strcmp(pvta->txn_id, pvtb->txn_id) != 0))
+ if ( !svn_fs_fs__id_part_eq(&id_a->private_id.txn_id,
+ &id_b->private_id.txn_id)
+ || !svn_fs_fs__id_txn_used(&id_a->private_id.txn_id))
return FALSE;
}
- return (strcmp(pvta->node_id, pvtb->node_id) == 0);
+ return svn_fs_fs__id_part_eq(&id_a->private_id.node_id,
+ &id_b->private_id.node_id);
}
@@ -211,6 +325,18 @@ svn_fs_fs__id_compare(const svn_fs_id_t
return (svn_fs_fs__id_check_related(a, b) ? 1 : -1);
}
+int
+svn_fs_fs__id_part_compare(const svn_fs_fs__id_part_t *a,
+ const svn_fs_fs__id_part_t *b)
+{
+ if (a->revision < b->revision)
+ return -1;
+ if (a->revision > b->revision)
+ return 1;
+
+ return a->number < b->number ? -1 : a->number == b->number ? 0 : 1;
+}
+
/* Creating ID's. */
@@ -221,72 +347,72 @@ static id_vtable_t id_vtable = {
};
svn_fs_id_t *
-svn_fs_fs__id_txn_create_root(const char *txn_id,
+svn_fs_fs__id_txn_create_root(const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
- return svn_fs_fs__id_txn_create("0", "0", txn_id, pool);
-}
+ fs_fs__id_t *id = apr_pcalloc(pool, sizeof(*id));
+
+ /* node ID and copy ID are "0" */
+
+ id->private_id.txn_id = *txn_id;
+ id->private_id.rev_offset.revision = SVN_INVALID_REVNUM;
+
+ id->generic_id.vtable = &id_vtable;
+ id->generic_id.fsap_data = &id;
+ return (svn_fs_id_t *)id;
+}
svn_fs_id_t *
-svn_fs_fs__id_txn_create(const char *node_id,
- const char *copy_id,
- const char *txn_id,
+svn_fs_fs__id_txn_create(const svn_fs_fs__id_part_t *node_id,
+ const svn_fs_fs__id_part_t *copy_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool)
{
- svn_fs_id_t *id = apr_palloc(pool, sizeof(*id));
- id_private_t *pvt = apr_palloc(pool, sizeof(*pvt));
+ fs_fs__id_t *id = apr_pcalloc(pool, sizeof(*id));
+
+ id->private_id.node_id = *node_id;
+ id->private_id.copy_id = *copy_id;
+ id->private_id.txn_id = *txn_id;
+ id->private_id.rev_offset.revision = SVN_INVALID_REVNUM;
+
+ id->generic_id.vtable = &id_vtable;
+ id->generic_id.fsap_data = &id;
- pvt->node_id = apr_pstrdup(pool, node_id);
- pvt->copy_id = apr_pstrdup(pool, copy_id);
- pvt->txn_id = apr_pstrdup(pool, txn_id);
- pvt->rev = SVN_INVALID_REVNUM;
- pvt->offset = -1;
-
- id->vtable = &id_vtable;
- id->fsap_data = pvt;
- return id;
+ return (svn_fs_id_t *)id;
}
svn_fs_id_t *
-svn_fs_fs__id_rev_create(const char *node_id,
- const char *copy_id,
- svn_revnum_t rev,
- apr_off_t offset,
+svn_fs_fs__id_rev_create(const svn_fs_fs__id_part_t *node_id,
+ const svn_fs_fs__id_part_t *copy_id,
+ const svn_fs_fs__id_part_t *rev_offset,
apr_pool_t *pool)
{
- svn_fs_id_t *id = apr_palloc(pool, sizeof(*id));
- id_private_t *pvt = apr_palloc(pool, sizeof(*pvt));
+ fs_fs__id_t *id = apr_pcalloc(pool, sizeof(*id));
- pvt->node_id = apr_pstrdup(pool, node_id);
- pvt->copy_id = apr_pstrdup(pool, copy_id);
- pvt->txn_id = NULL;
- pvt->rev = rev;
- pvt->offset = offset;
-
- id->vtable = &id_vtable;
- id->fsap_data = pvt;
- return id;
+ id->private_id.node_id = *node_id;
+ id->private_id.copy_id = *copy_id;
+ id->private_id.txn_id.revision = SVN_INVALID_REVNUM;
+ id->private_id.rev_offset = *rev_offset;
+
+ id->generic_id.vtable = &id_vtable;
+ id->generic_id.fsap_data = &id;
+
+ return (svn_fs_id_t *)id;
}
svn_fs_id_t *
-svn_fs_fs__id_copy(const svn_fs_id_t *id, apr_pool_t *pool)
+svn_fs_fs__id_copy(const svn_fs_id_t *source, apr_pool_t *pool)
{
- svn_fs_id_t *new_id = apr_palloc(pool, sizeof(*new_id));
- id_private_t *new_pvt = apr_palloc(pool, sizeof(*new_pvt));
- id_private_t *pvt = id->fsap_data;
-
- new_pvt->node_id = apr_pstrdup(pool, pvt->node_id);
- new_pvt->copy_id = apr_pstrdup(pool, pvt->copy_id);
- new_pvt->txn_id = pvt->txn_id ? apr_pstrdup(pool, pvt->txn_id) : NULL;
- new_pvt->rev = pvt->rev;
- new_pvt->offset = pvt->offset;
-
- new_id->vtable = &id_vtable;
- new_id->fsap_data = new_pvt;
- return new_id;
+ fs_fs__id_t *id = (fs_fs__id_t *)source;
+ fs_fs__id_t *new_id = apr_palloc(pool, sizeof(*new_id));
+
+ *new_id = *id;
+ new_id->generic_id.fsap_data = new_id;
+
+ return (svn_fs_id_t *)new_id;
}
@@ -295,8 +421,7 @@ svn_fs_fs__id_parse(const char *data,
apr_size_t len,
apr_pool_t *pool)
{
- svn_fs_id_t *id;
- id_private_t *pvt;
+ fs_fs__id_t *id;
char *data_copy, *str;
/* Dup the ID data into POOL. Our returned ID will have references
@@ -304,10 +429,9 @@ svn_fs_fs__id_parse(const char *data,
data_copy = apr_pstrmemdup(pool, data, len);
/* Alloc a new svn_fs_id_t structure. */
- id = apr_palloc(pool, sizeof(*id));
- pvt = apr_palloc(pool, sizeof(*pvt));
- id->vtable = &id_vtable;
- id->fsap_data = pvt;
+ id = apr_pcalloc(pool, sizeof(*id));
+ id->generic_id.vtable = &id_vtable;
+ id->generic_id.fsap_data = &id;
/* Now, we basically just need to "split" this data on `.'
characters. We will use svn_cstring_tokenize, which will put
@@ -319,13 +443,15 @@ svn_fs_fs__id_parse(const char *data,
str = svn_cstring_tokenize(".", &data_copy);
if (str == NULL)
return NULL;
- pvt->node_id = str;
+ if (! part_parse(&id->private_id.node_id, str))
+ return NULL;
/* Copy Id */
str = svn_cstring_tokenize(".", &data_copy);
if (str == NULL)
return NULL;
- pvt->copy_id = str;
+ if (! part_parse(&id->private_id.copy_id, str))
+ return NULL;
/* Txn/Rev Id */
str = svn_cstring_tokenize(".", &data_copy);
@@ -338,119 +464,78 @@ svn_fs_fs__id_parse(const char *data,
svn_error_t *err;
/* This is a revision type ID */
- pvt->txn_id = NULL;
+ id->private_id.txn_id.revision = SVN_INVALID_REVNUM;
+ id->private_id.txn_id.number = 0;
data_copy = str + 1;
str = svn_cstring_tokenize("/", &data_copy);
if (str == NULL)
return NULL;
- pvt->rev = SVN_STR_TO_REV(str);
+ id->private_id.rev_offset.revision = SVN_STR_TO_REV(str);
- str = svn_cstring_tokenize("/", &data_copy);
- if (str == NULL)
- return NULL;
- err = svn_cstring_atoi64(&val, str);
+ err = svn_cstring_atoi64(&val, data_copy);
if (err)
{
svn_error_clear(err);
return NULL;
}
- pvt->offset = (apr_off_t)val;
+ id->private_id.rev_offset.number = (apr_uint64_t)val;
}
else if (str[0] == 't')
{
/* This is a transaction type ID */
- pvt->txn_id = str + 1;
- pvt->rev = SVN_INVALID_REVNUM;
- pvt->offset = -1;
+ id->private_id.rev_offset.revision = SVN_INVALID_REVNUM;
+ id->private_id.rev_offset.number = 0;
+
+ if (! txn_id_parse(&id->private_id.txn_id, str + 1))
+ return NULL;
}
else
return NULL;
- return id;
+ return (svn_fs_id_t *)id;
}
/* (de-)serialization support */
-/* Serialization of the PVT sub-structure within the CONTEXT.
- */
-static void
-serialize_id_private(svn_temp_serializer__context_t *context,
- const id_private_t * const *pvt)
-{
- const id_private_t *private = *pvt;
-
- /* serialize the pvt data struct itself */
- svn_temp_serializer__push(context,
- (const void * const *)pvt,
- sizeof(*private));
-
- /* append the referenced strings */
- svn_temp_serializer__add_string(context, &private->node_id);
- svn_temp_serializer__add_string(context, &private->copy_id);
- svn_temp_serializer__add_string(context, &private->txn_id);
-
- /* return to caller's nesting level */
- svn_temp_serializer__pop(context);
-}
-
/* Serialize an ID within the serialization CONTEXT.
*/
void
svn_fs_fs__id_serialize(svn_temp_serializer__context_t *context,
- const struct svn_fs_id_t * const *id)
+ const svn_fs_id_t * const *in)
{
+ const fs_fs__id_t *id = (const fs_fs__id_t *)*in;
+
/* nothing to do for NULL ids */
- if (*id == NULL)
+ if (id == NULL)
return;
/* serialize the id data struct itself */
- svn_temp_serializer__push(context,
- (const void * const *)id,
- sizeof(**id));
-
- /* serialize the id_private_t data sub-struct */
- serialize_id_private(context,
- (const id_private_t * const *)&(*id)->fsap_data);
-
- /* return to caller's nesting level */
- svn_temp_serializer__pop(context);
-}
-
-/* Deserialization of the PVT sub-structure in BUFFER.
- */
-static void
-deserialize_id_private(void *buffer, id_private_t **pvt)
-{
- /* fixup the reference to the only sub-structure */
- id_private_t *private;
- svn_temp_deserializer__resolve(buffer, (void**)pvt);
-
- /* fixup the sub-structure itself */
- private = *pvt;
- svn_temp_deserializer__resolve(private, (void**)&private->node_id);
- svn_temp_deserializer__resolve(private, (void**)&private->copy_id);
- svn_temp_deserializer__resolve(private, (void**)&private->txn_id);
+ svn_temp_serializer__add_leaf(context,
+ (const void * const *)in,
+ sizeof(fs_fs__id_t));
}
/* Deserialize an ID inside the BUFFER.
*/
void
-svn_fs_fs__id_deserialize(void *buffer, svn_fs_id_t **id)
+svn_fs_fs__id_deserialize(void *buffer, svn_fs_id_t **in_out)
{
+ fs_fs__id_t *id;
+
/* The id maybe all what is in the whole buffer.
* Don't try to fixup the pointer in that case*/
- if (*id != buffer)
- svn_temp_deserializer__resolve(buffer, (void**)id);
+ if (*in_out != buffer)
+ svn_temp_deserializer__resolve(buffer, (void**)in_out);
+
+ id = (fs_fs__id_t *)*in_out;
/* no id, no sub-structure fixup necessary */
- if (*id == NULL)
+ if (id == NULL)
return;
/* the stored vtable is bogus at best -> set the right one */
- (*id)->vtable = &id_vtable;
-
- /* handle sub-structures */
- deserialize_id_private(*id, (id_private_t **)&(*id)->fsap_data);
+ id->generic_id.vtable = &id_vtable;
+ id->generic_id.fsap_data = id;
}
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.h
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.h?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.h (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/id.h Wed Jul 24 13:24:44 2013
@@ -29,46 +29,71 @@
extern "C" {
#endif /* __cplusplus */
+/* A rev node ID in FSFS consists of a 3 of sub-IDs ("parts") that consist
+ * of a creation REVISION number and some revision-local counter value
+ * (NUMBER). Old-style ID parts use global counter values.
+ */
+typedef struct svn_fs_fs__id_part_t
+{
+ /* SVN_INVALID_REVNUM for txns -> not a txn, COUNTER must be 0.
+ SVN_INVALID_REVNUM for others -> not assigned to a revision, yet.
+ 0 for others -> old-style ID or the root in rev 0. */
+ svn_revnum_t revision;
+
+ /* sub-id value relative to REVISION. Its interpretation depends on
+ the part itself. In rev_offset, it is the offset value, in others
+ it represents a unique counter value. */
+ apr_uint64_t number;
+} svn_fs_fs__id_part_t;
+
+
/*** Operations on ID parts. ***/
/* Return TRUE, if both elements of the PART is 0, i.e. this is the default
* value if e.g. no copies were made of this node. */
-svn_boolean_t svn_fs_fs__id_part_is_root(const char *part);
+svn_boolean_t svn_fs_fs__id_part_is_root(const svn_fs_fs__id_part_t *part);
/* Return TRUE, if all element values of *LHS and *RHS match. */
-svn_boolean_t svn_fs_fs__id_part_eq(const char * lhs,
- const char * rhs);
+svn_boolean_t svn_fs_fs__id_part_eq(const svn_fs_fs__id_part_t *lhs,
+ const svn_fs_fs__id_part_t *rhs);
/* Return TRUE, if TXN_ID is used, i.e. doesn't contain just the defaults. */
-svn_boolean_t svn_fs_fs__id_txn_used(const char * const *txn_id);
+svn_boolean_t svn_fs_fs__id_txn_used(const svn_fs_fs__id_part_t *txn_id);
/* Reset TXN_ID to the defaults. */
-void svn_fs_fs__id_txn_reset(const char **txn_id);
+void svn_fs_fs__id_txn_reset(svn_fs_fs__id_part_t *txn_id);
+
+/* Parse the transaction id in DATA and store the result in *TXN_ID */
+svn_error_t *svn_fs_fs__id_txn_parse(svn_fs_fs__id_part_t *txn_id,
+ const char *data);
/* Convert the transaction id in *TXN_ID into a textual representation
* allocated in POOL. */
-const char *svn_fs_fs__id_txn_unparse(const char * const *txn_id,
+const char *svn_fs_fs__id_txn_unparse(const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
/*** ID accessor functions. ***/
/* Get the "node id" portion of ID. */
-const char *svn_fs_fs__id_node_id(const svn_fs_id_t *id);
+const svn_fs_fs__id_part_t *svn_fs_fs__id_node_id(const svn_fs_id_t *id);
/* Get the "copy id" portion of ID. */
-const char *svn_fs_fs__id_copy_id(const svn_fs_id_t *id);
+const svn_fs_fs__id_part_t *svn_fs_fs__id_copy_id(const svn_fs_id_t *id);
/* Get the "txn id" portion of ID, or NULL if it is a permanent ID. */
-const char *svn_fs_fs__id_txn_id(const svn_fs_id_t *id);
+const svn_fs_fs__id_part_t *svn_fs_fs__id_txn_id(const svn_fs_id_t *id);
+
+/* Get the "rev,offset" portion of ID. */
+const svn_fs_fs__id_part_t *svn_fs_fs__id_rev_offset(const svn_fs_id_t *id);
/* Get the "rev" portion of ID, or SVN_INVALID_REVNUM if it is a
transaction ID. */
svn_revnum_t svn_fs_fs__id_rev(const svn_fs_id_t *id);
-/* Access the "offset" portion of the ID, or -1 if it is a transaction
+/* Access the "offset" portion of the ID, or 0 if it is a transaction
ID. */
-apr_off_t svn_fs_fs__id_offset(const svn_fs_id_t *id);
+apr_uint64_t svn_fs_fs__id_offset(const svn_fs_id_t *id);
/* Return TRUE, if this is a transaction ID. */
svn_boolean_t svn_fs_fs__id_is_txn(const svn_fs_id_t *id);
@@ -89,23 +114,26 @@ svn_boolean_t svn_fs_fs__id_check_relate
int svn_fs_fs__id_compare(const svn_fs_id_t *a,
const svn_fs_id_t *b);
+/* Return 0 if A and B are equal, 1 if A is "greater than" B, -1 otherwise. */
+int svn_fs_fs__id_part_compare(const svn_fs_fs__id_part_t *a,
+ const svn_fs_fs__id_part_t *b);
+
/* Create the txn root ID for transaction TXN_ID. Allocate it in POOL. */
-svn_fs_id_t *svn_fs_fs__id_txn_create_root(const char *txn_id,
+svn_fs_id_t *svn_fs_fs__id_txn_create_root(const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
/* Create an ID within a transaction based on NODE_ID, COPY_ID, and
TXN_ID, allocated in POOL. */
-svn_fs_id_t *svn_fs_fs__id_txn_create(const char *node_id,
- const char *copy_id,
- const char *txn_id,
+svn_fs_id_t *svn_fs_fs__id_txn_create(const svn_fs_fs__id_part_t *node_id,
+ const svn_fs_fs__id_part_t *copy_id,
+ const svn_fs_fs__id_part_t *txn_id,
apr_pool_t *pool);
-/* Create a permanent ID based on NODE_ID, COPY_ID, REV, and OFFSET,
+/* Create a permanent ID based on NODE_ID, COPY_ID and REV_OFFSET,
allocated in POOL. */
-svn_fs_id_t *svn_fs_fs__id_rev_create(const char *node_id,
- const char *copy_id,
- svn_revnum_t rev,
- apr_off_t offset,
+svn_fs_id_t *svn_fs_fs__id_rev_create(const svn_fs_fs__id_part_t *node_id,
+ const svn_fs_fs__id_part_t *copy_id,
+ const svn_fs_fs__id_part_t *rev_offset,
apr_pool_t *pool);
/* Return a copy of ID, allocated from POOL. */
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/low_level.c Wed Jul 24 13:24:44 2013
@@ -629,7 +629,7 @@ read_rep_offsets(representation_t **rep_
if ((*rep_p)->revision == SVN_INVALID_REVNUM)
if (noderev_id)
- (*rep_p)->txn_id = svn_fs_fs__id_txn_id(noderev_id);
+ (*rep_p)->txn_id = *svn_fs_fs__id_txn_id(noderev_id);
return SVN_NO_ERROR;
}
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.c Wed Jul 24 13:24:44 2013
@@ -26,7 +26,6 @@
#include "svn_pools.h"
#include "private/svn_string_private.h"
-#include "key-gen.h"
#include "low_level.h"
#include "rep-cache.h"
#include "revprops.h"
@@ -150,8 +149,8 @@ recover_find_max_ids(svn_fs_t *fs,
svn_revnum_t rev,
apr_file_t *rev_file,
apr_off_t offset,
- char *max_node_id,
- char *max_copy_id,
+ apr_uint64_t *max_node_id,
+ apr_uint64_t *max_copy_id,
apr_pool_t *pool)
{
svn_fs_fs__rep_header_t *header;
@@ -214,7 +213,8 @@ recover_find_max_ids(svn_fs_t *fs,
char *str;
svn_node_kind_t kind;
svn_fs_id_t *id;
- const char *node_id, *copy_id;
+ const svn_fs_fs__id_part_t *rev_offset;
+ apr_uint64_t node_id, copy_id;
apr_off_t child_dir_offset;
const svn_string_t *path = svn__apr_hash_index_val(hi);
@@ -244,31 +244,26 @@ recover_find_max_ids(svn_fs_t *fs,
id = svn_fs_fs__id_parse(str, strlen(str), iterpool);
- if (svn_fs_fs__id_rev(id) != rev)
+ rev_offset = svn_fs_fs__id_rev_offset(id);
+ if (rev_offset->revision != rev)
{
/* If the node wasn't modified in this revision, we've already
checked the node and copy id. */
continue;
}
- node_id = svn_fs_fs__id_node_id(id);
- copy_id = svn_fs_fs__id_copy_id(id);
+ node_id = svn_fs_fs__id_node_id(id)->number;
+ copy_id = svn_fs_fs__id_copy_id(id)->number;
- if (svn_fs_fs__key_compare(node_id, max_node_id) > 0)
- {
- SVN_ERR_ASSERT(strlen(node_id) < MAX_KEY_SIZE);
- apr_cpystrn(max_node_id, node_id, MAX_KEY_SIZE);
- }
- if (svn_fs_fs__key_compare(copy_id, max_copy_id) > 0)
- {
- SVN_ERR_ASSERT(strlen(copy_id) < MAX_KEY_SIZE);
- apr_cpystrn(max_copy_id, copy_id, MAX_KEY_SIZE);
- }
+ if (node_id > *max_node_id)
+ *max_node_id = node_id;
+ if (copy_id > *max_copy_id)
+ *max_copy_id = copy_id;
if (kind == svn_node_file)
continue;
- child_dir_offset = svn_fs_fs__id_offset(id);
+ child_dir_offset = rev_offset->number;
SVN_ERR(recover_find_max_ids(fs, rev, rev_file, child_dir_offset,
max_node_id, max_copy_id, iterpool));
}
@@ -280,8 +275,8 @@ recover_find_max_ids(svn_fs_t *fs,
svn_error_t *
svn_fs_fs__find_max_ids(svn_fs_t *fs,
svn_revnum_t youngest,
- char *max_node_id,
- char *max_copy_id,
+ apr_uint64_t *max_node_id,
+ apr_uint64_t *max_copy_id,
apr_pool_t *pool)
{
fs_fs_data_t *ffd = fs->fsap_data;
@@ -320,8 +315,8 @@ recover_body(void *baton, apr_pool_t *po
svn_fs_t *fs = b->fs;
fs_fs_data_t *ffd = fs->fsap_data;
svn_revnum_t max_rev;
- char next_node_id_buf[MAX_KEY_SIZE], next_copy_id_buf[MAX_KEY_SIZE];
- char *next_node_id = NULL, *next_copy_id = NULL;
+ apr_uint64_t next_node_id = 0;
+ apr_uint64_t next_copy_id = 0;
svn_revnum_t youngest_rev;
svn_node_kind_t youngest_revprops_kind;
@@ -381,39 +376,23 @@ recover_body(void *baton, apr_pool_t *po
we go along. */
svn_revnum_t rev;
apr_pool_t *iterpool = svn_pool_create(pool);
- char max_node_id[MAX_KEY_SIZE] = "0", max_copy_id[MAX_KEY_SIZE] = "0";
- apr_size_t len;
for (rev = 0; rev <= max_rev; rev++)
{
- apr_file_t *rev_file;
- apr_off_t root_offset;
- svn_fs_id_t *root_id;
-
svn_pool_clear(iterpool);
if (b->cancel_func)
SVN_ERR(b->cancel_func(b->cancel_baton));
- SVN_ERR(svn_fs_fs__open_pack_or_rev_file(&rev_file, fs, rev,
- iterpool));
- SVN_ERR(svn_fs_fs__rev_get_root(&root_id, fs, rev, iterpool));
-
- root_offset = svn_fs_fs__id_offset(root_id);
- SVN_ERR(recover_find_max_ids(fs, rev, rev_file, root_offset,
- max_node_id, max_copy_id, iterpool));
- SVN_ERR(svn_io_file_close(rev_file, iterpool));
+ SVN_ERR(svn_fs_fs__find_max_ids(fs, rev, &next_node_id,
+ &next_copy_id, iterpool));
}
svn_pool_destroy(iterpool);
/* Now that we finally have the maximum revision, node-id and copy-id, we
can bump the two ids to get the next of each. */
- len = strlen(max_node_id);
- svn_fs_fs__next_key(max_node_id, &len, next_node_id_buf);
- next_node_id = next_node_id_buf;
- len = strlen(max_copy_id);
- svn_fs_fs__next_key(max_copy_id, &len, next_copy_id_buf);
- next_copy_id = next_copy_id_buf;
+ next_node_id++;
+ next_copy_id++;
}
/* Before setting current, verify that there is a revprops file
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.h
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.h?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.h (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/recovery.h Wed Jul 24 13:24:44 2013
@@ -31,8 +31,8 @@
svn_error_t *
svn_fs_fs__find_max_ids(svn_fs_t *fs,
svn_revnum_t youngest,
- char *max_node_id,
- char *max_copy_id,
+ apr_uint64_t *max_node_id,
+ apr_uint64_t *max_copy_id,
apr_pool_t *pool);
/* Recover the fsfs associated with filesystem FS.
Modified: subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c
URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c?rev=1506545&r1=1506544&r2=1506545&view=diff
==============================================================================
--- subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c (original)
+++ subversion/branches/fsfs-improvements/subversion/libsvn_fs_fs/temp_serializer.c Wed Jul 24 13:24:44 2013
@@ -183,7 +183,6 @@ serialize_representation(svn_temp_serial
serialize_checksum(context, &rep->md5_checksum);
serialize_checksum(context, &rep->sha1_checksum);
- svn_temp_serializer__add_string(context, &rep->txn_id);
svn_temp_serializer__add_string(context, &rep->uniquifier);
/* return to the caller's nesting level */
@@ -208,7 +207,6 @@ deserialize_representation(void *buffer,
deserialize_checksum(rep, &rep->md5_checksum);
deserialize_checksum(rep, &rep->sha1_checksum);
- svn_temp_deserializer__resolve(rep, (void **)&rep->txn_id);
svn_temp_deserializer__resolve(rep, (void **)&rep->uniquifier);
}
Re: svn commit: r1506545 [1/2] - in /subversion/branches/fsfs-improvements/subversion:
include/ libsvn_fs_fs/
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Thu, Aug 21, 2014 at 1:17 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 21.08.2014 11:33, Branko Čibej wrote:
>
> On 21.08.2014 10:44, Evgeny Kotkov wrote:
>
>
> I am wondering, whether this is a portable way of comparing structs.
>
>
> It is not. The portable way is to use the '==' operator.
>
>
> ... in C++, that is ...
>
> memcmp is not even safe (in general) if the struct is initialized to a
> known value (e.g., with memset to 0), because the compiler is allowed to
> use the struct padding any way it likes; and a function-local struct may
> not even exist on the stack until the memcmp is called, causing the
> compiler to spill its contents onto the stack.
>
> The only really portable way to compare structs is to (recursively)
> compare its members using '=='. That said, I've yet to see a platform where
> memcmp after memset (or calloc) didn't work.
>
>
Changed in r1619358 and 1620602.
Thanks everyone!
-- Stefan^2.
Re: svn commit: r1506545 [1/2] - in /subversion/branches/fsfs-improvements/subversion:
include/ libsvn_fs_fs/
Posted by Branko Čibej <br...@wandisco.com>.
On 21.08.2014 11:33, Branko Čibej wrote:
> On 21.08.2014 10:44, Evgeny Kotkov wrote:
>>
>> I am wondering, whether this is a portable way of comparing structs.
>
> It is not. The portable way is to use the '==' operator.
... in C++, that is ...
memcmp is not even safe (in general) if the struct is initialized to a
known value (e.g., with memset to 0), because the compiler is allowed to
use the struct padding any way it likes; and a function-local struct may
not even exist on the stack until the memcmp is called, causing the
compiler to spill its contents onto the stack.
The only really portable way to compare structs is to (recursively)
compare its members using '=='. That said, I've yet to see a platform
where memcmp after memset (or calloc) didn't work.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com
Re: svn commit: r1506545 [1/2] - in /subversion/branches/fsfs-improvements/subversion:
include/ libsvn_fs_fs/
Posted by Branko Čibej <br...@wandisco.com>.
On 21.08.2014 10:44, Evgeny Kotkov wrote:
>> Author: stefan2
>> Date: Wed Jul 24 13:24:44 2013
>> New Revision: 1506545
>>
>> URL: http://svn.apache.org/r1506545
>> Log:
>> On the fsfs-improvements branch: Finally switch to a new FSFS ID API.
>>
>> This replaces the previous string-based API with one that represents
>> IDs as quadruples of <revision,sub-id> pairs of integers. Thus, it
>> now matches the implicit usage of the node_id, branch_id, txn_id and
>> rev_offset parts of those IDs.
>>
>> The patch does four things. First, it replaces the current API in
>> id.*. Second, it must use the new svn_fs_fs__id_part_t * instead of
>> const char * in all FSFS code. In some places we have to add a level
>> of indirection as key parts can now be put on stack instead of being
>> pool-allocated but we always pass them along through pointers.
>>
>> Third, structs using a txn ID will use the new data type as well -
>> requiring more code to be updated. Lastly, we have to update code
>> that (de-)serializes IDs or handles ID assignment and increments.
> [...]
>
>> svn_fs_fs__id_eq(const svn_fs_id_t *a,
>> const svn_fs_id_t *b)
>> {
>> - id_private_t *pvta = a->fsap_data, *pvtb = b->fsap_data;
>> + fs_fs__id_t *id_a = (fs_fs__id_t *)a;
>> + fs_fs__id_t *id_b = (fs_fs__id_t *)b;
>>
>> if (a == b)
>> return TRUE;
>> - if (strcmp(pvta->node_id, pvtb->node_id) != 0)
>> - return FALSE;
>> - if (strcmp(pvta->copy_id, pvtb->copy_id) != 0)
>> - return FALSE;
>> - if ((pvta->txn_id == NULL) != (pvtb->txn_id == NULL))
>> - return FALSE;
>> - if (pvta->txn_id && pvtb->txn_id && strcmp(pvta->txn_id, pvtb->txn_id) != 0)
>> - return FALSE;
>> - if (pvta->rev != pvtb->rev)
>> - return FALSE;
>> - if (pvta->offset != pvtb->offset)
>> - return FALSE;
>> - return TRUE;
>> +
>> + return memcmp(&id_a->private_id, &id_b->private_id,
>> + sizeof(id_a->private_id)) == 0;
>> }
> I am wondering, whether this is a portable way of comparing structs.
It is not. The portable way is to use the '==' operator.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com
Re: svn commit: r1506545 [1/2] - in /subversion/branches/fsfs-improvements/subversion:
include/ libsvn_fs_fs/
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
> Author: stefan2
> Date: Wed Jul 24 13:24:44 2013
> New Revision: 1506545
>
> URL: http://svn.apache.org/r1506545
> Log:
> On the fsfs-improvements branch: Finally switch to a new FSFS ID API.
>
> This replaces the previous string-based API with one that represents
> IDs as quadruples of <revision,sub-id> pairs of integers. Thus, it
> now matches the implicit usage of the node_id, branch_id, txn_id and
> rev_offset parts of those IDs.
>
> The patch does four things. First, it replaces the current API in
> id.*. Second, it must use the new svn_fs_fs__id_part_t * instead of
> const char * in all FSFS code. In some places we have to add a level
> of indirection as key parts can now be put on stack instead of being
> pool-allocated but we always pass them along through pointers.
>
> Third, structs using a txn ID will use the new data type as well -
> requiring more code to be updated. Lastly, we have to update code
> that (de-)serializes IDs or handles ID assignment and increments.
[...]
> svn_fs_fs__id_eq(const svn_fs_id_t *a,
> const svn_fs_id_t *b)
> {
> - id_private_t *pvta = a->fsap_data, *pvtb = b->fsap_data;
> + fs_fs__id_t *id_a = (fs_fs__id_t *)a;
> + fs_fs__id_t *id_b = (fs_fs__id_t *)b;
>
> if (a == b)
> return TRUE;
> - if (strcmp(pvta->node_id, pvtb->node_id) != 0)
> - return FALSE;
> - if (strcmp(pvta->copy_id, pvtb->copy_id) != 0)
> - return FALSE;
> - if ((pvta->txn_id == NULL) != (pvtb->txn_id == NULL))
> - return FALSE;
> - if (pvta->txn_id && pvtb->txn_id && strcmp(pvta->txn_id, pvtb->txn_id) != 0)
> - return FALSE;
> - if (pvta->rev != pvtb->rev)
> - return FALSE;
> - if (pvta->offset != pvtb->offset)
> - return FALSE;
> - return TRUE;
> +
> + return memcmp(&id_a->private_id, &id_b->private_id,
> + sizeof(id_a->private_id)) == 0;
> }
I am wondering, whether this is a portable way of comparing structs. Isn't
it possible that fs_fs__id_t would compile with padding and hence, result in
false negative checks here?
It looks like we do not always use apr_pcalloc() to zero out the memory for
these structures (e.g. within the subversion/libsvn_fs_fs/temp_deserializer.c
code), so it might be possible that the padding bytes would contain some random
garbage, and we would attempt to compare it with memcmp().
Regards,
Evgeny Kotkov
Re: svn commit: r1506545 [1/2] - in /subversion/branches/fsfs-improvements/subversion:
include/ libsvn_fs_fs/
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
> Author: stefan2
> Date: Wed Jul 24 13:24:44 2013
> New Revision: 1506545
>
> URL: http://svn.apache.org/r1506545
> Log:
> On the fsfs-improvements branch: Finally switch to a new FSFS ID API.
>
> This replaces the previous string-based API with one that represents
> IDs as quadruples of <revision,sub-id> pairs of integers. Thus, it
> now matches the implicit usage of the node_id, branch_id, txn_id and
> rev_offset parts of those IDs.
>
> The patch does four things. First, it replaces the current API in
> id.*. Second, it must use the new svn_fs_fs__id_part_t * instead of
> const char * in all FSFS code. In some places we have to add a level
> of indirection as key parts can now be put on stack instead of being
> pool-allocated but we always pass them along through pointers.
>
> Third, structs using a txn ID will use the new data type as well -
> requiring more code to be updated. Lastly, we have to update code
> that (de-)serializes IDs or handles ID assignment and increments.
[...]
> svn_fs_fs__id_eq(const svn_fs_id_t *a,
> const svn_fs_id_t *b)
> {
> - id_private_t *pvta = a->fsap_data, *pvtb = b->fsap_data;
> + fs_fs__id_t *id_a = (fs_fs__id_t *)a;
> + fs_fs__id_t *id_b = (fs_fs__id_t *)b;
>
> if (a == b)
> return TRUE;
> - if (strcmp(pvta->node_id, pvtb->node_id) != 0)
> - return FALSE;
> - if (strcmp(pvta->copy_id, pvtb->copy_id) != 0)
> - return FALSE;
> - if ((pvta->txn_id == NULL) != (pvtb->txn_id == NULL))
> - return FALSE;
> - if (pvta->txn_id && pvtb->txn_id && strcmp(pvta->txn_id, pvtb->txn_id) != 0)
> - return FALSE;
> - if (pvta->rev != pvtb->rev)
> - return FALSE;
> - if (pvta->offset != pvtb->offset)
> - return FALSE;
> - return TRUE;
> +
> + return memcmp(&id_a->private_id, &id_b->private_id,
> + sizeof(id_a->private_id)) == 0;
> }
I am wondering, whether this is a portable way of comparing structs. Isn't
it possible that fs_fs__id_t would compile with padding and hence, result in
false negative checks here?
It looks like we do not always use apr_pcalloc() to zero out the memory for
these structures (e.g. within the subversion/libsvn_fs_fs/temp_deserializer.c
code), so it might be possible that the padding bytes would contain some random
garbage, and we would attempt to compare it with memcmp().
Regards,
Evgeny Kotkov