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