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)
> +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.
Received on Sat Oct 21 14:36:23 2006