On Fri, Feb 18, 2011 at 03:21:02PM -0600, Alex Elder wrote:
> Move the handling of duplicate names into its own function. As a
> result, all names other than "lost+found" files (not just those that
> get obfuscated) will be checked to avoid duplication.
>
> This makes the local buffer newname[] in generate_obfuscated_name()
> unnecessary, so just drop it and use the passed-in name.
>
> Signed-off-by: Alex Elder <aelder@xxxxxxx>
>
> This is a new change, not posted with this series previously.
Couple of minor comments below.
>
> ---
> db/metadump.c | 95
> +++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 49 insertions(+), 46 deletions(-)
>
> Index: b/db/metadump.c
> ===================================================================
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -469,6 +469,37 @@ in_lost_found(
> }
>
> /*
> + * Look up the given name in the name table. If it is already
> + * present, find an alternate and attempt to use that name instead.
> + *
> + * Returns 1 if the (possibly modified) name is not present in the
> + * name table. Returns 0 otherwise.
> + */
> +static int
> +handle_duplicates(xfs_dahash_t hash, size_t name_len, uchar_t *name)
handle_duplicate_name()?
> +{
> + int dup = 0;
> +
> + if (!nametable_find(hash, name_len, name))
> + return 1; /* Not already in table */
> +
> + /* Name is already in use. Need to find an alternate. */
> +
> + do {
> + obfuscate_name(hash, name_len, name);
> +
> + /*
> + * Search the name table to be sure we don't produce
> + * a name that's already been used.
> + */
> + if (!nametable_find(hash, name_len, name))
> + break;
> + } while (++dup < DUP_MAX);
> +
> + return dup < DUP_MAX ? 1 : 0;
> +}
> +
> +/*
> * Given a name and its hash value, massage the name in such a way
> * that the result is another name of equal length which shares the
> * same hash value.
> @@ -549,9 +580,6 @@ generate_obfuscated_name(
> uchar_t *name)
> {
> xfs_dahash_t hash;
> - int dup = 0;
> - uchar_t newname[NAME_MAX];
> - uchar_t *newp;
>
> /*
> * We don't obfuscate "lost+found" or any orphan files
> @@ -562,58 +590,33 @@ generate_obfuscated_name(
> if (ino && in_lost_found(ino, namelen, name))
> return;
>
> - /*
> - * If the name starts with a slash, just skip over it. We
> - * will copy our obfuscated name back into space following
> - * the slash when we're done. Our new name will not have
> - * the '/', and that's the version we'll keep in our
> - * duplicates table. Note that the namelen value passed in
> - * does not include the leading slash (if any).
> - */
> if (*name == '/')
> - name++;
> + name++; /* Skip over leading slash (if any). */
I think the comment explaining _why_ we are skipping over the slash
is much more important than the new comment that just describes what
the code is doing. Hence I'd prefer a slightly modified version of
the old comment rather than removing it altogether...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|