Yoshiki Hayashi <yoshiki@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.
Received on Sat Oct 21 14:36:23 2006