Hi Kelvin, On Mon, Dec 13, 2021 at 02:42:19PM -0800, Kelvin Zhang wrote: > Friendly ping
Sorry for the delay. I didn't mean to ignore this patch. Yet actually as I once said to Daeho, it would be better to introduce a generic callback iterate function and convert fuse/dump/fsck in one patchset. you could update some coding-style issues as erofs-utils follows Linux kernel style. > > On Wed, Dec 8, 2021 at 5:21 PM Kelvin Zhang <[email protected]> wrote: > > > Change-Id: Ia35708080a72ee204eaaddfc670d3cb8023a078c > > Signed-off-by: Kelvin Zhang <[email protected]> > > --- > > include/erofs/iterate.h | 57 +++++++++++++ > > lib/Makefile.am | 2 +- > > lib/iterate.c | 173 ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 231 insertions(+), 1 deletion(-) > > create mode 100644 include/erofs/iterate.h > > create mode 100644 lib/iterate.c > > > > diff --git a/include/erofs/iterate.h b/include/erofs/iterate.h > > new file mode 100644 > > index 0000000..96171a7 > > --- /dev/null > > +++ b/include/erofs/iterate.h > > @@ -0,0 +1,57 @@ > > +// > > +// Copyright (C) 2021 The Android Open Source Project > > +// > > +// Licensed under the Apache License, Version 2.0 (the "License"); > > +// you may not use this file except in compliance with the License. > > +// You may obtain a copy of the License at > > +// > > +// http://www.apache.org/licenses/LICENSE-2.0 > > +// > > +// Unless required by applicable law or agreed to in writing, software > > +// distributed under the License is distributed on an "AS IS" BASIS, > > +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > > implied. > > +// See the License for the specific language governing permissions and > > +// limitations under the License. > > +// SPDX is enough, it'd be better to be licensed as GPL-2.0+ OR Apache-2.0, see other files. > > + > > +#ifndef ITERATE_ITERATE > > +#define ITERATE_ITERATE > > + > > +#ifdef __cplusplus > > +extern "C" > > +{ > > +#endif > > + > > + > > +#include "erofs/io.h" > > +#include "erofs/print.h" > > + > > + > > +struct erofs_inode_info { > > + uint64_t inode_id; erofs_nid_t nid; > > + const char* name; one tab instead of 2 spaces. > > + bool is_reg_file; ftype. > > + u64 compressed_size; > > + u64 uncompressed_size; Could we get these directly from erofs_inode? so getting rid of them is better. > > + struct erofs_inode* inode; > > + void* arg; > > +}; > > +// Callback function for iterating over inodes of EROFS > > + > > +typedef bool (*ErofsIterCallback)(struct erofs_inode_info); no camelcases, erofs_readdir_cb seems better. and struct erofs_inode_info *. > > + > > +int erofs_iterate_dir(const struct erofs_sb_info* sbi, > > + erofs_nid_t nid, > > + erofs_nid_t parent_nid, > > + ErofsIterCallback cb, > > + void* arg); > > +int erofs_iterate_root_dir(const struct erofs_sb_info* sbi, > > + ErofsIterCallback cbg, > > + void* arg); > > +int erofs_get_occupied_size(struct erofs_inode* inode, erofs_off_t* size); > > + > > +#ifdef __cplusplus > > +} > > +#endif > > + > > +#endif // ITERATE_ITERATE > > diff --git a/lib/Makefile.am b/lib/Makefile.am > > index 67ba798..20c0e4f 100644 > > --- a/lib/Makefile.am > > +++ b/lib/Makefile.am > > @@ -27,7 +27,7 @@ noinst_HEADERS = $(top_srcdir)/include/erofs_fs.h \ > > noinst_HEADERS += compressor.h > > liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c > > exclude.c \ > > namei.c data.c compress.c compressor.c zmap.c > > decompress.c \ > > - compress_hints.c hashmap.c sha256.c blobchunk.c > > + compress_hints.c hashmap.c sha256.c blobchunk.c > > iterate.c > > liberofs_la_CFLAGS = -Wall -Werror -I$(top_srcdir)/include > > if ENABLE_LZ4 > > liberofs_la_CFLAGS += ${LZ4_CFLAGS} > > diff --git a/lib/iterate.c b/lib/iterate.c > > new file mode 100644 > > index 0000000..1a10ec1 > > --- /dev/null > > +++ b/lib/iterate.c > > @@ -0,0 +1,173 @@ > > +// > > +// Copyright (C) 2021 The Android Open Source Project > > +// > > +// Licensed under the Apache License, Version 2.0 (the "License"); > > +// you may not use this file except in compliance with the License. > > +// You may obtain a copy of the License at > > +// > > +// http://www.apache.org/licenses/LICENSE-2.0 > > +// > > +// Unless required by applicable law or agreed to in writing, software > > +// distributed under the License is distributed on an "AS IS" BASIS, > > +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > > implied. > > +// See the License for the specific language governing permissions and > > +// limitations under the License. > > +// Same here. > > + > > +#include "erofs/internal.h" > > +#include "erofs_fs.h" > > +#include "erofs/print.h" > > +#include "erofs/iterate.h" > > + > > +static int erofs_read_dirent(const struct erofs_sb_info* sbi, > > + const struct erofs_dirent* de, > > + erofs_nid_t nid, > > + erofs_nid_t parent_nid, > > + const char* dname, > > + ErofsIterCallback cb, > > + void* arg) { > > + int err; > > + erofs_off_t occupied_size = 0; > > + struct erofs_inode inode = {.nid = de->nid}; > > + err = erofs_read_inode_from_disk(&inode); > > + if (err) { > > + erofs_err("read file inode from disk failed!"); > > + return err; > > + } > > + err = erofs_get_occupied_size(&inode, &occupied_size); > > + if (err) { > > + erofs_err("get file size failed\n"); > > + return err; > > + } no need to do this. > > + char buf[PATH_MAX + 1]; > > + erofs_get_inode_name(sbi, de->nid, buf, PATH_MAX + 1); > > + struct erofs_inode_info info = { > > + .inode_id = de->nid, > > + .name = buf, > > + .is_reg_file = de->file_type == EROFS_FT_REG_FILE, > > + .compressed_size = occupied_size, > > + .uncompressed_size = inode.i_size, > > + .inode = &inode, > > + .arg = arg}; > > + cb(info); > > + if ((de->file_type == EROFS_FT_DIR) && de->nid != nid && > > + de->nid != parent_nid) { > > + err = erofs_iterate_dir(sbi, de->nid, nid, cb, arg); > > + if (err) { > > + erofs_err("parse dir nid %u error occurred\n", > > + (unsigned int)(de->nid)); > > + return err; > > + } > > + } > > + return 0; > > +} > > + > > +static inline int erofs_checkdirent(const struct erofs_dirent* de, > > + const struct erofs_dirent* last_de, > > + u32 maxsize, > > + const char* dname) { > > + int dname_len; > > + unsigned int nameoff = le16_to_cpu(de->nameoff); > > + if (nameoff < sizeof(struct erofs_dirent) || nameoff >= PAGE_SIZE) { > > + erofs_err("invalid de[0].nameoff %u @ nid %llu", nameoff, de->nid | > > 0ULL); > > + return -EFSCORRUPTED; > > + } > > + dname_len = (de + 1 >= last_de) ? strnlen(dname, maxsize - nameoff) > > + : le16_to_cpu(de[1].nameoff) - nameoff; > > + /* a corrupted entry is found */ > > + if (nameoff + dname_len > maxsize || dname_len > EROFS_NAME_LEN) { > > + erofs_err("bogus dirent @ nid %llu", le64_to_cpu(de->nid) | 0ULL); > > + DBG_BUGON(1); > > + return -EFSCORRUPTED; > > + } > > + if (de->file_type >= EROFS_FT_MAX) { > > + erofs_err("invalid file type %u", (unsigned int)(de->nid)); > > + return -EFSCORRUPTED; > > + } > > + return dname_len; > > +} > > + > > +int erofs_iterate_dir(const struct erofs_sb_info* sbi, > > + erofs_nid_t nid, nid is optional unless we do a fsck. Thanks, Gao Xiang
