[PATCH v2 2/2] Add API to iterate over inodes in EROFS
Gao Xiang
xiang at kernel.org
Tue Dec 14 11:43:12 AEDT 2021
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 <zhangkelvin at google.com> wrote:
>
> > Change-Id: Ia35708080a72ee204eaaddfc670d3cb8023a078c
> > Signed-off-by: Kelvin Zhang <zhangkelvin at google.com>
> > ---
> > 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
More information about the Linux-erofs
mailing list