Re: [PATCH 2/2] xfs_db: add an ls command

[Dave Chinner <david@fromorbit.com>]

On Mon, Oct 26, 2020 at 04:32:41PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add to xfs_db the ability to list a directory.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  db/namei.c               |  380 ++++++++++++++++++++++++++++++++++++++++++++++
>  libxfs/libxfs_api_defs.h |    1 
>  man/man8/xfs_db.8        |   14 ++
>  3 files changed, 395 insertions(+)
> 
> 
> diff --git a/db/namei.c b/db/namei.c
> index 3c9889d62338..b2c036e6777a 100644
> --- a/db/namei.c
> +++ b/db/namei.c
> @@ -221,8 +221,388 @@ static const cmdinfo_t path_cmd = {
>  	.help		= path_help,
>  };
>  
> +/* List a directory's entries. */
> +
> +static const char *filetype_strings[XFS_DIR3_FT_MAX] = {
> +	[XFS_DIR3_FT_UNKNOWN]	= N_("unknown"),
> +	[XFS_DIR3_FT_REG_FILE]	= N_("regular"),
> +	[XFS_DIR3_FT_DIR]	= N_("directory"),
> +	[XFS_DIR3_FT_CHRDEV]	= N_("chardev"),
> +	[XFS_DIR3_FT_BLKDEV]	= N_("blkdev"),
> +	[XFS_DIR3_FT_FIFO]	= N_("fifo"),
> +	[XFS_DIR3_FT_SOCK]	= N_("socket"),
> +	[XFS_DIR3_FT_SYMLINK]	= N_("symlink"),
> +	[XFS_DIR3_FT_WHT]	= N_("whiteout"),
> +};

What does N_() do that is different to _()?

> +static const char *
> +get_dstr(
> +	struct xfs_mount	*mp,
> +	uint8_t			filetype)
> +{
> +	if (!xfs_sb_version_hasftype(&mp->m_sb))
> +		return filetype_strings[XFS_DIR3_FT_UNKNOWN];
> +
> +	if (filetype >= XFS_DIR3_FT_MAX)
> +		return filetype_strings[XFS_DIR3_FT_UNKNOWN];
> +
> +	return filetype_strings[filetype];
> +}
> +
> +static void
> +dir_emit(
> +	struct xfs_mount	*mp,
> +	char			*name,
> +	ssize_t			namelen,
> +	xfs_ino_t		ino,
> +	uint8_t			dtype)
> +{
> +	char			*display_name;
> +	struct xfs_name		xname = { .name = name };
> +	const char		*dstr = get_dstr(mp, dtype);
> +	xfs_dahash_t		hash;
> +	bool			good;
> +
> +	if (namelen < 0) {
> +		/* Negative length means that name is null-terminated. */
> +		display_name = name;
> +		xname.len = strlen(name);
> +		good = true;
> +	} else {
> +		/*
> +		 * Otherwise, name came from a directory entry, so we have to
> +		 * copy the string to a buffer so that we can add the null
> +		 * terminator.
> +		 */
> +		display_name = malloc(namelen + 1);
> +		memcpy(display_name, name, namelen);
> +		display_name[namelen] = 0;
> +		xname.len = namelen;
> +		good = libxfs_dir2_namecheck(name, namelen);
> +	}
> +	hash = libxfs_dir2_hashname(mp, &xname);
> +
> +	dbprintf("%-18llu %-14s 0x%08llx %3d %s", ino, dstr, hash, xname.len,
> +			display_name);
> +	if (!good)
> +		dbprintf(_(" (corrupt)"));
> +	dbprintf("\n");

Can we get this to emit the directory offset of the entry as well?
Also, can this be done as a single dbprintf call like this?

	dbprintf(%-18llu %-14s 0x%08llx %3d %s %s\n",
		ino, dstr, hash, xname.len, display_name,
		good ? _("(good)") : _("(corrupt)"));

(there will be lots of output on big directories....)

> +static int
> +list_sfdir(
> +	struct xfs_da_args		*args)
> +{
> +	struct xfs_inode		*dp = args->dp;
> +	struct xfs_mount		*mp = dp->i_mount;
> +	struct xfs_dir2_sf_entry	*sfep;
> +	struct xfs_dir2_sf_hdr		*sfp;
> +	xfs_ino_t			ino;
> +	unsigned int			i;
> +	uint8_t				filetype;
> +
> +	sfp = (struct xfs_dir2_sf_hdr *)dp->i_df.if_u1.if_data;
> +
> +	/* . and .. entries */
> +	dir_emit(args->dp->i_mount, ".", -1, dp->i_ino, XFS_DIR3_FT_DIR);
> +
> +	ino = libxfs_dir2_sf_get_parent_ino(sfp);
> +	dir_emit(args->dp->i_mount, "..", -1, ino, XFS_DIR3_FT_DIR);
> +
> +	/* Walk everything else. */
> +	sfep = xfs_dir2_sf_firstentry(sfp);
> +	for (i = 0; i < sfp->count; i++) {
> +		ino = libxfs_dir2_sf_get_ino(mp, sfp, sfep);
> +		filetype = libxfs_dir2_sf_get_ftype(mp, sfep);
> +
> +		dir_emit(args->dp->i_mount, (char *)sfep->name, sfep->namelen,
> +				ino, filetype);
> +		sfep = libxfs_dir2_sf_nextentry(mp, sfp, sfep);
> +	}
> +
> +	return 0;
> +}

Hmmm - how much of the xfs_readdir() implementation from the kernel
does this duplicate? It doesn't contain the seek cookie stuff, but
otherwise it's almost identical, right?

[....]

> +/* If the io cursor points to a directory, list its contents. */
> +static int
> +ls_cur(
> +	char			*tag,
> +	bool			direct)

I find the name "direct" rather confusing here. according to
the help below, it will be true when we want to "list the directory
itself, not it's contents"....


> +{
> +	struct xfs_inode	*dp;
> +	int			ret = 0;
> +
> +	if (iocur_top->typ != &typtab[TYP_INODE]) {
> +		dbprintf(_("current object is not an inode.\n"));
> +		return -1;
> +	}
> +
> +	ret = -libxfs_iget(mp, NULL, iocur_top->ino, 0, &dp);
> +	if (ret) {
> +		dbprintf(_("failed to iget directory %llu, error %d\n"),
> +				(unsigned long long)iocur_top->ino, ret);
> +		return -1;
> +	}
> +
> +	if (S_ISDIR(VFS_I(dp)->i_mode) && !direct) {
> +		/* List the contents of a directory. */
> +		if (tag)
> +			dbprintf(_("%s:\n"), tag);
> +
> +		ret = listdir(dp);
> +		if (ret) {
> +			dbprintf(_("failed to list directory %llu: %s\n"),
> +					(unsigned long long)iocur_top->ino,
> +					strerror(ret));
> +			ret = -1;
> +			goto rele;
> +		}
> +	} else if (direct || !S_ISDIR(VFS_I(dp)->i_mode)) {
> +		/* List the directory entry associated with a single file. */
> +		char		inum[32];
> +
> +		if (!tag) {
> +			snprintf(inum, sizeof(inum), "<%llu>",
> +					(unsigned long long)iocur_top->ino);
> +			tag = inum;
> +		} else {
> +			char	*p = strrchr(tag, '/');
> +
> +			if (p)
> +				tag = p + 1;
> +		}
> +
> +		dir_emit(mp, tag, -1, iocur_top->ino,
> +				libxfs_mode_to_ftype(VFS_I(dp)->i_mode));

I'm not sure what this is supposed to do - we turn the current inode
if it's not a directory into a -directory entry- without actually
know it's name? And we can pass in an inode that isn't a directory
and do the same? This doesn't make a huge amount of sense to me - it
tries to display the inode number as a dirent?

> +	} else {
> +		dbprintf(_("current inode %llu is not a directory.\n"),
> +				(unsigned long long)iocur_top->ino);
> +		ret = -1;
> +		goto rele;
> +	}

I don't think we can get to this else branch. If we don't take the
first branch (dir && !direct), the either we are not a dir or direct
is set. The second branch will then be taken if we are not a dir or
direct is set....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com