[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] fs/erofs: Add support for EROFS
|
From: |
Glenn Washburn |
|
Subject: |
Re: [PATCH 1/2] fs/erofs: Add support for EROFS |
|
Date: |
Thu, 20 Jul 2023 12:37:26 -0500 |
On Thu, 20 Jul 2023 21:14:22 +0800
<zhaoyifan@sjtu.edu.cn> wrote:
> 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.
Yes, I believe this will get merged into master. The question is
"when", after or before release. Now that I know that the compression is
on a per-file basis and so this version can still be widely used (with
the condition that the kernel and initrd are uncompressed by EROFS), I
am in support of having this included in the upcoming release.
Ultimately the decision is Daniel's though.
> 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?
I have not looked into ZFS's usage of its internal library, nor how
hard it would be to make it a separate module. But I prefer the
separate module approach. Perhaps Daniel can speak to his preferred
method.
Glenn
> > -----Original Message-----
> > From: Glenn Washburn <development@efficientek.com>
> > Sent: Monday, July 17, 2023 11:51 AM
> > To: Yifan Zhao <zhaoyifan@sjtu.edu.cn>
> > Cc: The development of GNU GRUB <grub-devel@gnu.org>;
> > hsiangkao@linux.alibaba.com; jefflexu@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 <zhaoyifan@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 <zhaoyifan@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'. */
>