Pádraig Brady wrote:
> This is for use in a proposed coreutils `realpath` command.
> Specifically by these options:
>
> -L, --logical resolve `..' components before symlinks
> -s, --strip don't expand symlinks
...
Thanks. This looks good.
Some nits:
> Subject: [PATCH] canonicalize: add support for not resolving symlinks
>
> This will initially be used by a new coreutils realpath command.
>
> * lib/canonicalize.h: Add the CAN_NOLINKS flag to
> indicate we don't want to follow symlinks. Also
> provide CAN_MODE_MASK to aid setting these existing
> mutually exclusive values.
> * lib/canonicalize.c (canonicalize_filename_mode):
> Extract the flags from can_mode parameter, which
> are currently just used to select between stat()
> and lstat(). Also ensure that mutually exclusive
> values and flagged immediately as invalid.
s/and/are/
> * tests/test-canonicalize.c: Verify symlinks are
> not followed, and that invalid flag combinations
> are diagnosed.
> ---
> ChangeLog | 16 ++++++++++++++++
> lib/canonicalize.c | 18 +++++++++++++++---
> lib/canonicalize.h | 10 +++++++++-
> tests/test-canonicalize.c | 12 ++++++++++++
> 4 files changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index cc50b4a..06dfe77 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,19 @@
> +2011-12-29 Pádraig Brady <p...@draigbrady.com>
> +
> + canonicalize: add support for not resolving symlinks
> + * lib/canonicalize.h: Add the CAN_NOLINKS flag to
> + indicate we don't want to follow symlinks. Also
> + provide CAN_MODE_MASK to aid setting these existing
> + mutually exclusive values.
> + * lib/canonicalize.c (canonicalize_filename_mode):
> + Extract the flags from can_mode parameter, which
> + are currently just used to select between stat()
> + and lstat(). Also ensure that mutually exclusive
> + values and flagged immediately as invalid.
s/and/are/
> + * tests/test-canonicalize.c: Verify symlinks are
> + not followed, and that invalid flag combinations
> + are diagnosed.
> +
> 2011-12-25 Jim Meyering <meyer...@redhat.com>
>
> gitlog-to-changelog: do not clump multi-paragraph entries
> diff --git a/lib/canonicalize.c b/lib/canonicalize.c
> index 4fe9f30..c73a963 100644
> --- a/lib/canonicalize.c
> +++ b/lib/canonicalize.c
> @@ -31,6 +31,8 @@
> #include "xalloc.h"
> #include "xgetcwd.h"
>
> +#define MULTIPLE_BITS_SET(i) (((i) & ((i) - 1)) != 0)
> +
> /* In this file, we cannot handle file names longer than PATH_MAX.
> On systems with no file name length limit, use a fallback. */
> #ifndef PATH_MAX
> @@ -82,8 +84,9 @@ seen_triple (Hash_table **ht, char const *filename, struct
> stat const *st)
> /* Return the canonical absolute name of file NAME, while treating
> missing elements according to CAN_MODE. A canonical name
> does not contain any `.', `..' components nor any repeated file name
> - separators ('/') or symlinks. Whether components must exist
> - or not depends on canonicalize mode. The result is malloc'd. */
> + separators ('/') or, depdending on other CAN_MODE flags, symlinks.
> + Whether components must exist or not depends on canonicalize mode.
> + The result is malloc'd. */
>
> char *
> canonicalize_filename_mode (const char *name, canonicalize_mode_t can_mode)
> @@ -95,6 +98,15 @@ canonicalize_filename_mode (const char *name,
> canonicalize_mode_t can_mode)
> size_t extra_len = 0;
> Hash_table *ht = NULL;
> int saved_errno;
> + int can_flags = can_mode & ~CAN_MODE_MASK;
> + can_mode &= CAN_MODE_MASK;
> + bool logical = can_flags & CAN_NOLINKS;
> +
> + if (MULTIPLE_BITS_SET (can_mode))
> + {
> + errno = EINVAL;
> + return NULL;
> + }
>
> if (name == NULL)
> {
> @@ -185,7 +197,7 @@ canonicalize_filename_mode (const char *name,
> canonicalize_mode_t can_mode)
> dest += end - start;
> *dest = '\0';
>
> - if (lstat (rname, &st) != 0)
> + if ((logical?stat:lstat) (rname, &st) != 0)
Please add spaces around operators:
if ((logical ? stat : lstat) (rname, &st) != 0)
> {
> saved_errno = errno;
> if (can_mode == CAN_EXISTING)
> diff --git a/lib/canonicalize.h b/lib/canonicalize.h
> index 04ad79c..4bb5898 100644
> --- a/lib/canonicalize.h
> +++ b/lib/canonicalize.h
> @@ -19,6 +19,8 @@
>
> #include <stdlib.h> /* for canonicalize_file_name */
>
> +#define CAN_MODE_MASK (CAN_EXISTING | CAN_ALL_BUT_LAST | CAN_MISSING)
> +
> enum canonicalize_mode_t
> {
> /* All components must exist. */
> @@ -28,7 +30,13 @@ enum canonicalize_mode_t
> CAN_ALL_BUT_LAST = 1,
>
> /* No requirements on components existence. */
> - CAN_MISSING = 2
> + CAN_MISSING = 2,
> +
> + /* Don't expand symlinks. */
> + CAN_NOLINKS = 4,
> +
> + /* Modify the original (Not yet supported). */
> + CAN_NOALLOC = 8
Would it be better to add it along with the
change that adds support?
> };
> typedef enum canonicalize_mode_t canonicalize_mode_t;
...
