On Fri, Feb 18, 2011 at 03:21:01PM -0600, Alex Elder wrote:
> Move the check for short names out of is_special_dirent() and into
> generate_obfuscated_name(). That way the check is more directly
> associated with the algorithm that requires it.
>
> Similarly, move the check for inode == 0, since that case has to do
> with storing extended attributes (not files) in the name table.
>
> As a result, is_special_dirent() is really only focused on whether a
> given file is in the lost+found directory. Rename the function to
> reflect its more specific purpose.
>
> Signed-off-by: Alex Elder <aelder@xxxxxxx>
>
> Updated:
> - The previous version did not properly skip the "lost+found"
> directory itself; this one does.
> - Created a new definition representing the name of the orphanage
> directory. Encapsulate recognizing that directory into a new
> macro, is_lost_found().
> - Removed casts that eliminate a compile warning in calls to
> libxfs_da_hashname(); will do them separately later if needed.
>
> ---
> db/metadump.c | 76 +++++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 52 insertions(+), 24 deletions(-)
>
> Index: b/db/metadump.c
> ===================================================================
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2007 Silicon Graphics, Inc.
> + * Copyright (c) 2007, 2011 SGI
> * All Rights Reserved.
> *
> * This program is free software; you can redistribute it and/or
> @@ -377,40 +377,56 @@ random_filename_char(void)
> return c;
> }
>
> +/*
> + * Determine whether a name is one we shouldn't obfuscate because
> + * it's an orphan (or the "lost+found" directory itself). Note
> + * "cur_ino" is the inode for the directory currently being
> + * processed.
> + *
> + * Returns 1 if the name should NOT be obfuscated or 0 otherwise.
> + */
> +#define is_lost_found(mnt, dir_ino, nmlen, nm) \
> + ((dir_ino) == (mnt)->m_sb.sb_rootino && \
> + (nmlen) == ORPHANAGE_LEN && \
> + !memcmp((nm), ORPHANAGE, ORPHANAGE_LEN))
Perhaps a static inline function?
> +
> +#define ORPHANAGE "lost+found"
> +#define ORPHANAGE_LEN (sizeof ORPHANAGE - 1)
sizeof works without ()? Even it is does, it is unusual to do so,
and a little ambiguous....
> +
> static int
> -is_special_dirent(
> +in_lost_found(
Oh, that confused me for a second - in_lost_found and is_lost_found
are very similar in name, hence easily confused when scanning the
code. Not sure how better to name them, maybe you've got a better
idea, Alex?
> xfs_ino_t ino,
> int namelen,
> uchar_t *name)
> {
> static xfs_ino_t orphanage_ino = 0;
> - char s[32];
> + char s[24]; /* 21 is enough */
Why is 21 enough?
> int slen;
>
> - /*
> - * due to the XFS name hashing algorithm, we cannot obfuscate
> - * names with 4 chars or less.
> - */
> - if (namelen <= 4)
> - return 1;
> + /* Record the "lost+found" inode if we haven't done so already */
>
> - if (ino == 0)
> + ASSERT(ino != 0);
> + if (!orphanage_ino && is_lost_found(mp, cur_ino, namelen, name))
> + orphanage_ino = ino;
> +
> + /* We don't obfuscate the "lost+found" directory itself */
> +
> + if (ino == orphanage_ino)
> + return 1;
> +
> + /* Most files aren't in "lost+found" at all */
> +
> + if (cur_ino != orphanage_ino)
> return 0;
I'm judging by this that if a directory tree is attached to
lost+found we are obfuscating anything in that subdirectory?
>
> /*
> - * don't obfuscate lost+found nor any inodes within lost+found with
> - * the inode number
> + * Within "lost+found", we don't obfuscate any file whose
> + * name is the same as its inode number. Any others are
> + * stray files and can be obfuscated.
> */
> - if (cur_ino == mp->m_sb.sb_rootino && namelen == 10 &&
> - memcmp(name, "lost+found", 10) == 0) {
> - orphanage_ino = ino;
> - return 1;
> - }
> - if (cur_ino != orphanage_ino)
> - return 0;
> + slen = snprintf(s, sizeof s, "%llu", (unsigned long long) ino);
>
> - slen = sprintf(s, "%lld", (long long)ino);
> - return (slen == namelen && memcmp(name, s, namelen) == 0);
> + return slen == namelen && !memcmp(name, s, namelen);
> }
>
> static void
> @@ -426,13 +442,25 @@ generate_obfuscated_name(
> xfs_dahash_t newhash;
> uchar_t newname[NAME_MAX];
>
> - if (is_special_dirent(ino, namelen, name))
> - return;
> + /*
> + * Our obfuscation algorithm requires at least 5-character
> + * names, so don't bother if the name is too short.
> + */
> + if (namelen < 5)
> + return;
Please make usre you include the reason for this - that this is a
property of the name hashing algorithm.
> - hash = libxfs_da_hashname(name, namelen);
> + /*
> + * We don't obfuscate "lost+found" or any orphan files
> + * therein. When the name table is used for extended
> + * attributes, the inode number provided is 0, in which
> + * case we don't need to make this check.
> + */
> + if (ino && in_lost_found(ino, namelen, name))
> + return;
>
> /* create a random name with the same hash value */
>
> + hash = libxfs_da_hashname(name, namelen);
> do {
> dup = 0;
> newname[0] = '/';
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
>
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
