On 6/29/07, Erik Huelsmann <ehuels@gmail.com> wrote:
> Does anybody have any objections to me committing the patch below? It
> slightly changes the semantics of svn_io_get_dirents(), but in a way
> to become consistent with svn_io_check_path() *and* the old behaviour
> wasn't documented anyway.
>
> Log:
> [[[
> Factor out common code.
>
> * subversion/include/svn_io.h
> (svn_io_get_dirents2): Document that this function behaves consistently
> with svn_io_check_path().
>
> * subversion/libsvn_subr/io.c
> (map_apr_finfo_to_node_kind): New. Contains the common code.
> (svn_io_check_path,
> svn_io_get_dirents2): Adjust to call factored out function.
> ]]]
>
> Index: subversion/include/svn_io.h
> ===================================================================
> --- subversion/include/svn_io.h (revision 25563)
> +++ subversion/include/svn_io.h (working copy)
> @@ -1,7 +1,7 @@
> /**
> * @copyright
> * ====================================================================
> - * Copyright (c) 2000-2006 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2007 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -808,6 +808,9 @@
> * @note The `.' and `..' directories normally returned by
> * apr_dir_read() are NOT returned in the hash.
> *
> + * @note The kind field in the @a dirents is set according to the mapping
> + * as documented for svn_io_check_path()
> + *
> * @since New in 1.3.
> */
> svn_error_t *svn_io_get_dirents2(apr_hash_t **dirents,
> Index: subversion/libsvn_subr/io.c
> ===================================================================
> --- subversion/libsvn_subr/io.c (revision 25563)
> +++ subversion/libsvn_subr/io.c (working copy)
> @@ -89,6 +89,26 @@
> #endif
>
>
> +static void
> +map_apr_finfo_to_node_kind(svn_node_kind_t *kind,
> + svn_boolean_t *is_special,
> + apr_finfo_t *finfo)
> +{
> + *is_special = FALSE;
> +
> + if (finfo->filetype == APR_REG)
> + *kind = svn_node_file;
> + else if (finfo->filetype == APR_DIR)
> + *kind = svn_node_dir;
> + else if (finfo->filetype == APR_LNK)
> + {
> + *is_special = TRUE;
> + *kind = svn_node_file;
> + }
> + else
> + *kind = svn_node_unknown;
> +}
> +
> /* Helper for svn_io_check_path() and svn_io_check_resolved_path();
> essentially the same semantics as those two, with the obvious
> interpretation for RESOLVE_SYMLINKS. */
> @@ -114,7 +134,7 @@
>
> flags = resolve_symlinks ? APR_FINFO_MIN : (APR_FINFO_MIN | APR_FINFO_LINK);
> apr_err = apr_stat(&finfo, path_apr, flags, pool);
> -
> +
> if (APR_STATUS_IS_ENOENT(apr_err))
> *kind = svn_node_none;
> else if (APR_STATUS_IS_ENOTDIR(apr_err))
> @@ -122,22 +142,10 @@
> else if (apr_err)
> return svn_error_wrap_apr(apr_err, _("Can't check path '%s'"),
> svn_path_local_style(path, pool));
> - else if (finfo.filetype == APR_NOFILE)
> - *kind = svn_node_unknown;
> - else if (finfo.filetype == APR_REG)
> - *kind = svn_node_file;
> - else if (finfo.filetype == APR_DIR)
> - *kind = svn_node_dir;
> - else if (finfo.filetype == APR_LNK)
> - {
> - is_special = TRUE;
> - *kind = svn_node_file;
> - }
> - else
> - *kind = svn_node_unknown;
>
> + map_apr_finfo_to_node_kind(kind, &is_special, &finfo);
That should have been:
> + else
> + map_apr_finfo_to_node_kind(kind, &is_special, &finfo);
ofcourse. Other comments?
> *is_special_p = is_special;
> -
> +
> return SVN_NO_ERROR;
> }
>
> @@ -1922,25 +1930,14 @@
> else
> {
> const char *name;
> - svn_io_dirent_t *dirent = apr_pcalloc(pool, sizeof(*dirent));
> + svn_io_dirent_t *dirent = apr_palloc(pool, sizeof(*dirent));
>
> SVN_ERR(svn_path_cstring_to_utf8(&name, this_entry.name, pool));
> -
> - if (this_entry.filetype == APR_REG)
> - dirent->kind = svn_node_file;
> - else if (this_entry.filetype == APR_DIR)
> - dirent->kind = svn_node_dir;
> - else if (this_entry.filetype == APR_LNK)
> - {
> - dirent->kind = svn_node_file;
> - dirent->special = TRUE;
> - }
> - else
> - /* ### Currently, Subversion supports just symlinks; other
> - * entry types are reported as regular files. This is inconsistent
> - * with svn_io_check_path(). */
> - dirent->kind = svn_node_file;
>
> + map_apr_finfo_to_node_kind(&(dirent->kind),
> + &(dirent->special),
> + &this_entry);
> +
> apr_hash_set(*dirents, name, APR_HASH_KEY_STRING, dirent);
> }
> }
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 29 01:18:19 2007