Thank you for the reply! As Xiang pointed out, we believe that uncompressed support for EROFS is still valuable, so I would be very grateful if this support could be merged into the GRUB mainline.
By the way, we expect to bring support for the EROFS compression part within a few weeks. However, AFAIK, GRUB lacks generic support for lz4 compression algorithms. Would it be more appropriate to introduce a dedicated lz4 library for EROFS, like zfs, or generic lz4 support for grub? > -----Original Message----- > From: Glenn Washburn <developm...@efficientek.com> > Sent: Monday, July 17, 2023 11:51 AM > To: Yifan Zhao <zhaoyi...@sjtu.edu.cn> > Cc: The development of GNU GRUB <grub-devel@gnu.org>; > hsiang...@linux.alibaba.com; jeffl...@linux.alibaba.com > Subject: Re: [PATCH 1/2] fs/erofs: Add support for EROFS > > On Mon, 17 Jul 2023 00:03:08 +0800 > Yifan Zhao <zhaoyi...@sjtu.edu.cn> wrote: > > > EROFS is a lightweight read-only filesystem designed for performance > > which has already been widely used in several scenarios, such as > > Android system partitions, container images, and rootfs for embedded > devices. > > > > This patch brings EROFS uncompressed support. Now, it's possible to > > boot directly through GRUB with an EROFS rootfs. > > > > EROFS compressed support will be developed later since it has more > > work to polish. > > I would expect that a very small minority are using EROFS without > compression. > Does that also seem true to you? This is relevant because I believe GRUB is in > a feature freeze, but if there were a compelling reason to include this in the > upcoming release, Daniel might make an exception (just guessing). On the > one hand, in favor of including the current code without compression in the > next release is that its simple enough and doesn't touch existing code, so a > bug should have minor impact on the rest of GRUB. On the other hand, if > there is a low probability of its use, perhaps its not worth it. As it > stands, it > seems that GRUB is being released every couple years. > > When you create an updated patch series, it would be much appreciated if > you use --range-diff or --interdiff so that reviewers (me) can verify more > easily the changes. > > > > > Signed-off-by: Yifan Zhao <zhaoyi...@sjtu.edu.cn> > > --- > > The link here shows how to boot the kernel from the EROFS partition with > this patch. > > https://precious-celery-715.notion.site/Demo-of-booting-kernel-in-a-ER > > OFS-partition-c6d199f937a2489bafd03d3083c294b5 > > > > INSTALL | 8 +- > > Makefile.util.def | 1 + > > docs/grub.texi | 4 +- > > grub-core/Makefile.core.def | 5 + > > grub-core/fs/erofs.c | 971 ++++++++++++++++++++++++++++++++++++ > > grub-core/kern/misc.c | 14 + > > include/grub/misc.h | 1 + > > 7 files changed, 998 insertions(+), 6 deletions(-) create mode > > 100644 grub-core/fs/erofs.c > > > > diff --git a/INSTALL b/INSTALL > > index b93fe9c61..1939e4745 100644 > > --- a/INSTALL > > +++ b/INSTALL > > @@ -77,15 +77,15 @@ Prerequisites for make-check: > > > > * If running a Linux kernel the following modules must be loaded: > > - fuse, loop > > - - btrfs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, minix, > > nilfs2, > > + - btrfs, erofs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, > > + minix, nilfs2, > > reiserfs, udf, xfs > > - On newer kernels, the exfat kernel modules may be used instead of the > > exfat FUSE filesystem > > * The following are Debian named packages required mostly for the full > > suite of filesystem testing (but some are needed by other tests as well): > > - - btrfs-progs, dosfstools, e2fsprogs, exfat-utils, f2fs-tools, genromfs, > > - hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs, > > squashfs-tools, > > - reiserfsprogs, udftools, xfsprogs, zfs-fuse > > + - btrfs-progs, dosfstools, erofs_utils, e2fsprogs, exfat-utils, > > f2fs-tools, > > + genromfs, hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs, > > + squashfs-tools, reiserfsprogs, udftools, xfsprogs, zfs-fuse > > Thank you for updating the documentation. It would be nice to also update > docs/grub.texi where appropriate and note that there is no compression > support. I see two places that could use an update (case-insensitive search > for "btrfs" should find them). > Thanks for pointing it out. I will note that EROFS support is incomplete currently. > > - exfat-fuse, if not using the exfat kernel module > > - gzip, lzop, xz-utils > > - attr, cpio, g++, gawk, parted, recode, tar, util-linux diff --git > > a/Makefile.util.def b/Makefile.util.def index 1e9a13d3e..ca2a32a7f > > 100644 > > --- a/Makefile.util.def > > +++ b/Makefile.util.def > > @@ -98,6 +98,7 @@ library = { > > common = grub-core/fs/cpio_be.c; > > common = grub-core/fs/odc.c; > > common = grub-core/fs/newc.c; > > + common = grub-core/fs/erofs.c; > > common = grub-core/fs/ext2.c; > > common = grub-core/fs/fat.c; > > common = grub-core/fs/exfat.c; > > diff --git a/docs/grub.texi b/docs/grub.texi index > > b81267980..20c119c96 100644 > > --- a/docs/grub.texi > > +++ b/docs/grub.texi > > @@ -361,7 +361,7 @@ blocklist notation. The currently supported > > filesystem types are @dfn{Amiga Fast FileSystem (AFFS)}, @dfn{AtheOS > > fs}, @dfn{BeFS}, @dfn{BtrFS} (including raid0, raid1, raid10, gzip > > and lzo), @dfn{cpio} (little- and big-endian bin, odc and newc > > variants), -@dfn{Linux ext2/ext3/ext4}, @dfn{DOS FAT12/FAT16/FAT32}, > > +@dfn{EROFS}, @dfn{Linux ext2/ext3/ext4}, @dfn{DOS > FAT12/FAT16/FAT32}, > > @dfn{exFAT}, @dfn{F2FS}, @dfn{HFS}, @dfn{HFS+}, @dfn{ISO9660} > > (including Joliet, Rock-ridge and multi-chunk files), @dfn{JFS}, > > @dfn{Minix fs} (versions 1, 2 and 3), @dfn{nilfs2}, @@ -6212,7 +6212,7 > > @@ assumed to be encoded in UTF-8. > > NTFS, JFS, UDF, HFS+, exFAT, long filenames in FAT, Joliet part of > > ISO9660 are treated as UTF-16 as per specification. AFS and BFS are > > read as UTF-8, again according to specification. BtrFS, cpio, tar, > > squash4, minix, -minix2, minix3, ROMFS, ReiserFS, XFS, ext2, ext3, > > ext4, FAT (short names), > > +minix2, minix3, ROMFS, ReiserFS, XFS, EROFS, ext2, ext3, ext4, FAT > > +(short names), > > F2FS, RockRidge part of ISO9660, nilfs2, UFS1, UFS2 and ZFS are > > assumed to be UTF-8. This might be false on systems configured with > > legacy charset but as long as the charset used is superset of ASCII > > you should be able to diff --git a/grub-core/Makefile.core.def > > b/grub-core/Makefile.core.def index d2cf29584..eccf9b6cf 100644 > > --- a/grub-core/Makefile.core.def > > +++ b/grub-core/Makefile.core.def > > @@ -1438,6 +1438,11 @@ module = { > > common = fs/odc.c; > > }; > > > > +module = { > > + name = erofs; > > + common = fs/erofs.c; > > +}; > > + > > module = { > > name = ext2; > > common = fs/ext2.c; > > diff --git a/grub-core/fs/erofs.c b/grub-core/fs/erofs.c new file mode > > 100644 index 000000000..f1366ce05 > > --- /dev/null > > +++ b/grub-core/fs/erofs.c > > @@ -0,0 +1,971 @@ > > +/* erofs.c - Enhanced Read-Only File System */ > > +/* > > + * GRUB -- GRand Unified Bootloader > > + * Copyright (C) 2023 Free Software Foundation, Inc. > > + * > > + * GRUB is free software: you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published > > +by > > + * the Free Software Foundation, either version 3 of the License, or > > + * (at your option) any later version. > > + * > > + * GRUB is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. > > + */ > > + > > +#include <grub/disk.h> > > +#include <grub/dl.h> > > +#include <grub/err.h> > > +#include <grub/file.h> > > +#include <grub/fs.h> > > +#include <grub/fshelp.h> > > +#include <grub/misc.h> > > +#include <grub/mm.h> > > +#include <grub/safemath.h> > > +#include <grub/types.h> > > + > > +GRUB_MOD_LICENSE ("GPLv3+"); > > + > > +#define ROUND_UP(x, y) ({ grub_divmod64 (x + (y - 1), y, NULL) * y; > > +}) > > Is there a reason ALIGN_UP is not used instead of the above macro? > I miss the useful macro and will fix it in an updated patch. > > + > > +#define EROFS_SUPER_OFFSET (1024) > > +#define EROFS_MAGIC 0xE0F5E1E2 > > +#define EROFS_ISLOTBITS (5) > > + > > +#define EROFS_FEATURE_INCOMPAT_CHUNKED_FILE 0x00000004 #define > > +EROFS_ALL_FEATURE_INCOMPAT > (EROFS_FEATURE_INCOMPAT_CHUNKED_FILE) > > + > > +struct grub_erofs_super > > +{ > > + grub_uint32_t magic; > > + grub_uint32_t checksum; > > + grub_uint32_t feature_compat; > > + grub_uint8_t blkszbits; > > Perhaps rename to "log2_blksz". Changes made as required. > > > + grub_uint8_t sb_extslots; > > + > > + grub_uint16_t root_nid; > > + grub_uint64_t inos; > > + > > + grub_uint64_t build_time; > > + grub_uint32_t build_time_nsec; > > + grub_uint32_t blocks; > > + grub_uint32_t meta_blkaddr; > > + grub_uint32_t xattr_blkaddr; > > + grub_uint8_t uuid[16]; > > + grub_uint8_t volume_name[16]; > > + grub_uint32_t feature_incompat; > > + > > + union > > + { > > + grub_uint16_t available_compr_algs; > > + grub_uint16_t lz4_max_distance; > > + } GRUB_PACKED u1; > > + > > + grub_uint16_t extra_devices; > > + grub_uint16_t devt_slotoff; > > + grub_uint8_t dirblkbits; > > And maybe log2_dirblksz? And I notice that this is never used. I know nothing > about EROFS, but this looks like it has to do with directories. What is this > used for, if its not needed in the directory reading code below? In a future version, EROFS will support directory block size being a multiple of base blksz, because large dir block size improves directory fanout efficiency. Currently, this field is reserved and its value must be 0. Changed its name as you required. > > + grub_uint8_t xattr_prefix_count; > > + grub_uint32_t xattr_prefix_start; > > + grub_uint64_t packed_nid; > > + grub_uint8_t reserved2[24]; > > +} GRUB_PACKED; > > + > > +#define EROFS_INODE_LAYOUT_COMPACT 0 > > +#define EROFS_INODE_LAYOUT_EXTENDED 1 > > + > > +enum > > +{ > > + EROFS_INODE_FLAT_PLAIN = 0, > > + EROFS_INODE_COMPRESSED_FULL = 1, > > + EROFS_INODE_FLAT_INLINE = 2, > > + EROFS_INODE_COMPRESSED_COMPACT = 3, > > + EROFS_INODE_CHUNK_BASED = 4, > > + EROFS_INODE_DATALAYOUT_MAX > > +}; > > + > > +#define EROFS_I_VERSION_MASKS 0x01 > > +#define EROFS_I_DATALAYOUT_MASKS 0x07 > > + > > +#define EROFS_I_VERSION_BIT 0 > > +#define EROFS_I_DATALAYOUT_BIT 1 > > + > > +struct grub_erofs_inode_chunk_info > > +{ > > + grub_uint16_t format; > > + grub_uint16_t reserved; > > +} GRUB_PACKED; > > + > > +#define EROFS_CHUNK_FORMAT_BLKBITS_MASK 0x001F #define > > +EROFS_CHUNK_FORMAT_INDEXES 0x0020 > > + > > +#define EROFS_BLOCK_MAP_ENTRY_SIZE 4 > > + > > +#define EROFS_NULL_ADDR -1 > > + > > +struct grub_erofs_inode_chunk_index > > +{ > > + grub_uint16_t advise; > > + grub_uint16_t device_id; > > + grub_uint32_t blkaddr; > > +}; > > + > > +union grub_erofs_inode_i_u > > +{ > > + grub_uint32_t compressed_blocks; > > + grub_uint32_t raw_blkaddr; > > + > > + grub_uint32_t rdev; > > + > > + struct grub_erofs_inode_chunk_info c; }; > > + > > +struct grub_erofs_inode_compact > > +{ > > + grub_uint16_t i_format; > > + > > + grub_uint16_t i_xattr_icount; > > + grub_uint16_t i_mode; > > + grub_uint16_t i_nlink; > > + grub_uint32_t i_size; > > + grub_uint32_t i_reserved; > > + > > + union grub_erofs_inode_i_u i_u; > > + > > + grub_uint32_t i_ino; > > + grub_uint16_t i_uid; > > + grub_uint16_t i_gid; > > + grub_uint32_t i_reserved2; > > +} GRUB_PACKED; > > + > > +struct grub_erofs_inode_extended > > +{ > > + grub_uint16_t i_format; > > + > > + grub_uint16_t i_xattr_icount; > > + grub_uint16_t i_mode; > > + grub_uint16_t i_reserved; > > + grub_uint64_t i_size; > > + > > + union grub_erofs_inode_i_u i_u; > > + > > + grub_uint32_t i_ino; > > + > > + grub_uint32_t i_uid; > > + grub_uint32_t i_gid; > > + grub_uint64_t i_mtime; > > + grub_uint32_t i_mtime_nsec; > > + grub_uint32_t i_nlink; > > + grub_uint8_t i_reserved2[16]; > > +} GRUB_PACKED; > > + > > +enum > > +{ > > + EROFS_FT_UNKNOWN, > > + EROFS_FT_REG_FILE, > > + EROFS_FT_DIR, > > + EROFS_FT_CHRDEV, > > + EROFS_FT_BLKDEV, > > + EROFS_FT_FIFO, > > + EROFS_FT_SOCK, > > + EROFS_FT_SYMLINK, > > + EROFS_FT_MAX > > +}; > > + > > +#define EROFS_NAME_LEN 255 > > + > > +struct grub_erofs_dirent > > +{ > > + grub_uint64_t nid; > > + grub_uint16_t nameoff; > > + grub_uint8_t file_type; > > + grub_uint8_t reserved; > > +} GRUB_PACKED; > > + > > +enum > > +{ > > + BH_Meta, > > + BH_Mapped, > > +}; > > + > > +#define EROFS_MAP_MAPPED (1 << BH_Mapped) > > + > > +struct grub_erofs_map_blocks > > +{ > > + grub_off_t m_pa, m_la; > > + grub_off_t m_plen, m_llen; > > + grub_uint32_t m_flags; > > +}; > > + > > +struct grub_erofs_xattr_ibody_header > > +{ > > + grub_uint32_t h_reserved; > > + grub_uint8_t h_shared_count; > > + grub_uint8_t h_reserved2[7]; > > + grub_uint32_t h_shared_xattrs[0]; > > +}; > > + > > +struct grub_fshelp_node > > +{ > > + struct grub_erofs_data *data; > > + struct grub_erofs_inode_extended inode; > > + > > + grub_uint64_t ino; > > + grub_uint8_t inode_type; > > + grub_uint8_t inode_datalayout; > > + > > + /* if the inode has been read into memory? */ > > + bool inode_read; > > +}; > > + > > +struct grub_erofs_data > > +{ > > + grub_disk_t disk; > > + struct grub_erofs_super sb; > > + > > + struct grub_fshelp_node inode; > > +}; > > + > > +#define erofs_blocksz(data) (1u << data->sb.blkszbits) > > + > > +static inline grub_uint64_t > > +erofs_iloc (grub_fshelp_node_t node) > > +{ > > + struct grub_erofs_super *sb = &node->data->sb; > > + > > + return (grub_le_to_cpu32 (sb->meta_blkaddr) << sb->blkszbits) + > > + (node->ino << EROFS_ISLOTBITS); } > > + > > +static grub_err_t > > +grub_erofs_read_inode (struct grub_erofs_data *data, > > +grub_fshelp_node_t node) { > > + struct grub_erofs_inode_compact *dic; > > + grub_err_t err; > > + grub_uint64_t addr = erofs_iloc (node); > > + > > + dic = (struct grub_erofs_inode_compact *)&node->inode; > > + > > + err = grub_disk_read (data->disk, addr >> GRUB_DISK_SECTOR_BITS, > > + addr & (GRUB_DISK_SECTOR_SIZE - 1), > > 8 spaces should be converted to tabs. > > > + sizeof (struct grub_erofs_inode_compact), > > + dic); if (err) > > + return err; > > + > > + node->inode_type = > > + (dic->i_format >> EROFS_I_VERSION_BIT) & EROFS_I_VERSION_MASKS; > > + node->inode_datalayout = > > + (dic->i_format >> EROFS_I_DATALAYOUT_BIT) & > > + EROFS_I_DATALAYOUT_MASKS; > > + > > + switch (node->inode_type) > > + { > > + case EROFS_INODE_LAYOUT_EXTENDED: > > + addr += sizeof (struct grub_erofs_inode_compact); > > + err = grub_disk_read ( > > + data->disk, addr >> GRUB_DISK_SECTOR_BITS, > > + addr & (GRUB_DISK_SECTOR_SIZE - 1), > > + sizeof (struct grub_erofs_inode_extended) - > > + sizeof (struct grub_erofs_inode_compact), > > + (char *)dic + sizeof (struct grub_erofs_inode_compact)); > > + if (err) > > + return err; > > + break; > > + case EROFS_INODE_LAYOUT_COMPACT: > > + break; > > + default: > > + return grub_error (GRUB_ERR_BAD_FS, > > + "invalid inode version %u @ inode %" > > PRIuGRUB_UINT64_T, > > + node->inode_type, node->ino); > > + } > > + > > + node->inode_read = true; > > + > > + return 0; > > +} > > + > > +static inline grub_uint64_t > > +erofs_inode_size (grub_fshelp_node_t node) { > > + return node->inode_type == EROFS_INODE_LAYOUT_COMPACT > > + ? sizeof (struct grub_erofs_inode_compact) > > + : sizeof (struct grub_erofs_inode_extended); } > > + > > +static inline grub_uint64_t > > +erofs_inode_file_size (grub_fshelp_node_t node) { > > + struct grub_erofs_inode_compact *dic = > > + (struct grub_erofs_inode_compact *)&node->inode; > > + > > + return node->inode_type == EROFS_INODE_LAYOUT_COMPACT > > + ? grub_le_to_cpu32 (dic->i_size) > > + : grub_le_to_cpu64 (node->inode.i_size); } > > + > > +static inline grub_uint32_t > > +erofs_inode_xattr_ibody_size (grub_fshelp_node_t node) { > > + grub_uint16_t cnt = grub_le_to_cpu16 (node->inode.i_xattr_icount); > > + > > + return cnt ? sizeof (struct grub_erofs_xattr_ibody_header) + > > + (cnt - 1) * sizeof (grub_uint32_t) > > + : 0; > > +} > > + > > +static inline grub_uint64_t > > +erofs_inode_mtime (grub_fshelp_node_t node) { > > + return node->inode_type == EROFS_INODE_LAYOUT_COMPACT > > + ? grub_le_to_cpu64 (node->data->sb.build_time) > > + : grub_le_to_cpu64 (node->inode.i_mtime); } > > + > > +static grub_err_t > > +grub_erofs_map_blocks_flatmode (grub_fshelp_node_t node, > > + struct grub_erofs_map_blocks *map) { > > + grub_off_t nblocks, lastblk, file_size; > > + grub_off_t tailendpacking = > > + (node->inode_datalayout == EROFS_INODE_FLAT_INLINE) ? 1 : 0; > > + grub_uint32_t blocksz = erofs_blocksz (node->data); > > + > > + file_size = erofs_inode_file_size (node); nblocks = grub_divmod64 > > + (file_size + blocksz - 1, blocksz, NULL); > > Would it not be better to do the following? > > nblocks = (file_size + (1U << node->data->sb.blkszbits)) >> node->data- > >sb.blkszbits; > Yes, I think it would be nblocks = (file_size + blocksz - 1) >> node->data->sb.blkszbits; > And if so, then maybe create a erofs_log_blocksz() macro. > > > + lastblk = nblocks - tailendpacking; > > + > > + map->m_flags = EROFS_MAP_MAPPED; > > + > > + if (map->m_la < lastblk * blocksz) > > s/lastblk * blocksz/(lastblk * blocksz)/ > > > + { > > + map->m_pa = > > + grub_le_to_cpu32 (node->inode.i_u.raw_blkaddr) * blocksz + map- > >m_la; > > + map->m_plen = lastblk * blocksz - map->m_la; > > + } > > + else if (tailendpacking) > > + { > > + map->m_pa = erofs_iloc (node) + erofs_inode_size (node) + > > + erofs_inode_xattr_ibody_size (node) + map->m_la % > > + blocksz; > > s/map->m_la % blocksz/(map->m_la % blocksz)/ > > > + map->m_plen = file_size - map->m_la; > > + > > + if (map->m_pa % blocksz + map->m_plen > blocksz) > > s/map->m_pa % blocksz + map->m_plen/((map->m_pa % blocksz) + map- > >m_plen)/ > > Add explicit parenthesis for the code below too. > I'll add them. > > + return grub_error ( > > + GRUB_ERR_BAD_FS, > > + "inline data cross block boundary @ inode %" PRIuGRUB_UINT64_T, > > + node->ino); > > + } > > + else > > + { > > This parenthesis is unneeded. Yep. > > > + return grub_error (GRUB_ERR_BAD_FS, > > + "internal error: invalid map->m_la=%" > > PRIuGRUB_UINT64_T > > + " @ inode %" PRIuGRUB_UINT64_T, > > + map->m_la, node->ino); > > + } > > + > > + map->m_llen = map->m_plen; > > + return GRUB_ERR_NONE; > > +} > > + > > +static grub_err_t > > +grub_erofs_map_blocks_chunkmode (grub_fshelp_node_t node, > > + struct grub_erofs_map_blocks *map) { > > + grub_uint16_t chunk_format = grub_le_to_cpu16 > > +(node->inode.i_u.c.format); > > + grub_off_t unit, pos; > > + grub_uint64_t chunknr; > > + grub_uint8_t chunkbits; > > + grub_err_t err; > > + > > + if (chunk_format & EROFS_CHUNK_FORMAT_INDEXES) > > + unit = sizeof (struct grub_erofs_inode_chunk_index); else > > + unit = EROFS_BLOCK_MAP_ENTRY_SIZE; > > + > > + chunkbits = node->data->sb.blkszbits + > > + (chunk_format & EROFS_CHUNK_FORMAT_BLKBITS_MASK); > > + > > + chunknr = map->m_la >> chunkbits; > > + pos = ROUND_UP (erofs_iloc (node) + erofs_inode_size (node) + > > + erofs_inode_xattr_ibody_size (node), > > + unit); > > + pos += chunknr * unit; > > + > > + map->m_la = chunknr << chunkbits; > > + map->m_plen = grub_min (1ULL << chunkbits, > > + ROUND_UP (erofs_inode_file_size (node) - > > map->m_la, > > + erofs_blocksz (node->data))); > > + > > + if (chunk_format & EROFS_CHUNK_FORMAT_INDEXES) > > + { > > + struct grub_erofs_inode_chunk_index idx; > > + grub_uint32_t blkaddr; > > + > > + err = grub_disk_read (node->data->disk, pos >> > GRUB_DISK_SECTOR_BITS, > > + pos & (GRUB_DISK_SECTOR_SIZE - 1), unit, &idx); > > + if (err) > > + return err; > > + > > + blkaddr = grub_le_to_cpu32 (idx.blkaddr); > > + > > + if (blkaddr == (grub_uint32_t)EROFS_NULL_ADDR) > > It is preferred to have a space after the cast parenthesis, elsewhere also. I've overlooked it. Thanks for pointing it out. > > + { > > + map->m_pa = 0; > > + map->m_flags = 0; > > + } > > + else > > + { > > + map->m_pa = blkaddr << node->data->sb.blkszbits; > > + map->m_flags = EROFS_MAP_MAPPED; > > + } > > + } > > + else > > + { > > + grub_uint32_t blkaddr_le, blkaddr; > > + > > + err = > > + grub_disk_read (node->data->disk, pos >> GRUB_DISK_SECTOR_BITS, > > + pos & (GRUB_DISK_SECTOR_SIZE - 1), unit, > > &blkaddr_le); > > + if (err) > > + return err; > > + > > + blkaddr = grub_le_to_cpu32 (blkaddr_le); > > + if (blkaddr == (grub_uint32_t)EROFS_NULL_ADDR) > > + { > > + map->m_pa = 0; > > + map->m_flags = 0; > > + } > > + else > > + { > > + map->m_pa = blkaddr << node->data->sb.blkszbits; > > + map->m_flags = EROFS_MAP_MAPPED; > > + } > > + } > > + > > + map->m_llen = map->m_plen; > > + return GRUB_ERR_NONE; > > +} > > + > > +static grub_err_t > > +grub_erofs_map_blocks (grub_fshelp_node_t node, > > + struct grub_erofs_map_blocks *map) { > > + if (map->m_la >= erofs_inode_file_size (node)) > > + { > > + map->m_llen = map->m_plen = 0; > > + map->m_pa = 0; > > + map->m_flags = 0; > > + return GRUB_ERR_NONE; > > + } > > + > > + if (node->inode_datalayout != EROFS_INODE_CHUNK_BASED) > > + return grub_erofs_map_blocks_flatmode (node, map); > > + else > > + return grub_erofs_map_blocks_chunkmode (node, map); } > > + > > +static grub_err_t > > +grub_erofs_read_raw_data (grub_fshelp_node_t node, char *buf, > grub_off_t size, > > + grub_off_t offset, grub_off_t *bytes) { > > + struct grub_erofs_map_blocks map; > > + grub_err_t err; > > + > > + if (bytes) > > + *bytes = 0; > > + > > + if (!node->inode_read) > > + { > > + err = grub_erofs_read_inode (node->data, node); > > + if (err) > > + return err; > > + } > > + > > + grub_memset (&map, 0, sizeof (map)); > > + > > + grub_off_t cur = offset; > > + while (cur < offset + size) > > + { > > + char *const estart = buf + cur - offset; > > + grub_off_t eend, moff = 0; > > + > > + map.m_la = cur; > > + err = grub_erofs_map_blocks (node, &map); > > + if (err) > > + return err; > > + > > + eend = grub_min (offset + size, map.m_la + map.m_llen); > > + if (!(map.m_flags & EROFS_MAP_MAPPED)) > > + { > > + if (!map.m_llen) > > + { > > + /* reached EOF */ > > + grub_memset (estart, 0, offset + size - cur); > > + cur = offset + size; > > + continue; > > + } > > + > > + /* Hole */ > > + grub_memset (estart, 0, eend - cur); > > + cur = eend; > > + if (bytes) > > + *bytes += eend - cur; > > + continue; > > + } > > + > > + if (cur > map.m_la) > > + { > > + moff = cur - map.m_la; > > + map.m_la = cur; > > + } > > + > > + err = grub_disk_read (node->data->disk, > > + (map.m_pa + moff) >> GRUB_DISK_SECTOR_BITS, > > + (map.m_pa + moff) & (GRUB_DISK_SECTOR_SIZE - > > 1), > > + eend - map.m_la, estart); > > + if (err) > > + return err; > > + > > + if (bytes) > > + *bytes += eend - map.m_la; > > + > > + cur = eend; > > + } > > + > > + return GRUB_ERR_NONE; > > +} > > + > > +static int > > +grub_erofs_iterate_dir (grub_fshelp_node_t dir, > > + grub_fshelp_iterate_dir_hook_t hook, void > > +*hook_data) { > > + grub_off_t offset = 0, file_size; > > + grub_err_t err; > > + grub_uint32_t blocksz = erofs_blocksz (dir->data); > > + char *buf; > > + > > + if (!dir->inode_read) > > + { > > + err = grub_erofs_read_inode (dir->data, dir); > > + if (err) > > + return 0; > > + } > > + > > + file_size = erofs_inode_file_size (dir); buf = grub_malloc > > + (blocksz); if (!buf) > > + { > > + grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory"); > > + return 0; > > + } > > + > > + while (offset < file_size) > > + { > > + grub_off_t maxsize = grub_min (blocksz, file_size - offset); > > + struct grub_erofs_dirent *de = (void *)buf, *end; > > + grub_uint16_t nameoff; > > + > > + err = grub_erofs_read_raw_data (dir, buf, maxsize, offset, NULL); > > + if (err) > > + goto not_found; > > + > > + nameoff = grub_le_to_cpu16 (de->nameoff); > > + if (nameoff < sizeof (struct grub_erofs_dirent) || nameoff > blocksz) > > + { > > + grub_error (GRUB_ERR_BAD_FS, > > + "invalid de[0].nameoff %u @ inode %" > > PRIuGRUB_UINT64_T, > > + nameoff, dir->ino); > > + goto not_found; > > + } > > + > > + end = (struct grub_erofs_dirent *)((char *)de + nameoff); > > + while (de < end) > > + { > > + struct grub_fshelp_node *fdiro; > > + enum grub_fshelp_filetype type; > > + char filename[EROFS_NAME_LEN + 1]; > > + unsigned int de_namelen; > > + const char *de_name; > > + > > + fdiro = grub_malloc (sizeof (struct grub_fshelp_node)); > > + if (!fdiro) > > + { > > + grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory"); > > + goto not_found; > > + } > > + > > + fdiro->data = dir->data; > > + fdiro->ino = grub_le_to_cpu64 (de->nid); > > + fdiro->inode_read = false; > > + > > + nameoff = grub_le_to_cpu16 (de->nameoff); > > + de_name = buf + nameoff; > > + if (de + 1 >= end) > > + de_namelen = grub_strnlen (de_name, maxsize - nameoff); > > + else > > + de_namelen = grub_le_to_cpu16 (de[1].nameoff) - nameoff; > > + > > + grub_memcpy (filename, de_name, de_namelen); > > + filename[de_namelen] = '\0'; > > + > > + switch (grub_le_to_cpu16 (de->file_type)) > > + { > > + case EROFS_FT_REG_FILE: > > + case EROFS_FT_BLKDEV: > > + case EROFS_FT_CHRDEV: > > + case EROFS_FT_FIFO: > > + case EROFS_FT_SOCK: > > + type = GRUB_FSHELP_REG; > > + break; > > + case EROFS_FT_DIR: > > + type = GRUB_FSHELP_DIR; > > + break; > > + case EROFS_FT_SYMLINK: > > + type = GRUB_FSHELP_SYMLINK; > > + break; > > + case EROFS_FT_UNKNOWN: > > + default: > > + type = GRUB_FSHELP_UNKNOWN; > > + break; > > + } > > + > > + if (hook (filename, type, fdiro, hook_data)) > > + { > > + grub_free (buf); > > + return 1; > > + } > > + > > + ++de; > > + } > > + > > + offset += maxsize; > > + } > > + > > +not_found: > > + grub_free (buf); > > + return 0; > > +} > > + > > +static char * > > +grub_erofs_read_symlink (grub_fshelp_node_t node) { > > + char *symlink; > > + grub_size_t sz; > > + > > + if (!node->inode_read) > > + { > > + grub_erofs_read_inode (node->data, node); > > + if (grub_errno) > > + { > > + return NULL; > > + } > > + } > > + > > + if (grub_add (erofs_inode_file_size (node), 1, &sz)) > > + { > > + grub_error (GRUB_ERR_OUT_OF_RANGE, N_ ("overflow is detected")); > > + return NULL; > > + } > > + > > + symlink = grub_malloc (sz); > > + if (!symlink) > > + return NULL; > > + > > + grub_erofs_read_raw_data (node, symlink, sz - 1, 0, NULL); if > > + (grub_errno) > > + { > > + grub_free (symlink); > > + return NULL; > > + } > > + > > + symlink[sz - 1] = '\0'; > > + return symlink; > > +} > > + > > +static struct grub_erofs_data * > > +grub_erofs_mount (grub_disk_t disk, bool read_root) { > > + struct grub_erofs_super sb; > > + grub_err_t err; > > + struct grub_erofs_data *data; > > + grub_uint32_t feature; > > + > > + err = grub_disk_read (disk, EROFS_SUPER_OFFSET >> > GRUB_DISK_SECTOR_BITS, 0, > > + sizeof (sb), &sb); if (grub_errno == > > + GRUB_ERR_OUT_OF_RANGE) > > + grub_error (GRUB_ERR_BAD_FS, "not a valid erofs filesystem"); if > > + (err) > > + return NULL; > > + if (sb.magic != grub_cpu_to_le32_compile_time (EROFS_MAGIC)) > > + { > > + grub_error (GRUB_ERR_BAD_FS, "not a valid erofs filesystem"); > > + return NULL; > > + } > > + > > + feature = grub_le_to_cpu32 (sb.feature_incompat); if (feature & > > + ~EROFS_ALL_FEATURE_INCOMPAT) > > + { > > + grub_error (GRUB_ERR_BAD_FS, "unsupported features: 0x%x", > > + feature & ~EROFS_ALL_FEATURE_INCOMPAT); > > + return NULL; > > + } > > + > > + data = grub_malloc (sizeof (*data)); if (!data) > > + { > > + grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory"); > > + return NULL; > > + } > > + > > + data->disk = disk; > > + data->sb = sb; > > + > > + if (read_root) > > + { > > + data->inode.data = data; > > + data->inode.ino = grub_le_to_cpu16 (sb.root_nid); > > + err = grub_erofs_read_inode (data, &data->inode); > > + if (err) > > + { > > + grub_free (data); > > + return NULL; > > + } > > + } > > + > > + return data; > > +} > > + > > +/* Context for grub_erofs_dir. */ > > +struct grub_erofs_dir_ctx > > +{ > > + grub_fs_dir_hook_t hook; > > + void *hook_data; > > + struct grub_erofs_data *data; > > +}; > > + > > +/* Helper for grub_erofs_dir. */ > > +static int > > +grub_erofs_dir_iter (const char *filename, enum grub_fshelp_filetype > filetype, > > + grub_fshelp_node_t node, void *data) { > > + struct grub_erofs_dir_ctx *ctx = data; > > + struct grub_dirhook_info info; > > + > > + grub_memset (&info, 0, sizeof (info)); if (!node->inode_read) > > + { > > + grub_erofs_read_inode (ctx->data, node); > > + grub_errno = GRUB_ERR_NONE; > > + } > > + > > + if (node->inode_read) > > + { > > + info.mtimeset = 1; > > + info.mtime = erofs_inode_mtime (node); > > + } > > + > > + info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR); > > + grub_free (node); > > + return ctx->hook (filename, &info, ctx->hook_data); } > > + > > +static grub_err_t > > +grub_erofs_dir (grub_device_t device, const char *path, > grub_fs_dir_hook_t hook, > > + void *hook_data) > > +{ > > + grub_fshelp_node_t fdiro = NULL; > > + struct grub_erofs_dir_ctx ctx = { > > + .hook = hook, > > + .hook_data = hook_data, > > + }; > > + > > + ctx.data = grub_erofs_mount (device->disk, true); if (!ctx.data) > > + goto fail; > > + > > + grub_fshelp_find_file (path, &ctx.data->inode, &fdiro, > grub_erofs_iterate_dir, > > + grub_erofs_read_symlink, GRUB_FSHELP_DIR); > > + if (grub_errno) > > + goto fail; > > + > > + grub_erofs_iterate_dir (fdiro, grub_erofs_dir_iter, &ctx); > > + > > +fail: > > + if (fdiro != &ctx.data->inode) > > + grub_free (fdiro); > > + grub_free (ctx.data); > > + > > + return grub_errno; > > +} > > + > > +static grub_err_t > > +grub_erofs_open (grub_file_t file, const char *name) { > > + struct grub_erofs_data *data; > > + struct grub_fshelp_node *fdiro = 0; > > + grub_err_t err; > > + > > + data = grub_erofs_mount (file->device->disk, true); if (!data) > > + { > > + err = grub_errno; > > + goto fail; > > + } > > + > > + err = > > + grub_fshelp_find_file (name, &data->inode, &fdiro, > grub_erofs_iterate_dir, > > + grub_erofs_read_symlink, > > + GRUB_FSHELP_REG); if (err) > > + goto fail; > > + > > + if (!fdiro->inode_read) > > + { > > + err = grub_erofs_read_inode (data, fdiro); > > + if (err) > > + goto fail; > > + } > > + > > + grub_memcpy (&data->inode, fdiro, sizeof (*fdiro)); grub_free > > + (fdiro); > > + > > + file->data = data; > > + file->size = erofs_inode_file_size (&data->inode); > > + > > + return GRUB_ERR_NONE; > > + > > +fail: > > + if (fdiro != &data->inode) > > + grub_free (fdiro); > > + grub_free (data); > > + > > + return err; > > +} > > + > > +static grub_ssize_t > > +grub_erofs_read (grub_file_t file, char *buf, grub_size_t len) { > > + struct grub_erofs_data *data = file->data; > > + struct grub_fshelp_node *inode = &data->inode; > > + grub_off_t off = file->offset, ret = 0; > > + grub_off_t file_size; > > + > > + if (!inode->inode_read) > > + { > > + grub_erofs_read_inode (data, inode); > > + if (grub_errno) > > + { > > + ret = 0; > > + grub_error (GRUB_ERR_IO, "cannot read @ inode %" > PRIuGRUB_UINT64_T, > > + inode->ino); > > + goto end; > > + } > > + } > > + > > + file_size = erofs_inode_file_size (inode); > > + > > + if (off >= file_size) > > + goto end; > > + > > + if (off + len > file_size) > > + len = file_size - off; > > + > > + grub_erofs_read_raw_data (inode, buf, len, off, &ret); if > > + (grub_errno) > > + { > > + ret = 0; > > + grub_error (GRUB_ERR_IO, "cannot read file @ inode %" > PRIuGRUB_UINT64_T, > > + inode->ino); > > + goto end; > > + } > > + > > +end: > > + return ret; > > +} > > + > > +static grub_err_t > > +grub_erofs_close (grub_file_t file) > > +{ > > + grub_free (file->data); > > + > > + return GRUB_ERR_NONE; > > +} > > + > > +static grub_err_t > > +grub_erofs_uuid (grub_device_t device, char **uuid) { > > + struct grub_erofs_data *data = grub_erofs_mount (device->disk, > > +false); > > This should probably be changed to the following so that we don't return a > previous error. I overlooked that. Thanks. > struct grub_erofs_data *data; > > grub_errno = GRUB_ERR_NONE; > data = grub_erofs_mount (device->disk, false); > > > + > > + if (data) > > + *uuid = grub_xasprintf ( > > + "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x- > %02x%02x%02x%02x%02x%" > > + "02x", > > + data->sb.uuid[0], data->sb.uuid[1], data->sb.uuid[2], data- > >sb.uuid[3], > > + data->sb.uuid[4], data->sb.uuid[5], data->sb.uuid[6], data- > >sb.uuid[7], > > + data->sb.uuid[8], data->sb.uuid[9], data->sb.uuid[10], > > + data->sb.uuid[11], data->sb.uuid[12], data->sb.uuid[13], > > + data->sb.uuid[14], data->sb.uuid[15]); > > I want to point out that grub_xasprintf can fail and not set grub_errno, > namely when calling grub_malloc. In this case this function *should not* > return GRUB_ERR_NONE (but likely will and if it doesn't it will be from some > previous failure unrelated to this function). I consider this a bug in > grub_xasprintf, so this ideally is correct. I think I'll send a patch for > this. > > Glenn Thanks for the reminder. I think the code can be left unchanged here. Thanks, Yifan Zhao > > + else > > + *uuid = NULL; > > + > > + grub_free (data); > > + > > + return grub_errno; > > +} > > + > > +static grub_err_t > > +grub_erofs_label (grub_device_t device, char **label) { > > + struct grub_erofs_data *data = grub_erofs_mount (device->disk, > > +false); > > + > > + if (data) > > + *label = grub_strndup ((char *)data->sb.volume_name, > > + sizeof (data->sb.volume_name)); else > > + *label = NULL; > > + > > + grub_free (data); > > + > > + return grub_errno; > > +} > > + > > +static grub_err_t > > +grub_erofs_mtime (grub_device_t device, grub_int64_t *tm) { > > + struct grub_erofs_data *data = grub_erofs_mount (device->disk, > > +false); > > + > > + if (!data) > > + *tm = 0; > > + else > > + *tm = grub_le_to_cpu64 (data->sb.build_time); > > + > > + grub_free (data); > > + > > + return grub_errno; > > +} > > + > > +static struct grub_fs grub_erofs_fs = { > > + .name = "erofs", > > + .fs_dir = grub_erofs_dir, > > + .fs_open = grub_erofs_open, > > + .fs_read = grub_erofs_read, > > + .fs_close = grub_erofs_close, > > + .fs_uuid = grub_erofs_uuid, > > + .fs_label = grub_erofs_label, > > + .fs_mtime = grub_erofs_mtime, > > +#ifdef GRUB_UTIL > > + .reserved_first_sector = 1, > > + .blocklist_install = 0, > > +#endif > > + .next = 0, > > +}; > > + > > +GRUB_MOD_INIT (erofs) > > +{ > > + grub_fs_register (&grub_erofs_fs); > > +} > > + > > +GRUB_MOD_FINI (erofs) > > +{ > > + grub_fs_unregister (&grub_erofs_fs); } > > \ No newline at end of file > > diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c index > > 2890aad49..66cbb3c5c 100644 > > --- a/grub-core/kern/misc.c > > +++ b/grub-core/kern/misc.c > > @@ -594,6 +594,20 @@ grub_strlen (const char *s) > > return p - s; > > } > > > > +grub_size_t > > +grub_strnlen (const char *s, grub_size_t n) { > > + const char *p = s; > > + > > + if (n == 0) > > + return 0; > > + > > + while (*p && n--) > > + p++; > > + > > + return p - s; > > +} > > + > > static inline void > > grub_reverse (char *str) > > { > > diff --git a/include/grub/misc.h b/include/grub/misc.h index > > 1b35a167f..eb4aa55c1 100644 > > --- a/include/grub/misc.h > > +++ b/include/grub/misc.h > > @@ -335,6 +335,7 @@ char *EXPORT_FUNC(grub_strdup) (const char *s) > > WARN_UNUSED_RESULT; char *EXPORT_FUNC(grub_strndup) (const char > *s, > > grub_size_t n) WARN_UNUSED_RESULT; void > *EXPORT_FUNC(grub_memset) > > (void *s, int c, grub_size_t n); grub_size_t EXPORT_FUNC(grub_strlen) > > (const char *s) WARN_UNUSED_RESULT; > > +grub_size_t EXPORT_FUNC(grub_strnlen) (const char *s, grub_size_t n) > > +WARN_UNUSED_RESULT; > > > > /* Replace all `ch' characters of `input' with `with' and copy the > > result into `output'; return EOS address of `output'. */ _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel