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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs