On Mon, Oct 26, 2020 at 04:32:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Add a command to xfs_db so that we can navigate to inodes by path.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
....
> +/* Given a directory and a structured path, walk the path and set the cursor. */
> +static int
> +path_navigate(
> + struct xfs_mount *mp,
> + xfs_ino_t rootino,
> + struct dirpath *dirpath)
> +{
> + struct xfs_inode *dp;
> + xfs_ino_t ino = rootino;
> + unsigned int i;
> + int error;
> +
> + error = -libxfs_iget(mp, NULL, ino, 0, &dp);
> + if (error)
> + return error;
> +
> + for (i = 0; i < dirpath->depth; i++) {
> + struct xfs_name xname = {
> + .name = dirpath->path[i],
> + .len = strlen(dirpath->path[i]),
> + };
> +
> + if (!S_ISDIR(VFS_I(dp)->i_mode)) {
> + error = ENOTDIR;
> + goto rele;
> + }
> +
> + error = -libxfs_dir_lookup(NULL, dp, &xname, &ino, NULL);
> + if (error)
> + goto rele;
> + if (!xfs_verify_ino(mp, ino)) {
> + error = EFSCORRUPTED;
> + goto rele;
> + }
> +
> + libxfs_irele(dp);
> + dp = NULL;
> +
> + error = -libxfs_iget(mp, NULL, ino, 0, &dp);
> + switch (error) {
> + case EFSCORRUPTED:
> + case EFSBADCRC:
> + case 0:
> + break;
> + default:
> + return error;
> + }
> + }
> +
> + set_cur_inode(ino);
> +rele:
> + if (dp)
> + libxfs_irele(dp);
> + return error;
> +}
This could return negative errors....
> +/* Walk a directory path to an inode and set the io cursor to that inode. */
> +static int
> +path_walk(
> + char *path)
> +{
> + struct dirpath *dirpath;
> + char *p = path;
> + xfs_ino_t rootino = mp->m_sb.sb_rootino;
> + int ret = 0;
> +
> + if (*p == '/') {
> + /* Absolute path, start from the root inode. */
> + p++;
> + } else {
> + /* Relative path, start from current dir. */
> + if (iocur_top->typ != &typtab[TYP_INODE]) {
> + dbprintf(_("current object is not an inode.\n"));
> + return -1;
> + }
> +
> + if (!S_ISDIR(iocur_top->mode)) {
> + dbprintf(_("current inode %llu is not a directory.\n"),
> + (unsigned long long)iocur_top->ino);
> + return -1;
> + }
> + rootino = iocur_top->ino;
> + }
> +
> + dirpath = path_parse(p);
> + if (!dirpath) {
> + dbprintf(_("%s: not enough memory to parse.\n"), path);
> + return -1;
> + }
and this could return -ENOMEM here with no error message....
> +
> + ret = path_navigate(mp, rootino, dirpath);
> + if (ret) {
> + dbprintf(_("%s: %s\n"), path, strerror(ret));
> + ret = -1;
> + }
... don't overwrite ret here, move the dbprintf() to the caller and
the one error message captures all possible errors.
Also, no need for _() for a format string that contains no
translatable text....
Otherwise, looks fine.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
