You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Yoshiki Hayashi <yo...@xemacs.org> on 2001/02/22 03:52:51 UTC
[PATCH] svn_fs__open_dag
I wanted to be more familiar with libsvn_fs code so I
started writing part of it. But be careful! This is my
first real function in subversion. You might want to
rewrite it from scratch. It does compile and I think it's
correct, though.
The upside of living in a different timezone is that it's
very unlikely that my work will collide with otheres. ;-)
I have one question with regard to svn_fs__dag_close which
was just deleted. It's still referenced by tree.c and I'd
like to resurrect it. It will call just apr_pool_destroy
(dag->pool) if I understand it correctly. The problem of
current implementation is that dag->pool points to
trail->pool. Shouldn't dag->pool be subpool of trail->pool?
Oh, the downside of different timezone is that we can't
communicate in real time basis. :-)
(svn_fs__dag_dup): New function.
(svn_fs__dag_open): Implement it.
Index: dag.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/dag.c,v
retrieving revision 1.27
diff -u -r1.27 dag.c
--- dag.c 2001/02/21 23:28:27 1.27
+++ dag.c 2001/02/22 03:51:05
@@ -103,6 +103,15 @@
*/
+dag_node_t *
+svn_fs__dag_dup (dag_node_t *node,
+ trail_t *trail)
+{
+ dag_node_t *dag_dup = apr_palloc (trail->pool, sizeof (*node));
+ memcpy (dag_dup, node, sizeof (*node));
+ return dag_dup;
+}
+
const svn_fs_id_t *svn_fs__dag_get_id (dag_node_t *node)
{
return node->id;
@@ -159,7 +168,7 @@
itself a list. */
skel_t *header = node->contents->children;
- /* The 3nd element of the header, IF it exists, is the header's
+ /* The 3rd element of the header, IF it exists, is the header's
first `flag'. It could be NULL. */
skel_t *flag = header->children->next->next;
@@ -288,14 +297,45 @@
}
-svn_error_t *svn_fs__dag_open (dag_node_t **child_p,
- dag_node_t *parent,
- const char *name,
- trail_t *trail)
-{
- abort();
- /* NOTREACHED */
- return NULL;
+svn_error_t *
+svn_fs__dag_open (dag_node_t **child_p,
+ dag_node_t *parent,
+ const char *name,
+ trail_t *trail)
+{
+ /* (HEADER ENTRY ...) */
+ skel_t *entry = parent->contents->next;
+ dag_node_t *child = apr_palloc (trail->pool, sizeof (*child));
+
+ while (entry)
+ {
+ /* Sanity check. All entry in dir must be a form of (NAME ID). */
+ if (entry->is_atom || ! entry->children->is_atom)
+ abort ();
+
+ if (! memcmp (entry->children->data, name, entry->children->len))
+ {
+ skel_t *id = entry->children->next;
+ child->id = svn_fs_parse_id (id->data, id->len, trail->pool);
+ break;
+ }
+
+ entry = entry->next;
+ }
+
+ if (! child->id)
+ return svn_error_createf
+ (SVN_ERR_FS_NOT_FOUND, 0, 0, trail->pool,
+ "entry not found: filesystem `%s', entry `%s'",
+ parent->fs->env_path, name);
+
+ child->fs = parent->fs;
+ SVN_ERR (svn_fs__get_rep (&child->contents, child->fs, child->id, trail));
+
+ child->pool = trail->pool;
+ *child_p = child;
+
+ return SVN_NO_ERROR;
}
svn_error_t *svn_fs__dag_delete (dag_node_t *parent,
--
Yoshiki Hayashi
Re: [PATCH] svn_fs__open_dag
Posted by Karl Fogel <kf...@galois.collab.net>.
Greg Stein <gs...@lyra.org> writes:
> Ah, yes. It was renamed recently, and my working copy still showed it as
> is_atom.
>
> You're right: svn_fs__matches_atom()
Note that svn_fs__matches_atom() is not for testing whether or not a
skel is an atom, it is for testing whether it is an atom matching a
given string.
To simply test whether it is an atom, just look at skel->is_atom.
-K
Re: [PATCH] svn_fs__open_dag
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 22, 2001 at 04:11:44PM +0900, Yoshiki Hayashi wrote:
> Greg Stein <gs...@lyra.org> writes:
> > > One more patch. This fixes the same bug GregS pointed out
> > > to similar code.
> >
> > Actually, the right choice is to use svn_fs__is_atom() :-)
>
> Ah, you mean svn_fs__matches_atom (). I grepped is_atom
> instead of looking at skel.h and concluded it is not
> implemented yet.
Ah, yes. It was renamed recently, and my working copy still showed it as
is_atom.
You're right: svn_fs__matches_atom()
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] svn_fs__open_dag, take 3
Posted by Jim Blandy <ji...@zwingli.cygnus.com>.
Yoshiki Hayashi <yo...@xemacs.org> writes:
> +dag_node_t *
> +svn_fs__dag_dup (dag_node_t *node,
> + trail_t *trail)
> +{
> + dag_node_t *dag_dup = apr_palloc (trail->pool, sizeof (*node));
> + memcpy (dag_dup, node, sizeof (*node));
> + return dag_dup;
> +}
The duplicate must have its pool set correctly, and its ID must be
allocated in its own pool.
Mike's committed his own definition for this function, which had the
same problem with the ID; I'll commit a fix for it shortly, along with
the content caching stuff.
Re: [PATCH] svn_fs__open_dag, take 3
Posted by Karl Fogel <kf...@galois.collab.net>.
Jim Blandy writes, regarding the svn_fs__dag_open patch:
> Shouldn't you call find_dir_entry on parent before you verify that
> it's a directory?
+1 (Clarification: I think Jim meant to write "after", not "before".)
Re: [PATCH] svn_fs__open_dag, take 3
Posted by Jim Blandy <ji...@zwingli.cygnus.com>.
Yoshiki Hayashi <yo...@xemacs.org> writes:
> -svn_error_t *svn_fs__dag_open (dag_node_t **child_p,
> - dag_node_t *parent,
> - const char *name,
> - trail_t *trail)
> +svn_error_t *
> +svn_fs__dag_open (dag_node_t **child_p,
> + dag_node_t *parent,
> + const char *name,
> + trail_t *trail)
> {
> - abort();
> - /* NOTREACHED */
> - return NULL;
> -}
> + skel_t *entry = find_dir_entry (parent, name);
> + dag_node_t *child = apr_pcalloc (trail->pool, sizeof (*child));
> +
> + if (! svn_fs__dag_is_directory (parent))
> + {
> + svn_string_t *unparsed_id = svn_fs_unparse_id (parent->id, trail->pool);
> + return
> + svn_error_createf
> + (SVN_ERR_FS_NOT_DIRECTORY, 0, NULL, parent->pool,
> + "Node revision `%s' is not a directory in filesystem `%s'",
> + unparsed_id->data, parent->fs->env_path);
> + }
> +
> + /* ### Turn this into a separate function? */
> +
> + /* Verify NAME is a valid entry. */
> + if (! strcmp (name, ".") || ! strcmp (name, "..") || ! strcmp (name, ""))
> + return svn_fs__err_path_syntax (parent->fs, name);
>
> + if (entry)
> + {
> + /* Entry is (NAME ID) */
> + skel_t *id = entry->children->next;
> + child->id = svn_fs_parse_id (id->data, id->len, trail->pool);
> + }
> + else
> + return svn_fs__err_no_such_entry (parent->fs, parent->id, name);
> +
> + child->fs = parent->fs;
> + SVN_ERR (svn_fs__get_node_revision (&child->contents,
> + child->fs, child->id, trail));
> +
> + child->pool = trail->pool;
> + *child_p = child;
> +
> + return SVN_NO_ERROR;
> +}
Shouldn't you call find_dir_entry on parent before you verify that
it's a directory?
Shouldn't you verify that NAME is valid before you try to look it up
in the directory? Granted, directories should never contain invalid
names, but this is a bit subtle, and I'm not sure everyone will notice
that before changing the code in the future. I think it's generally
best to validate the input before doing anything with it.
> svn_error_t *svn_fs__dag_delete (dag_node_t *parent,
> Index: err.c
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_fs/err.c,v
> retrieving revision 1.24
> diff -u -r1.24 err.c
> --- err.c 2001/02/15 23:02:13 1.24
> +++ err.c 2001/02/23 07:28:16
> @@ -238,7 +238,18 @@
> path, fs->env_path);
> }
>
> +svn_error_t *
> +svn_fs__err_no_such_entry (svn_fs_t *fs,
> + const svn_fs_id_t *id,
> + const char *name)
> +{
> + svn_string_t *unparsed_id = svn_fs_unparse_id (id, fs->pool);
>
> + return svn_error_createf
> + (SVN_ERR_FS_NO_SUCH_ENTRY, 0, 0, fs->pool,
> + "No entry named `%s' in filesystem `%s', directory revision `%s'",
> + name, fs->env_path, unparsed_id->data);
> +}
>
>
> /*
> Index: err.h
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_fs/err.h,v
> retrieving revision 1.20
> diff -u -r1.20 err.h
> --- err.h 2001/02/15 23:02:13 1.20
> +++ err.h 2001/02/23 07:28:16
> @@ -133,6 +133,9 @@
> /* SVN_ERR_FS_NOT_DIRECTORY: PATH does not refer to a directory in FS. */
> svn_error_t *svn_fs__err_not_directory (svn_fs_t *fs, const char *path);
>
> +/* SVN_ERR_FS_NO_SUCH_ENTRY: NAME does not exists in directory ID in FS. */
> +svn_error_t *svn_fs__err_no_such_entry (svn_fs_t *fs, const svn_fs_id_t *id,
> + const char *name);
>
> #endif /* SVN_LIBSVN_FS_ERR_H */
You should use SVN_ERR_FS_NOT_FOUND, which already exists.
[PATCH] svn_fs__open_dag, take 3
Posted by Yoshiki Hayashi <yo...@xemacs.org>.
Jim Blandy <ji...@zwingli.cygnus.com> writes:
[...]
Thanks for detailed review. This is an updated patch.
(svn_fs__dag_dup): New function.
(svn_fs__dag_open): Implement it.
(svn_fs__err_no_such_entry): New function.
Index: dag.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/dag.c,v
retrieving revision 1.35
diff -u -r1.35 dag.c
--- dag.c 2001/02/23 06:16:20 1.35
+++ dag.c 2001/02/23 07:28:16
@@ -11,6 +11,10 @@
* ====================================================================
*/
+#define APR_WANT_STRFUNC
+#define APR_WANT_MEMFUNC
+#include "apr_want.h"
+
#include "svn_path.h"
#include "svn_fs.h"
#include "dag.h"
@@ -107,6 +111,15 @@
*/
+dag_node_t *
+svn_fs__dag_dup (dag_node_t *node,
+ trail_t *trail)
+{
+ dag_node_t *dag_dup = apr_palloc (trail->pool, sizeof (*node));
+ memcpy (dag_dup, node, sizeof (*node));
+ return dag_dup;
+}
+
const svn_fs_id_t *svn_fs__dag_get_id (dag_node_t *node)
{
return node->id;
@@ -402,16 +415,49 @@
}
-svn_error_t *svn_fs__dag_open (dag_node_t **child_p,
- dag_node_t *parent,
- const char *name,
- trail_t *trail)
+svn_error_t *
+svn_fs__dag_open (dag_node_t **child_p,
+ dag_node_t *parent,
+ const char *name,
+ trail_t *trail)
{
- abort();
- /* NOTREACHED */
- return NULL;
-}
+ skel_t *entry = find_dir_entry (parent, name);
+ dag_node_t *child = apr_pcalloc (trail->pool, sizeof (*child));
+
+ if (! svn_fs__dag_is_directory (parent))
+ {
+ svn_string_t *unparsed_id = svn_fs_unparse_id (parent->id, trail->pool);
+ return
+ svn_error_createf
+ (SVN_ERR_FS_NOT_DIRECTORY, 0, NULL, parent->pool,
+ "Node revision `%s' is not a directory in filesystem `%s'",
+ unparsed_id->data, parent->fs->env_path);
+ }
+
+ /* ### Turn this into a separate function? */
+
+ /* Verify NAME is a valid entry. */
+ if (! strcmp (name, ".") || ! strcmp (name, "..") || ! strcmp (name, ""))
+ return svn_fs__err_path_syntax (parent->fs, name);
+ if (entry)
+ {
+ /* Entry is (NAME ID) */
+ skel_t *id = entry->children->next;
+ child->id = svn_fs_parse_id (id->data, id->len, trail->pool);
+ }
+ else
+ return svn_fs__err_no_such_entry (parent->fs, parent->id, name);
+
+ child->fs = parent->fs;
+ SVN_ERR (svn_fs__get_node_revision (&child->contents,
+ child->fs, child->id, trail));
+
+ child->pool = trail->pool;
+ *child_p = child;
+
+ return SVN_NO_ERROR;
+}
svn_error_t *svn_fs__dag_delete (dag_node_t *parent,
Index: err.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/err.c,v
retrieving revision 1.24
diff -u -r1.24 err.c
--- err.c 2001/02/15 23:02:13 1.24
+++ err.c 2001/02/23 07:28:16
@@ -238,7 +238,18 @@
path, fs->env_path);
}
+svn_error_t *
+svn_fs__err_no_such_entry (svn_fs_t *fs,
+ const svn_fs_id_t *id,
+ const char *name)
+{
+ svn_string_t *unparsed_id = svn_fs_unparse_id (id, fs->pool);
+ return svn_error_createf
+ (SVN_ERR_FS_NO_SUCH_ENTRY, 0, 0, fs->pool,
+ "No entry named `%s' in filesystem `%s', directory revision `%s'",
+ name, fs->env_path, unparsed_id->data);
+}
/*
Index: err.h
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/err.h,v
retrieving revision 1.20
diff -u -r1.20 err.h
--- err.h 2001/02/15 23:02:13 1.20
+++ err.h 2001/02/23 07:28:16
@@ -133,6 +133,9 @@
/* SVN_ERR_FS_NOT_DIRECTORY: PATH does not refer to a directory in FS. */
svn_error_t *svn_fs__err_not_directory (svn_fs_t *fs, const char *path);
+/* SVN_ERR_FS_NO_SUCH_ENTRY: NAME does not exists in directory ID in FS. */
+svn_error_t *svn_fs__err_no_such_entry (svn_fs_t *fs, const svn_fs_id_t *id,
+ const char *name);
#endif /* SVN_LIBSVN_FS_ERR_H */
--
Yoshiki Hayashi
Re: [PATCH] svn_fs__open_dag
Posted by Jim Blandy <ji...@zwingli.cygnus.com>.
Yoshiki Hayashi <yo...@xemacs.org> writes:
> -svn_error_t *svn_fs__dag_open (dag_node_t **child_p,
> - dag_node_t *parent,
> - const char *name,
> - trail_t *trail)
> -{
> - abort();
> - /* NOTREACHED */
> - return NULL;
> +svn_error_t *
> +svn_fs__dag_open (dag_node_t **child_p,
> + dag_node_t *parent,
> + const char *name,
> + trail_t *trail)
> +{
> + /* (HEADER ENTRY ...) */
> + skel_t *entry = parent->contents->next;
> + dag_node_t *child = apr_palloc (trail->pool, sizeof (*child));
You should verify that PARENT is indeed a directory, and return
an SVN_FS_NOT_DIRECTORY error. You should also verify that NAME is a
valid directory entry --- not '', '.', or '..'. I had a function
which would even check that it was a valid UTF-8 encoding, but I don't
know where it went.
> + while (entry)
> + {
> + if (entry->is_atom)
> + return svn_fs__err_corrupt_node_revision (parent->fs, parent->id);
In theory, node-rev.h has ensured that the skel is a well-formed
NODE-REVISION, so these checks are unnecessary. If you want to do
them, however, you might find svn_fs__list_length useful. It returns
-1 on atoms, and otherwise returns the list's length, so something
like:
if (svn_fs__list_length (skel) == 3)
verifies that skel is a list, and that it is of length 3.
(As a very minor point, you might want to use `for' for linked list
traversals: something like `for (PTR = HEAD; PTR; PTR = PTR-next)'
puts the general facts of the traversal all in one place. Not every
traversal fits the pattern, but when it does, I think it's nice to put
it in this form.)
> + /* (NAME ID) */
> + if (svn_fs__matches_atom (entry->children, name))
> + {
> + skel_t *id = entry->children->next;
> + child->id = svn_fs_parse_id (id->data, id->len, trail->pool);
> + break;
> + }
> +
> + entry = entry->next;
> + }
> +
> + if (! child->id)
> + return svn_fs__err_no_such_entry (parent->fs, parent->id, name);
You never ensured that child->id is zero at the top of the loop. You
should probably use apr_pcalloc.
> + child->fs = parent->fs;
> + SVN_ERR (svn_fs__get_rep (&child->contents, child->fs, child->id, trail));
You want to call svn_fs__get_node_revision here. `get_rep' gives you
the REPRESENTATION skel, which could be in either fulltext or
deltified form. get_node_revision handles all that for you.
> +
> + child->pool = trail->pool;
> + *child_p = child;
> +
> + return SVN_NO_ERROR;
> }
The err.[ch] changes are correct, no surprise.
Re: [PATCH] svn_fs__open_dag
Posted by Yoshiki Hayashi <yo...@xemacs.org>.
Greg Stein <gs...@lyra.org> writes:
> > One more patch. This fixes the same bug GregS pointed out
> > to similar code.
>
> Actually, the right choice is to use svn_fs__is_atom() :-)
Ah, you mean svn_fs__matches_atom (). I grepped is_atom
instead of looking at skel.h and concluded it is not
implemented yet.
> What is the svn_fs__dag_dup function for? I didn't see it used anywhere. I'm
> not sure if "dup" makes sense (rather than multiple references), so you may
> want to omit that function until a need arises.
I'm not sure why dup is necessary but it's called from
tree.c L201, function root_node.
> Also, note that most FS errors are generated by functions in err.c. This
> would include a "corrupt <whatever>" rather than using abort(). The "not
> found" would also go into err.c
Thanks. I have much to learn. I'm not sure which error is
used here but I hope this one does things right.
(svn_fs__dag_dup): New function.
(svn_fs__dag_open): Implement it.
(svn_fs__err_no_such_entry): New function.
Index: dag.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/dag.c,v
retrieving revision 1.27
diff -u -r1.27 dag.c
--- dag.c 2001/02/21 23:28:27 1.27
+++ dag.c 2001/02/22 07:05:26
@@ -103,6 +103,15 @@
*/
+dag_node_t *
+svn_fs__dag_dup (dag_node_t *node,
+ trail_t *trail)
+{
+ dag_node_t *dag_dup = apr_palloc (trail->pool, sizeof (*node));
+ memcpy (dag_dup, node, sizeof (*node));
+ return dag_dup;
+}
+
const svn_fs_id_t *svn_fs__dag_get_id (dag_node_t *node)
{
return node->id;
@@ -159,7 +168,7 @@
itself a list. */
skel_t *header = node->contents->children;
- /* The 3nd element of the header, IF it exists, is the header's
+ /* The 3rd element of the header, IF it exists, is the header's
first `flag'. It could be NULL. */
skel_t *flag = header->children->next->next;
@@ -288,14 +297,42 @@
}
-svn_error_t *svn_fs__dag_open (dag_node_t **child_p,
- dag_node_t *parent,
- const char *name,
- trail_t *trail)
-{
- abort();
- /* NOTREACHED */
- return NULL;
+svn_error_t *
+svn_fs__dag_open (dag_node_t **child_p,
+ dag_node_t *parent,
+ const char *name,
+ trail_t *trail)
+{
+ /* (HEADER ENTRY ...) */
+ skel_t *entry = parent->contents->next;
+ dag_node_t *child = apr_palloc (trail->pool, sizeof (*child));
+
+ while (entry)
+ {
+ if (entry->is_atom)
+ return svn_fs__err_corrupt_node_revision (parent->fs, parent->id);
+
+ /* (NAME ID) */
+ if (svn_fs__matches_atom (entry->children, name))
+ {
+ skel_t *id = entry->children->next;
+ child->id = svn_fs_parse_id (id->data, id->len, trail->pool);
+ break;
+ }
+
+ entry = entry->next;
+ }
+
+ if (! child->id)
+ return svn_fs__err_no_such_entry (parent->fs, parent->id, name);
+
+ child->fs = parent->fs;
+ SVN_ERR (svn_fs__get_rep (&child->contents, child->fs, child->id, trail));
+
+ child->pool = trail->pool;
+ *child_p = child;
+
+ return SVN_NO_ERROR;
}
svn_error_t *svn_fs__dag_delete (dag_node_t *parent,
Index: err.c
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/err.c,v
retrieving revision 1.24
diff -u -r1.24 err.c
--- err.c 2001/02/15 23:02:13 1.24
+++ err.c 2001/02/22 07:05:26
@@ -238,7 +238,18 @@
path, fs->env_path);
}
+svn_error_t *
+svn_fs__err_no_such_entry (svn_fs_t *fs,
+ const svn_fs_id_t *id,
+ const char *name)
+{
+ svn_string_t *unparsed_id = svn_fs_unparse_id (id, fs->pool);
+ return svn_error_createf
+ (SVN_ERR_FS_NOT_FOUND, 0, 0, fs->pool,
+ "No entry named `%s' in filesystem `%s', directory revision `%s'",
+ name, fs->env_path, unparsed_id->data);
+}
/*
Index: err.h
===================================================================
RCS file: /cvs/subversion/subversion/libsvn_fs/err.h,v
retrieving revision 1.20
diff -u -r1.20 err.h
--- err.h 2001/02/15 23:02:13 1.20
+++ err.h 2001/02/22 07:05:27
@@ -133,6 +133,9 @@
/* SVN_ERR_FS_NOT_DIRECTORY: PATH does not refer to a directory in FS. */
svn_error_t *svn_fs__err_not_directory (svn_fs_t *fs, const char *path);
+/* SVN_ERR_FS_NOT_FOUND: NAME does not exists in directory ID in FS. */
+svn_error_t *svn_fs__err_no_such_entry (svn_fs_t *fs, const svn_fs_id_t *id,
+ const char *name);
#endif /* SVN_LIBSVN_FS_ERR_H */
--
Yoshiki Hayashi
Re: [PATCH] svn_fs__open_dag
Posted by Jim Blandy <ji...@zwingli.cygnus.com>.
Greg Stein <gs...@lyra.org> writes:
> What is the svn_fs__dag_dup function for? I didn't see it used anywhere. I'm
> not sure if "dup" makes sense (rather than multiple references), so you may
> want to omit that function until a need arises.
It's used in tree.c, no?
Re: [PATCH] svn_fs__open_dag
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 22, 2001 at 01:35:33PM +0900, Yoshiki Hayashi wrote:
> Jim Blandy <ji...@zwingli.cygnus.com> writes:
>
> > Great! I'll check it over tomorrow...
>
> One more patch. This fixes the same bug GregS pointed out
> to similar code.
Actually, the right choice is to use svn_fs__is_atom() :-)
What is the svn_fs__dag_dup function for? I didn't see it used anywhere. I'm
not sure if "dup" makes sense (rather than multiple references), so you may
want to omit that function until a need arises.
Also, note that most FS errors are generated by functions in err.c. This
would include a "corrupt <whatever>" rather than using abort(). The "not
found" would also go into err.c
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] svn_fs__open_dag
Posted by Yoshiki Hayashi <yo...@xemacs.org>.
Jim Blandy <ji...@zwingli.cygnus.com> writes:
> Great! I'll check it over tomorrow...
One more patch. This fixes the same bug GregS pointed out
to similar code.
> > I have one question with regard to svn_fs__dag_close which
> > was just deleted. It's still referenced by tree.c and I'd
> > like to resurrect it. It will call just apr_pool_destroy
> > (dag->pool) if I understand it correctly. The problem of
> > current implementation is that dag->pool points to
> > trail->pool. Shouldn't dag->pool be subpool of trail->pool?
>
> Your understanding is correct. We decided that it wasn't important
> enough to free dag nodes before their underlying pools were freed.
> What would you like to do with svn_fs__dag_close, other than free
> memory?
Hmm. I'd say none. It seems somehow I thought dag_node_t
can be big. But it only mainains reference to node-revision
skel...
--- dag.c.1 Thu Feb 22 13:15:11 2001
+++ dag.c Thu Feb 22 13:22:27 2001
@@ -313,7 +313,8 @@
if (entry->is_atom || ! entry->children->is_atom)
abort ();
- if (! memcmp (entry->children->data, name, entry->children->len))
+ if (entry->children->len == strlen (name)
+ && ! memcmp (entry->children->data, name, entry->children->len))
{
skel_t *id = entry->children->next;
child->id = svn_fs_parse_id (id->data, id->len, trail->pool);
--
Yoshiki Hayashi
Re: [PATCH] svn_fs__open_dag
Posted by Jim Blandy <ji...@zwingli.cygnus.com>.
Yoshiki Hayashi <yo...@xemacs.org> writes:
> I wanted to be more familiar with libsvn_fs code so I
> started writing part of it. But be careful! This is my
> first real function in subversion. You might want to
> rewrite it from scratch. It does compile and I think it's
> correct, though.
Great! I'll check it over tomorrow...
> I have one question with regard to svn_fs__dag_close which
> was just deleted. It's still referenced by tree.c and I'd
> like to resurrect it. It will call just apr_pool_destroy
> (dag->pool) if I understand it correctly. The problem of
> current implementation is that dag->pool points to
> trail->pool. Shouldn't dag->pool be subpool of trail->pool?
Your understanding is correct. We decided that it wasn't important
enough to free dag nodes before their underlying pools were freed.
What would you like to do with svn_fs__dag_close, other than free
memory?
> Oh, the downside of different timezone is that we can't
> communicate in real time basis. :-)
Unless one is annoying one's family by staying up late and reading
mail... :)