erofs-utils: lib: add API to get pathname of EROFS inode
Gao Xiang
hsiangkao at linux.alibaba.com
Sat Dec 18 11:52:46 AEDT 2021
Hi Igor,
On Fri, Dec 17, 2021 at 02:30:55PM +0200, Igor Eisberg wrote:
>
Thanks for your patch. I have to sleep for a while since I'm really
tired these days..
As Yue Hu said, you could send out patches in the text rather as a
attachment. That is the common way for most communities.
Some comments below (if you don't mind I will update when I apply):
> From 3061d65ebab01802782bfe791b829bc00b386f91 Mon Sep 17 00:00:00 2001
> From: Igor Ostapenko <igoreisberg at gmail.com>
> Date: Fri, 17 Dec 2021 14:08:23 +0200
> Subject: erofs-utils: lib: add API to get pathname of EROFS inode
>
> * General-purpose erofs_get_pathname function utilizing erofs_iterate_dir,
> with recursion and a reused context to avoid overflowing the stack.
> Recommended buffer size is PATH_MAX. Zero-filling the buffer is not
> necessary.
> * dump: PATH_MAX+1 is not required since the definition of PATH_MAX is
> "chars in a path name including nul".
> * Fix missing ctx->de_ftype = de->file_type; in traverse_dirents
> (was never set).
Thanks for catching this. I will fold it in the original patch...
> * Return err from erofs_iterate_dir instead of hardcoded 0, to allow
> breaking the iteration by the callback using a non-zero return code.
>
> Signed-off-by: Igor Ostapenko <igoreisberg at gmail.com>
> ---
> dump/main.c | 72 ++--------------------------------
> include/erofs/dir.h | 1 +
> lib/dir.c | 96 ++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 98 insertions(+), 71 deletions(-)
>
> diff --git a/dump/main.c b/dump/main.c
> index 7f3f743..3c3cc3f 100644
> --- a/dump/main.c
> +++ b/dump/main.c
> @@ -328,71 +328,6 @@ static inline int erofs_checkdirent(struct erofs_dirent *de,
> return dname_len;
> }
>
> -static int erofs_get_pathname(erofs_nid_t nid, erofs_nid_t parent_nid,
> - erofs_nid_t target, char *path, unsigned int pos)
> -{
> - int err;
> - erofs_off_t offset;
> - char buf[EROFS_BLKSIZ];
> - struct erofs_inode inode = { .nid = nid };
> -
> - path[pos++] = '/';
> - if (target == sbi.root_nid)
> - return 0;
> -
> - err = erofs_read_inode_from_disk(&inode);
> - if (err) {
> - erofs_err("read inode failed @ nid %llu", nid | 0ULL);
> - return err;
> - }
> -
> - offset = 0;
> - while (offset < inode.i_size) {
> - erofs_off_t maxsize = min_t(erofs_off_t,
> - inode.i_size - offset, EROFS_BLKSIZ);
> - struct erofs_dirent *de = (void *)buf;
> - struct erofs_dirent *end;
> - unsigned int nameoff;
> -
> - err = erofs_pread(&inode, buf, maxsize, offset);
> - if (err)
> - return err;
> -
> - nameoff = le16_to_cpu(de->nameoff);
> - end = (void *)buf + nameoff;
> - while (de < end) {
> - const char *dname;
> - int len;
> -
> - nameoff = le16_to_cpu(de->nameoff);
> - dname = (char *)buf + nameoff;
> - len = erofs_checkdirent(de, end, maxsize, dname);
> - if (len < 0)
> - return len;
> -
> - if (le64_to_cpu(de->nid) == target) {
> - memcpy(path + pos, dname, len);
> - path[pos + len] = '\0';
> - return 0;
> - }
> -
> - if (de->file_type == EROFS_FT_DIR &&
> - le64_to_cpu(de->nid) != parent_nid &&
> - le64_to_cpu(de->nid) != nid) {
> - memcpy(path + pos, dname, len);
> - err = erofs_get_pathname(le64_to_cpu(de->nid),
> - nid, target, path, pos + len);
> - if (!err)
> - return 0;
> - memset(path + pos, 0, len);
> - }
> - ++de;
> - }
> - offset += maxsize;
> - }
> - return -1;
> -}
> -
> static int erofsdump_map_blocks(struct erofs_inode *inode,
> struct erofs_map_blocks *map, int flags)
> {
> @@ -411,7 +346,7 @@ static void erofsdump_show_fileinfo(bool show_extent)
> erofs_off_t size;
> u16 access_mode;
> struct erofs_inode inode = { .nid = dumpcfg.nid };
> - char path[PATH_MAX + 1] = {0};
> + char path[PATH_MAX];
> char access_mode_str[] = "rwxrwxrwx";
> char timebuf[128] = {0};
> unsigned int extent_count = 0;
> @@ -441,8 +376,7 @@ static void erofsdump_show_fileinfo(bool show_extent)
> return;
> }
>
> - err = erofs_get_pathname(sbi.root_nid, sbi.root_nid,
> - inode.nid, path, 0);
> + err = erofs_get_pathname(&inode, path, sizeof(path));
> if (err < 0) {
> erofs_err("file path not found @ nid %llu", inode.nid | 0ULL);
> return;
> @@ -598,7 +532,7 @@ static void erofsdump_print_statistic(void)
> .cb = erofsdump_dirent_iter,
> .de_nid = sbi.root_nid,
> .dname = "",
> - .de_namelen = 0
> + .de_namelen = 0,
> };
>
> err = erofsdump_readdir(&ctx);
> diff --git a/include/erofs/dir.h b/include/erofs/dir.h
> index 25d6ce7..9d56f3f 100644
> --- a/include/erofs/dir.h
> +++ b/include/erofs/dir.h
> @@ -45,6 +45,7 @@ struct erofs_dir_context {
>
> /* iterate over inodes that are in directory */
> int erofs_iterate_dir(struct erofs_dir_context *ctx, bool fsck);
> +int erofs_get_pathname(struct erofs_inode *inode, char *buf, size_t size);
>
> #ifdef __cplusplus
> }
> diff --git a/lib/dir.c b/lib/dir.c
> index 63e35ba..a3edf0b 100644
> --- a/lib/dir.c
> +++ b/lib/dir.c
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0
> +#include <stdlib.h>
> #include "erofs/print.h"
> #include "erofs/dir.h"
> -#include <stdlib.h>
>
> static int traverse_dirents(struct erofs_dir_context *ctx,
> void *dentry_blk, unsigned int lblk,
> @@ -64,6 +64,7 @@ static int traverse_dirents(struct erofs_dir_context *ctx,
>
> ctx->dname = de_name;
> ctx->de_namelen = de_namelen;
> + ctx->de_ftype = de->file_type;
> ctx->dot_dotdot = is_dot_dotdot_len(de_name, de_namelen);
> if (ctx->dot_dotdot) {
> switch (de_namelen) {
> @@ -121,7 +122,7 @@ out:
> int erofs_iterate_dir(struct erofs_dir_context *ctx, bool fsck)
> {
> struct erofs_inode *dir = ctx->dir;
> - int err;
> + int err = 0;
> erofs_off_t pos;
> char buf[EROFS_BLKSIZ];
>
> @@ -163,5 +164,96 @@ int erofs_iterate_dir(struct erofs_dir_context *ctx, bool fsck)
> dir->nid | 0ULL);
> return -EFSCORRUPTED;
> }
> + return err;
> +}
> +
> +#define EROFS_PATHNAME_FOUND 1
> +
> +struct get_pathname_context {
> + struct erofs_dir_context ctx;
> + erofs_nid_t nid;
> + char *buf;
> + size_t size;
> + size_t pos;
> +};
> +
> +static int get_pathname_iter(struct erofs_dir_context *ctx)
> +{
> + int ret;
> + struct get_pathname_context *pctx = (void *)ctx;
> + const char *dname = ctx->dname;
> + size_t len = ctx->de_namelen;
> + size_t pos = pctx->pos;
> +
> + if (ctx->de_nid == pctx->nid) {
> + if (pos + len + 2 > pctx->size) {
> + erofs_err("get_pathname buffer not large enough: len %zd, size %zd",
> + pos + len + 2, pctx->size);
> + return -ENOMEM;
> + }
> +
> + pctx->buf[pos++] = '/';
> + strncpy(pctx->buf + pos, dname, len);
> + pctx->buf[pos + len] = '\0';
> + return EROFS_PATHNAME_FOUND;
> + }
> +
> + if (ctx->de_ftype == EROFS_FT_DIR && !ctx->dot_dotdot) {
> + struct erofs_inode dir = { .nid = ctx->de_nid };
> +
> + ret = erofs_read_inode_from_disk(&dir);
> + if (ret) {
> + erofs_err("read inode failed @ nid %llu", dir.nid | 0ULL);
> + return ret;
> + }
> +
> + ctx->dir = &dir;
I think old `ctx->dir', `pnid' and `flag' should be saved
when fsck == true.
If fsck == false, I'd suggest not set
EROFS_READDIR_VALID_PNID as well. Let me revise it.
Thanks,
Gao Xiang
More information about the Linux-erofs
mailing list