[PATCH v5 10/13] error: Use format code PRIxGRUB_UINT64_T for 64-bit arg in grub_error
The macro ELF_R_TYPE does not change the underlying type. Here its argument
is a 64-bit Elf64_Xword. Make sure the format code matches.
For the riscv architecture, rel->r_info could be either Elf32_Xword or
Elf64_Xword depending on if 32 or 64-bit risc is being built. So cast to
64-bit value regardless.
Signed-off-by: Glenn Washburn
---
grub-core/efiemu/i386/loadcore64.c | 3 ++-
grub-core/kern/arm64/dl.c | 3 ++-
grub-core/kern/ia64/dl.c | 3 ++-
grub-core/kern/riscv/dl.c | 5 +++--
grub-core/kern/sparc64/dl.c| 3 ++-
grub-core/kern/x86_64/dl.c | 3 ++-
6 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/grub-core/efiemu/i386/loadcore64.c
b/grub-core/efiemu/i386/loadcore64.c
index 18facf47f..7316efc39 100644
--- a/grub-core/efiemu/i386/loadcore64.c
+++ b/grub-core/efiemu/i386/loadcore64.c
@@ -122,7 +122,8 @@ grub_arch_efiemu_relocate_symbols64 (grub_efiemu_segment_t
segs,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
- N_("relocation 0x%x is not implemented
yet"),
+ N_("relocation 0x%" PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/arm64/dl.c b/grub-core/kern/arm64/dl.c
index fb0337319..401672374 100644
--- a/grub-core/kern/arm64/dl.c
+++ b/grub-core/kern/arm64/dl.c
@@ -184,7 +184,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%" PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/ia64/dl.c b/grub-core/kern/ia64/dl.c
index ebcf31629..ddf240976 100644
--- a/grub-core/kern/ia64/dl.c
+++ b/grub-core/kern/ia64/dl.c
@@ -137,7 +137,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/riscv/dl.c b/grub-core/kern/riscv/dl.c
index 6fb8385ef..e6c4f89ff 100644
--- a/grub-core/kern/riscv/dl.c
+++ b/grub-core/kern/riscv/dl.c
@@ -331,8 +331,9 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
-ELF_R_TYPE (rel->r_info));
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
+(grub_uint64_t) ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/sparc64/dl.c b/grub-core/kern/sparc64/dl.c
index 739be4717..60ce183fb 100644
--- a/grub-core/kern/sparc64/dl.c
+++ b/grub-core/kern/sparc64/dl.c
@@ -177,7 +177,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/x86_64/dl.c b/grub-core/kern/x86_64/dl.c
index 3a73e6e6c..8d2800c9e 100644
--- a/grub-core/kern/x86_64/dl.c
+++ b/grub-core/kern/x86_64/dl.c
@@ -107,7 +107,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] style: Format string macro should have a space between quotes
Signed-off-by: Glenn Washburn
---
grub-core/disk/luks2.c | 38 +++---
grub-core/fs/xfs.c | 4 ++--
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index 7460d7b58..125e8609a 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -272,7 +272,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_digest_t *d, grub_luks2_s
grub_json_getuint64 (>idx, , NULL) ||
grub_json_getchild (, , 0) ||
luks2_parse_keyslot (k, ))
-return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot index
%"PRIuGRUB_SIZE, keyslot_json_idx);
+return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot index
%" PRIuGRUB_SIZE, keyslot_json_idx);
/* Get digest that matches the keyslot. */
if (grub_json_getvalue (, root, "digests") ||
@@ -284,13 +284,13 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_digest_t *d, grub_luks2_s
grub_json_getuint64 (>idx, , NULL) ||
grub_json_getchild (, , 0) ||
luks2_parse_digest (d, ))
- return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index
%"PRIuGRUB_SIZE, json_idx);
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index
%" PRIuGRUB_SIZE, json_idx);
if ((d->keyslots & (1 << k->idx)))
break;
}
if (json_idx == size)
- return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot
\"%"PRIuGRUB_UINT64_T"\"", k->idx);
+ return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot \"%"
PRIuGRUB_UINT64_T "\"", k->idx);
/* Get segment that matches the digest. */
if (grub_json_getvalue (, root, "segments") ||
@@ -302,13 +302,13 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_digest_t *d, grub_luks2_s
grub_json_getuint64 (>idx, , NULL) ||
grub_json_getchild (, , 0) ||
luks2_parse_segment (s, ))
- return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment
index %"PRIuGRUB_SIZE, json_idx);
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment
index %" PRIuGRUB_SIZE, json_idx);
if ((d->segments & (1 << s->idx)))
break;
}
if (json_idx == size)
-return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest
\"%"PRIuGRUB_UINT64_T"\"", d->idx);
+return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest \"%"
PRIuGRUB_UINT64_T "\"", d->idx);
return GRUB_ERR_NONE;
}
@@ -623,18 +623,18 @@ luks2_recover_key (grub_disk_t source,
if (keyslot.priority == 0)
{
- grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due
to priority\n", keyslot.idx);
+ grub_dprintf ("luks2", "Ignoring keyslot \"%" PRIuGRUB_UINT64_T "\"
due to priority\n", keyslot.idx);
continue;
}
- grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n",
keyslot.idx);
+ grub_dprintf ("luks2", "Trying keyslot \"%" PRIuGRUB_UINT64_T "\"\n",
keyslot.idx);
/* Sector size should be one of 512, 1024, 2048, or 4096. */
if (!(segment.sector_size == 512 || segment.sector_size == 1024 ||
segment.sector_size == 2048 || segment.sector_size == 4096))
{
- grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" sector"
-" size %"PRIuGRUB_UINT64_T" is not one of"
+ grub_dprintf ("luks2", "Segment \"%" PRIuGRUB_UINT64_T "\" sector"
+" size %" PRIuGRUB_UINT64_T " is not one of"
" 512, 1024, 2048, or 4096\n",
segment.idx, segment.sector_size);
continue;
@@ -650,9 +650,9 @@ luks2_recover_key (grub_disk_t source,
if (max_crypt_sectors < crypt->offset_sectors)
{
- grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has offset"
-" %"PRIuGRUB_UINT64_T" which is greater than"
-" source disk size %"PRIuGRUB_UINT64_T","
+ grub_dprintf ("luks2", "Segment \"%" PRIuGRUB_UINT64_T "\" has offset"
+" %" PRIuGRUB_UINT64_T " w
[PATCH v5 12/13] error: Use format code llu for 64-bit uint bp->blk_prop in grub_error
For some reason PRIuGRUB_UINT64_T is not expanding to llu, but to lu, which
causes the format string check to fail. Use literal and force cast until
this is debugged.
Signed-off-by: Glenn Washburn
---
grub-core/fs/zfs/zfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index b6e1e178d..a3691d220 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -1869,8 +1869,8 @@ zio_read (blkptr_t *bp, grub_zfs_endian_t endian, void
**buf,
{
if (BPE_GET_ETYPE(bp) != BP_EMBEDDED_TYPE_DATA)
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
- "unsupported embedded BP (type=%u)\n",
- BPE_GET_ETYPE(bp));
+ "unsupported embedded BP (type=%llu)\n",
+ (long long unsigned int)BPE_GET_ETYPE(bp));
lsize = BPE_GET_LSIZE(bp);
psize = BF64_GET_SB(grub_zfs_to_cpu64 ((bp)->blk_prop, endian), 25, 7,
0, 1);
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v5 11/13] error: Use format code PRIuGRUB_UINT64_T for 64-bit typed fileblock in grub_error
Signed-off-by: Glenn Washburn
---
grub-core/fs/hfsplus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 9c4e4c88c..b538f52e0 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -188,7 +188,8 @@ grub_hfsplus_read_block (grub_fshelp_node_t node,
grub_disk_addr_t fileblock)
|| !nnode)
{
grub_error (GRUB_ERR_READ_ERROR,
- "no block found for the file id 0x%x and the block offset
0x%x",
+ "no block found for the file id 0x%x and the block"
+ " offset 0x%"PRIuGRUB_UINT64_T,
node->fileid, fileblock);
break;
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Threading of patch series (was: [PATCH v6 00/14] error: Do compile-time format string checking on grub>)
Hi Paul, On Sat, 6 Mar 2021 07:59:18 +0100 Paul Menzel wrote: > Dear Glenn, > > > Am 06.03.21 um 00:15 schrieb Glenn Washburn: > > On Fri, 5 Mar 2021 17:27:01 +0100 Daniel Kiper wrote: > > […] > > >> By the way, my I ask you once again to send each patch series as > >> separate thread. Now you are attaching all patch sets to one cover > >> letter which is confusing. Please stop doing that. > > > > How do you pull patch series from the mailing list? I'm curious if > > this is messing with that. Also what email client do you use? > > > > You are correct that I'm attaching all new versions of the patch > > series to one cover letter, but each patch series also has its own > > cover letter. So I don't consider the cover letter that is the root > > of the thread to be the cover letter for the new patch series. > > > > I can't find our prior correspondence. I recall saying that the > > patchset series in question had been not done in a less than ideal > > way because I had start by replying to the cover letter of the prior > > patchset and then switched to replying to a particular patchset > > cover letter. This was because with experience I determined that > > attaching to the thread of the next version of a patchset to the > > cover letter of the first version of the patchset looked much nicer > > in my browser. I also recall saying that I'd do this in the future > > to see if it worked well doing it properly from the start. > > > > My goal is to keep all versions of a patchset together in a client > > with tree view of threads (eg. my mail client claws-mail). This > > makes it easy to go back to a prior patchset to look at comments. I > > also wanted to meet this goal in an aesthetically pleasing way. The > > first attempt which attached a new version of a patchset to its > > priors cover letter did not meet this because it created a deep > > tree for patchsets with lots of revisions. However, attaching each > > new patchset to the cover letter of the first patchset, keeps > > thread tree to a minimum. It also makes it to collapse only > > certain patchset versions (aside from the first). Also, since I > > have threads sorted by thread date, I see patchsets ordered from > > most recent to least recent, which it how I like to order all my > > emails. > > > > The current case does not strictly adhere to these rules because I'm > > taking v4 as the initial patchset, which perhaps may be the source > > of some confusion. Other than that I think its worked out nicely. > > > > So I'm curious if this is causing some issue with tooling. And I'm > > curious what exactly is causing confusion? In my browser its fairly > > obvious that the root of the thread is the cover letter for v4 of > > the patch series and that the cover letters of attached patch > > series are different, noted by a different version number. > > At least here, Mozilla Thunderbird 78.8.0 is only able to collapse > the top thread and not sub threads. Thanks for the feedback. I don't suppose that's a big loss of functionality with respect the way I'm sending new patchset versions. If the thread is super long, just collapse the whole thread. Perhaps I'm failing to see the issue. What could be annoying is if you want to go back several patch revisions and compare with the current one. But that would still be easier this way than the current practice because the previous version is likely to be between a lot of other threads. Now it definitely would be more problematic for clients that can not collapse threads (perhaps text-based ones?). Does anyone have this issue? > The mailing list archive does not seem to care [1], though that might > be because the v4 patch set cover letter is in a different month. Yes, I believe that the mailing list only threads on a per month basis. I've actually been annoyed by this in the past with other list using the same setup when trying to read multi-month threads. > Anyway, as *displaying* of threading is not defined and different > between user agents, maybe it’s better to not rely on that. I'm confused by this. I accept the premise, but I don't understand "rely" in this context, which I interpret as "to assume". I _am_ relying on the behavior of my email client for my personal usage. I understand your comment to be analogous to saying that I am relying on the display capabilities of others email clients when I send a patch with --thread in git send-email because there may be someone on the list who chooses to use an email client that does not support a threaded view. I'm less concerned with the capabilities of other clients than I am for how this ne
Re: [PATCH] disk/pata: Prepend log message with PATA
On Mon, 1 Mar 2021 13:43:06 +0100
Paul Menzel wrote:
> Seeing
>
> no device connected
>
> it’s not clear, what component this belongs to. So prepend it to log:
>
> PATA: no device connected
I ran in to this just several days ago in the ehci_test, which may be
unrelated to how its showing up for you. The ehci_test runs the
"nativedisk" command to pull in the ehci module, but this also loads
the pata module and others. Without looking much further, I'm thinking
the pata module is printing that message when no pata disks are found.
There's only a usb mass storage disk and ATAPI(?) cdrom configured in
qemu.
So in addition to this patch, it would be nice to not have the pata
module print that message when no devices are found. Perhaps a
grub_dprintf would be better.
Glenn
Reviewed-by: Glenn Washburn
> Signed-off-by: Paul Menzel
> ---
> grub-core/disk/pata.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/disk/pata.c b/grub-core/disk/pata.c
> index 23eef2be1..f9a3d3132 100644
> --- a/grub-core/disk/pata.c
> +++ b/grub-core/disk/pata.c
> @@ -294,7 +294,7 @@ check_device (struct grub_pata_device *dev)
>grub_uint8_t sec = grub_pata_regget (dev, GRUB_ATA_REG_SECTORS);
>grub_dprintf ("ata", "sectors=0x%x\n", sec);
>if (sec != 0x5A)
> -return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "no device
> connected");
> +return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "PATA: no device
> connected");
>/* The above test may detect a second (slave) device
> connected to a SATA controller which supports only one
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v6 00/14] error: Do compile-time format string checking on grub>
On Fri, 5 Mar 2021 17:27:01 +0100 Daniel Kiper wrote: > On Thu, Mar 04, 2021 at 06:22:31PM -0600, Glenn Washburn wrote: > > Daniel, you mentioned wanting a separate patch series which is the > > real fix for patch #12. I've added it to this patch series, since > > they go together. I can send the single patch as a separate thread > > if that still desirable. > > For all Reviewed-by: Daniel Kiper > > I will take all stuff up to #13. The #14 will be applied after 2.06 > release. Great! > Thank you for fixing these issues. > > By the way, my I ask you once again to send each patch series as > separate thread. Now you are attaching all patch sets to one cover > letter which is confusing. Please stop doing that. How do you pull patch series from the mailing list? I'm curious if this is messing with that. Also what email client do you use? You are correct that I'm attaching all new versions of the patch series to one cover letter, but each patch series also has its own cover letter. So I don't consider the cover letter that is the root of the thread to be the cover letter for the new patch series. I can't find our prior correspondence. I recall saying that the patchset series in question had been not done in a less than ideal way because I had start by replying to the cover letter of the prior patchset and then switched to replying to a particular patchset cover letter. This was because with experience I determined that attaching to the thread of the next version of a patchset to the cover letter of the first version of the patchset looked much nicer in my browser. I also recall saying that I'd do this in the future to see if it worked well doing it properly from the start. My goal is to keep all versions of a patchset together in a client with tree view of threads (eg. my mail client claws-mail). This makes it easy to go back to a prior patchset to look at comments. I also wanted to meet this goal in an aesthetically pleasing way. The first attempt which attached a new version of a patchset to its priors cover letter did not meet this because it created a deep tree for patchsets with lots of revisions. However, attaching each new patchset to the cover letter of the first patchset, keeps thread tree to a minimum. It also makes it to collapse only certain patchset versions (aside from the first). Also, since I have threads sorted by thread date, I see patchsets ordered from most recent to least recent, which it how I like to order all my emails. The current case does not strictly adhere to these rules because I'm taking v4 as the initial patchset, which perhaps may be the source of some confusion. Other than that I think its worked out nicely. So I'm curious if this is causing some issue with tooling. And I'm curious what exactly is causing confusion? In my browser its fairly obvious that the root of the thread is the cover letter for v4 of the patch series and that the cover letters of attached patch series are different, noted by a different version number. Glenn > Daniel > > > Changes since v5 > > * Fix formatting issues with spaces around format string macros > > and casts > > * Add extra patch #14 which is the better fix for #12, but needs > > more testing > > > > Glenn > > > > Glenn Washburn (14): > > misc: Format string for grub_error should be a literal > > error: grub_error missing format string argument > > error: grub_error format string add missing format code > > dmraid_nvidia: Format string error in grub_error > > grub_error: Use format code PRIuGRUB_SIZE for variables of type > > grub_size_t > > pgp: Format code for grub_error is incorrect > > efi: Format string error in grub_error > > error: Use PRI* macros to get correct format string code across > > architectures > > error: Use format code PRIxGRUB_UINT64_T for 64-bit uint argument > > in grub_error > > error: Use format code PRIxGRUB_UINT64_T for 64-bit arg in > > grub_error error: Use format code PRIuGRUB_UINT64_T for 64-bit > > typed fileblock in grub_error > > error: Use format code llu for 64-bit uint bp->blk_prop in > > grub_error error: Do compile-time format string checking on > > grub_error zfs: Use grub_uint64_t instead of 1ULL in BF64_*CODE > > macros ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/7] Add xHCI USB support
Good day Patrick, On Wed, 24 Feb 2021 07:46:48 +0100 Patrick Rudolph wrote: > Hi Glenn, > yes it's the same patch series, but has been cleaned and improved a > lot. I've addressed most of the comments received earlier. I see that I implied that you hadn't and I didn't mean to. I only meant to suggest that there were possible unaddressed issues. Glad to hear those were already taken care of. > As stated in the commit message, USB 3.1 and USB 3.2 are not > tested, as I lack hardware to test this. I'm not going to look into > this any further into this as it works fine with USB 3.0 compliant > xhci controllers. > > I'l fix the remaining issues and publish a new patch series. Great. Are you thinking of adding in some tests? If not, I would appreciate knowing the qemu opts used to create the virtual machine (I'm unsure how to turn on xHCI in qemu). Glenn > > On Fri, Feb 19, 2021 at 7:11 PM Glenn Washburn > wrote: > > > > Hi Patrick, > > > > Thanks for the contribution. I think this would be a great addition > > to GRUB. However, there are a few issues I see at the moment. > > > > On Mon, 7 Dec 2020 08:41:20 +0100 > > Patrick Rudolph wrote: > > > > > Add basic support for xHCI USB controllers and non root xHCI hubs. > > > The motivation is to use this code on platforms that do not > > > provide user input by runtime services (like BIOS or UEFI > > > platform) do. This is the case when GRUB is used as coreboot > > > payload for example. > > > > > > The code is based on seabios implementation, but has been heavily > > > modified to match grubs internals. > > > > Are these the same changes as in > > https://github.com/Nitrokey/grub.git referenced in a previous > > email? I'm asking because there were some comments that were > > unaddressed. So, if it is, could you please address Thomas's email > > (I haven't checked to see if they are still relevant if so)? (see: > > https://marc.info/?l=grub-devel=159769164332293) > > > > Searching in the mailing list, I found this attempt in early 2017. > > Having not looked at the code at all, could this help in addressing > > some of Thomas's concerns? > > https://github.com/bjornfor/grub/tree/add-coreboot-xhci-driver-2nd-attempt-v2 > > > > I've run your code through the GitLab CI I've configured, and there > > was a build failure on x86_64-efi. Looks like some implicit type > > casting issues. I believe it failed for both gcc 8.1 and 10.1. Here > > is the error log when using gcc 10.1: > > https://gitlab.com/gwashburn/grub/-/jobs/1025317154#L594 > > > > Also, I think that any xhci support should be accompanied by passing > > make check tests. See uhci_test for an example of how this might be > > done. Since you've already tested on qemu, I think you're 90% of the > > way there in making some tests. Both of the ones listed before > > would be nice. If you'd like some guidance or help, let me know. > > > > Glenn > > > > > Changes done in version 2: > > > * Code cleanup > > > * Code style fixes > > > * Don't leak memory buffers > > > * Compile without warnings > > > * Add more defines > > > * Add more helper functions > > > * Don't assume a 1:1 virtual to physical mapping > > > * Flush cachelines after writing buffers > > > * Don't use hardcoded page size > > > * Proper scratchpad register setup > > > * xHCI bios ownership handoff > > > > > > Changes done in version 3: > > > * Several bug fixes for real hardware > > > * Fixed a race condition detecting events, which doesn't appear > > > on qemu based xHCI controllers > > > * Don't accidently disable USB3.0 devices after first command > > > * Support arbitrary protocol speed IDs > > > * Coding style cleanup > > > * Support for non root SuperSpeed hubs > > > > > > Changes Tested: > > > * Qemu system x86_64 > > >* virtual USB HID keyboard (usb-kbd) > > >* virtual USB HID mass storage (usb-storage) > > > * Intel C246 integrated xHCI (Supermicro X11SSH-F) > > >* iKVM HID keyboard > > >* USB3 HID mass storage (controller root port) > > >* External USB HID keyboard > > > > > > TODO: > > > * Test on more hardware > > > * Test on USB3 hubs > > > * Support for USB 3.1 and USB 3.2 controllers > > > > > > Patrick Rudolph (7): > > > grub-core/bus/usb: Parse SuperSpeed companion descriptors > > > usb: Add enum for xHCI &
Re: multiboot2 and module2 boot issues via GRUB2
On Thu, 1 Apr 2021 20:43:46 +0100 Andrew Cooper via Grub-devel wrote: > On 01/04/2021 09:44, Roger Pau Monné wrote: > > On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote: > >> On 01.04.2021 03:06, Roman Shaposhnik wrote: > >>> And the obvious next question: is my EVE usecase esoteric enough > >>> that I should just go ahead and do a custom GRUB patch or is > >>> there a more general interest in this? > >> Not sure if it ought to be a grub patch - the issue could as well > >> be dealt with in Xen, by concatenating modules to form a monolithic > >> initrd. > > I would rather have it done in the loader than Xen, mostly because > > it's a Linux boot specific format, and hence I don't think Xen > > should have any knowledge about it. > > > > If it turns out to be impossible to implement on the loader side we > > should consider doing it in Xen, but that's not my first option. > > Concatenating random things which may or may not be initrds is > absolutely not something Xen should do. We don't have enough context > to do it safely/sensibly. > > Honestly, I like the idea of supporting something like this generally > in grub. Linux already commonly has initrd preparation prepending an > uncompressed microcode CPIO archive, and I can see a usability > advantage from maintaining the initrd fragments separately. > > Looking at the grub manual, this behaviour of the `initrd` command > isn't even documented. Perhaps that should be fixed first, and then > maybe `module2_multi` added too? If you mean that the concatenation of multiple initrds by the `initrd` command isn't documented (not sure that's the behavior your'r referring to), it actually has been recently documented (by me) and is in master. You're probably looking at the last release (2.04) documentation, in which it is indeed not documented. This should change soon after release and the documentation gets updated accordingly. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: RFC: Grub project management
On Thu, 11 Feb 2021 23:50:38 -0500 Eli Schwartz wrote: > On 2/11/21 10:53 PM, Glenn Washburn wrote: > > For patches that people don't want to disappear on the list, I > > think a merge request can help mitigate that. Also since the merge > > request is effectively a whole commit tree, instead of some free > > floating diffs, we can know precisely which commit its based off of > > (master? or last stable?). > > > > This is not intended to change the current patch submission > > requirements for the project. Patches will still need to be sent to > > the list and will be reviewed on the list. I think for non-trivial > > patches it should be required to make a merge request as well so > > that the CI passing is a requirement. I hope that by enforcing a > > passing CI that maintainer and reviewer time is saved by ensuring > > that patch series pass the sniff test. > > Sending patches to both the mailing list and a gitlab repo "for CI > purposes" seems confusingly redundant. > > It would probably make more sense to use a patchwork configured to run > CI on incoming patches, and maybe even respond to the mailing list > with status reports. > > e.g. > > https://lists.sr.ht/~sircmpwn/sr.ht-dev/patches/20108 > > (Using a patchwork also prevents patches from disappearing!) There are two main parts to this work, the CI and the merge request automation. As I see it, your issue is not with the CI, but the merge requests. And you're right the merge request part is not ideal. There is redundancy, but I don't think its that big of a deal. There does come more confusion when determining what patch series version the merge request is currently at and testing. This could be mitigated by having the title of the merge updated when the merge branch is updated or editing the merge request to use a new branch of the changes which has the version number in its name. Sure a few extra steps, but I don't see it as too much to ask. Plus, we're putting the responsibility on the merge author and not project maintainers. And yes, someone can in bad faith abuse the system to waste maintainer time, but they'll quickly be rooted out. Also, I was looking for a solution that didn't require me hosting anything and I don't know of a free hosted patchwork service. It looks like sourcehut fits the bill. Now I'm curious if there's a reason that GRUB isn't already using that service, even if unofficially. Perhaps, I'll look in to switching to that service. I think a lot of the work done in my GitLab CI changes could be reused for other CI systems or we can use just the CI part of these changes. That GitLab repo should be setup already to trigger a CI pipeline whenever master changes (but only once the .gitlab-ci.yml file is in master). Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: RFC: Grub project management
On Fri, 12 Feb 2021 17:35:05 -0500 Eli Schwartz wrote: > On 2/12/21 4:58 PM, Glenn Washburn wrote: > > And you're right the merge request part is not ideal. There > > is redundancy, but I don't think its that big of a deal. There does > > come more confusion when determining what patch series version the > > merge request is currently at and testing. This could be mitigated > > by having the title of the merge updated when the merge branch is > > updated or editing the merge request to use a new branch of the > > changes which has the version number in its name. Sure a few extra > > steps, but I don't see it as too much to ask. Plus, we're putting > > the responsibility on the merge author and not project maintainers. > > And yes, someone can in bad faith abuse the system to waste > > maintainer time, but they'll quickly be rooted out. > > > > Also, I was looking for a solution that didn't require me hosting > > anything and I don't know of a free hosted patchwork service. It > > looks like sourcehut fits the bill. Now I'm curious if there's a > > reason that GRUB isn't already using that service, even if > > unofficially. Perhaps, I'll look in to switching to that service. > > https://libreplanet.org/wiki/FSF_2020_forge_evaluation > > There are several interesting options for "we can do better than > savannah these days". Sourcehut is one of the software forges under > review. > > However as far as I know, there's no unified approach to this, so the > real question reduces back down to "why isn't there something, > regardless of what" -- and the answer is likely something along the > lines of "no one pushed for it, therefore inertia". I poked around on a sourcehut account a little and its not obvious that can can integrate with an existing mailing list. It looks like it expects to host the mailinglist. I'm thinking there might be a way to configure it act like a peer mailinglist, where it would receive email from grub-devel and emails it received would be sent to grub-devel. Reading more about patchwork, it seems to have its own set of issues, partly revolving around using a mailing list of development as we do. see: https://lwn.net/Articles/773456/ > Aside: the evaluation is *very* critical of gitlab.com, though it > considers self-hosted gitlab CE should alleviate a bunch of the listed > concerns. That evaluation wasn't clear on where the captcha was used. I can't recall ever seeing a captcha on their site, certainly not on a regular basis. I wonder how hard the GNU position would be against using GitLab in the manner I suggest (modifying my original comment about requiring merge requests for CI, but having it optional). Most of their complaints seem to be around javascript. Would that be alleviated by using scripts to do things via the API? > sourcehut and pagure are the likely contenders for a GNU/FSF endorsed > forge, and sourcehut is the only software forge I'm aware of > *anywhere* that considers mailing lists to be a/the core development > workflow (and therefore integrates a patchwork). > > > I think a lot of the work done in my GitLab CI changes could be > > reused for other CI systems or we can use just the CI part of these > > changes. That GitLab repo should be setup already to trigger a CI > > pipeline whenever master changes (but only once the .gitlab-ci.yml > > file is in master). > > Yeah, having a somewhat independent script to run CI builds is good, > it helps avoid getting locked into specific CI providers. :) > > As we've seen from e.g. > > https://www.theregister.com/2020/11/02/travis_ci_pricng/ > https://news.ycombinator.com/item?id=25338983 > > Free CI is not guaranteed to stick around... Nope, nothing is. But GitLab's may last longer than others that have CI as a core business. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: RFC: Grub project management
On Fri, 12 Feb 2021 17:16:39 -0500 Konrad Rzeszutek Wilk wrote: > On Thu, Feb 11, 2021 at 09:53:37PM -0600, Glenn Washburn wrote: > > I believe I speak for more than just myself when I say that the > > current development process leaves much to be desired. GRUB has > > been in a feature freeze since last March, with the release being > > pushed rescheduled several times and now looks like its projected > > to be in March (assuming its not pushed back again). I get the > > impression that Daniel is overloaded with non-GRUB > > responsibilities. I also wonder if part of the issue is concerned > > about making a release which he believes hasn't had sufficient > > testing. > > > > Daniel, could you please clarify what needs to be done to get the > > release out the door and what the community can do to help expedite > > this process? > > What I am understanding is that Daniel is the bottlenck here. That is > his reviews of patches (and fixes up) are what is holding up the > acceptance? Its not clear to me exactly what the nature of the bottleneck is. I would like some clarity on this. I'm frustrated at what appears to me to be a lack of transparency. So I'm left guessing at what the needs are. > > > > In the meantime, patches are being sent to the list which will not > > be considered for acceptance until the release is out and feature > > freeze lifted. Does anyone believe that someone is going to go back > > to last March and check all the submitted patches to see about > > including them? > > Daniel (from the years I have worked with him) is very detailed > person who has a nice workflow to make sure they are there. That's great to hear. > > I judge the answer to be no. So patches will be continually falling > > into a blackhole. What we need is an issue/merge management system > > so > > Do you have an example of a particular patchset that went into a > blackhole? I can't really say what went into a blackhole because I don't know what's on Daniel's list of patches to circle back to once the feature freeze is lifted. I can give a list of patches that I see as being more at risk (not that I personally care about all of them, but they seem in concept worthy of inclusion). * [PATCH 0/4] UEFI IPv6 and DHCPv6 support Apparently this was posted back in 2016 and never merged either, since the author works for SuSE. He'll probably come back in another 4 years if its forgotten about * [PATCH] minix: avoid mistakenly probing ext2 filesystems There was no review of this patch. The author appears to be from a company, so perhaps he'll keep pushing. * [PATCH v2 0/7] support >512b sector disks with old/buggy firmware Also not reviewed, not super active author. * search: Support searching for partition UUID with --part-uuid Never replied to. Author does not appear to be backed by a company. * [PATCH] Cryptomount support for key files and detached header I reviewed this patch and recommended against it being accepted as is, but having something like GitLab's merge requests where a merge request was submitted and closed, still keeps track of this for other users that would want to use this patch. * [PATCH] search: Support searching for GPT partition label with --part-label Not reviewed * [PATCH v1] fix kernel cmdline corruption Will this get dropped like in 2018? * v7 for detached headers and key files I don't think these should be included as is, but these patches have been attempted to be merged in one form or another since 2015. Even if they weren't accepted, having a closed out merge request allows grub users like myself to search for detached header and find the closed MR and potentially use it. The barrier to entry for someone to search the mailing list, get the patches, and finding an upstream commit that they'll apply against is a lot higher. * [PATCH 0/5] Testing improvements These are my patches from almost 2 months ago. They haven't been reviewed. Ok, now there's a list, so these might now avoid the blackhole. There's lots of things that I'm finding on the list of potentially useful, unaccepted patches. > > things don't get lost, or can easily be found. > > I would assume that folks who care about their patches would repost > once the release goes out? Depends on what level of care you're talking about. They obviously care about their patch or they wouldn't have taken the time to create it and submit it to the list. What I think what you mean is "they don't care enough to baby-sit the patch for as long as it takes for the GRUB project to get to the point where non-critical patches are accepted again." And how do you know that by the time these patches are gotten back to that the authors will be in a place where they will be willing or able to pick the
[CI 09/17] grub-shell: Put all generated files into working dir and use better file names
When running tests there are many invocations of grub-shell, and because the
output files are all random names in the same tmp directory, it becomes more
work to figure out which files went with which grub-shell invocations. So
all generated files from one invocation of grub-shell are put into a
randomly named directory, so as not to collide with other grub-shell
invocations. And now that the generated files can be put in a location where
they will not get stepped on, and they can be named sensible names.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index e80471126..8c6ed76d7 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -71,6 +71,8 @@ exec_show_error () {
fi
}
+work_directory=${WORKDIR:-`mktemp -d "${TMPDIR:-/tmp}/grub-shell.XX"`}
|| exit 1
+
. "${builddir}/grub-core/modinfo.sh"
qemuopts="${GRUB_QEMU_OPTS}"
serial_port=com0
@@ -80,7 +82,7 @@ pseries=n
disk="hda "
case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
*-emu)
- device_map=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+ device_map="$work_directory/device.map"
boot=emu
console=console
disk=0
@@ -318,14 +320,14 @@ for option in "$@"; do
done
if [ "x${source}" = x ] ; then
-tmpfile=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+tmpfile="$work_directory/testcase.cfg"
while read REPLY; do
echo "$REPLY" >> ${tmpfile}
done
source=${tmpfile}
fi
-cfgfile=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+cfgfile="$work_directory/grub.cfg"
cat <${cfgfile}
grubshell=yes
enable_progress_indicator=0
@@ -357,7 +359,8 @@ if [ $trim = 1 ]; then
echo "echo; echo $trim_head" >>${cfgfile}
fi
-rom_directory=`mktemp -d "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+rom_directory="$work_directory/rom_directory"
+mkdir -p "$rom_directory"
for mod in ${modules}
do
@@ -378,7 +381,7 @@ test -z "$debug" || echo "GRUB script: ${cfgfile}" >&2
test -z "$debug" || echo "GRUB testcase script: ${tmpfile}" >&2
test -z "$debug" || echo "Boot device: ${boot}" >&2
-isofile=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+isofile="$work_directory/grub.iso"
test -z "$debug" || echo "GRUB ISO file: ${isofile}" >&2
test -z "$debug" || echo "GRUB ROM directory: ${rom_directory}" >&2
@@ -450,7 +453,7 @@ if [ x$boot = xmips_qemu ]; then
fi
if [ x$boot = xcoreboot ]; then
-imgfile=`mktemp "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+imgfile="$work_directory/coreboot.img"
cp "${GRUB_COREBOOT_ROM}" "${imgfile}"
"${GRUB_CBFSTOOL}" "${imgfile}" add-payload -f
"${rom_directory}/coreboot.elf" -n fallback/payload
bootdev="-bios ${imgfile}"
@@ -493,14 +496,15 @@ copy_extra_files() {
}
if [ x$boot = xnet ]; then
-netdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+netdir="$work_directory/netdir"
+mkdir -p "$netdir"
pkgdatadir="@builddir@" "@builddir@/grub-mknetdir"
"--grub-mkimage=${builddir}/grub-mkimage" "--directory=${builddir}/grub-core"
"--net-directory=$netdir" ${mkrescue_args} > /dev/null
cp "${cfgfile}" "$netdir/boot/grub/grub.cfg"
cp "${source}" "$netdir/boot/grub/testcase.cfg"
[ -z "$files" ] || copy_extra_files "$netdir" $files
timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -boot n -net
"user,tftp=$netdir,bootfile=/boot/grub/${grub_modinfo_target_cpu}-${grub_modinfo_platform}/core.$netbootext"
-net nic | cat | tr -d "\r" | do_trim
elif [ x$boot = xemu ]; then
-rootdir="$(mktemp -d "${TMPDIR:-/tmp}/tmp.XX")"
+rootdir="$work_directory/rootdir"
grubdir="$rootdir/boot/grub"
mkdir -p "$grubdir/fonts"
mkdir -p "$grubdir/themes"
@@ -515,7 +519,7 @@ elif [ x$boot = xemu ]; then
cp "${cfgfile}" "$grubdir/grub.cfg"
cp "${source}" "$grubdir/testcase.cfg"
[ -z "$files" ] || copy_extra_files "$rootdir" $files
-roottar="$(mktemp "${TMPDIR:-/tmp}/tmp.XX")"
+roottar="$work_directory/root.tar"
(cd "$rootdir"; tar cf "$roottar" .)
@builddir@/grub-core/grub-emu -m "$device_map" --memdisk "$roottar" -r
memdisk -d "/boot/grub" | tr -d "\r" | do_trim
test -n "$debug" || rm -rf "$rootdir"
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 07/17] grub-shell: Only show grub-mkrescue output if it returns an error
The previous behavior ignored an error and the output from grub-mkrescue.
This made it a pain to discover that grub-mkrescue was the reason that tests
which rely on grub-shell were failing. Even after discovering grub-mkrescue
was the culprit, there was no output to indicate why it was failing. It
turns out that grub-mkrescue is a thin wrapper around xorriso. So if you do
not have xorriso installed it will fail with an error message about not
being able to find xorriso.
This change will allow grub-mkrescue output to be written to stderr, only if
grub-mkrescue fails. If grub-mkrescue succeeds, there will be no output
from grub-mkrescue so as not to interfere with the functioning of tests.
This change should have no effect on the running of tests or other uses of
grub-shell as it only modifies the error path.
Also, if grub-mkrescue fails, the script exits early. Since grub-shell
needs the iso image created by grub-mkresue to boot the qemu instance, a
failure here should be considered fatal.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 602b16f3e..9d8c417da 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -60,6 +60,17 @@ Report bugs to .
EOF
}
+# Exec given argv and only show its output on STDERR if it returns an
+# error status.
+exec_show_error () {
+v=`$@ 2>&1`
+ret=$?
+if [ "$ret" != 0 ]; then
+echo "$v" >&2
+exit $ret
+fi
+}
+
. "${builddir}/grub-core/modinfo.sh"
qemuopts="${GRUB_QEMU_OPTS}"
serial_port=com0
@@ -383,13 +394,15 @@ if test -z "$debug"; then
fi
if [ x$boot != xnet ] && [ x$boot != xemu ]; then
-pkgdatadir="@builddir@" "@builddir@/grub-mkrescue" "--output=${isofile}"
"--override-directory=${builddir}/grub-core" \
+pkgdatadir="@builddir@" \
+exec_show_error "@builddir@/grub-mkrescue" "--output=${isofile}" \
+ "--override-directory=${builddir}/grub-core" \
--rom-directory="${rom_directory}" \
--locale-directory="@srcdir@/po" \
--themes-directory="@srcdir@/themes" \
$mkimage_extra_arg ${mkrescue_args} \
"/boot/grub/grub.cfg=${cfgfile}" "/boot/grub/testcase.cfg=${source}" \
- ${files} >/dev/null 2>&1
+ ${files} || exit $?
fi
if [ x$boot = xhd ]; then
if [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = arm64-efi ]
|| [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = arm-efi ]; then
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 05/17] grub-shell: Allow specifying non-default trim line contents
This will be useful for tests that have unwanted output from setup. This is
not documented because its only intended to be internal at the moment. Also,
--no-trim is allowed to explicitly turn off trim.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 93e9f5148..b409962f1 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -32,6 +32,7 @@ PATH="${builddir}:$PATH"
export PATH
trim=0
+trim_head=664cbea8-132f-4770-8aa4-1696d59ac35c
# Usage: usage
# Print the usage.
@@ -212,8 +213,13 @@ for option in "$@"; do
echo "$0 (GNU GRUB ${PACKAGE_VERSION})"
exit 0 ;;
--trim)
- trim=1
+ trim=1 ;;
+--trim=*)
+ trim=2
+ trim_head=`echo "$option" | sed -e 's/--trim=//' -e 's/,/ /g'`
;;
+--no-trim)
+ trim=0 ;;
--debug)
debug=1 ;;
--modules=*)
@@ -336,8 +342,6 @@ terminal_input ${term}
terminal_output ${term}
EOF
-trim_head=664cbea8-132f-4770-8aa4-1696d59ac35c
-
if [ $trim = 1 ]; then
echo "echo $trim_head" >>${cfgfile}
fi
@@ -452,8 +456,8 @@ fi
do_trim ()
{
-if [ $trim = 1 ]; then
- awk '{ if (have_head == 1) print $0; }
/664cbea8-132f-4770-8aa4-1696d59ac35c/ { have_head=1; }'
+if [ $trim = 1 ] || [ $trim = 2 ]; then
+ awk '{ if (have_head == 1) print $0; } /'"$trim_head"'/ { have_head=1;
}'
else
cat
fi
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 02/17] tests: Fix partmap_test for arm*-efi, disk numbering has changed
Perhaps using a newer UEFI firmware is the reason for the created test disk
showing up as hd2 instead of hd3.
Signed-off-by: Glenn Washburn
---
tests/partmap_test.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 6ef518b0a..7353dc70e 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -89,7 +89,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
disk=arc/scsi0/disk0/rdisk0
;;
arm*-efi)
- disk=hd3
+ disk=hd2
;;
*)
disk=hd0
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 16/17] ci: Add .gitlab-ci.yml for continuous integration with Gitlab CI
Signed-off-by: Glenn Washburn
---
.ci/build.sh| 66 +++
.ci/functions.gitlab.sh | 30 +
.ci/functions.sh| 30 +
.ci/make-images.sh | 71 +++
.ci/process-tests.sh| 108
.ci/test.sh | 106
.gitlab-ci.yml | 1184 +++
7 files changed, 1595 insertions(+)
create mode 100755 .ci/build.sh
create mode 100644 .ci/functions.gitlab.sh
create mode 100644 .ci/functions.sh
create mode 100755 .ci/make-images.sh
create mode 100755 .ci/process-tests.sh
create mode 100755 .ci/test.sh
create mode 100644 .gitlab-ci.yml
diff --git a/.ci/build.sh b/.ci/build.sh
new file mode 100755
index 0..14dd7f7c5
--- /dev/null
+++ b/.ci/build.sh
@@ -0,0 +1,66 @@
+#!/bin/bash
+
+set -eo pipefail
+
+# Environment variables
+# JOBS: Number of concurrent jobs while building, defaults to number of
+# processors plus 1, unless number of processors is 1 in which case its 1.
+# SRCDIR: Path to source files
+# BUILDDIR: Directory in which to place the build
+# INSTALLDIR: Directory to install binaries
+# ARCH: Architecture to build for
+# PLATFORM: Platform to build for
+# CONFIGURE_OPTS: Extra configure options
+# SHELL_TRACE: Set to 'y' to enable shell tracing
+
+[ "x${SHELL_TRACE}" = "xy" ] && set -x
+
+[ -f "$(dirname "$0")/functions.sh" ] &&
+. "$(dirname "$0")/functions.sh"
+
+[ -f "$(dirname "$0")/functions.$CI_TYPE.sh" ] &&
+. "$(dirname "$0")/functions.$CI_TYPE.sh"
+
+JOBS=${JOBS:-`getconf _NPROCESSORS_ONLN 2> /dev/null || echo 1`};
+[ "$JOBS" == 1 ] || JOBS=$(($JOBS + 1));
+
+TARGET="${ARCH}-${PLATFORM}"
+
+mkdir -pv ${BUILDDIR};
+
+RET=0;
+cd ${BUILDDIR};
+
+# echo -e "#!$SHELL\nREAL_SHELL=\${REAL_SHELL:-\$(readlink
/proc/\$\$/exe)}\nexec \$REAL_SHELL \$SHELL_OPTS \"\$@\""
>${BUILDDIR}/shell-wrapper.sh;
+# cat <${BUILDDIR}/shell-wrapper.sh
+# #!$SHELL
+# REAL_SHELL=\${REAL_SHELL:-$(readlink /proc/\$\$/exe)}
+# exec \$REAL_SHELL \$SHELL_OPTS \"\$@\"
+# EOF
+
+# chmod +x ${BUILDDIR}/shell-wrapper.sh;
+# export CONFIG_SHELL=${BUILDDIR}/shell-wrapper.sh;
+
+start_log -c -n "configure-$TARGET" "Configuring $TARGET";
+ls -la $SRCDIR;
+[ -x "$SRCDIR/configure" ] && $SRCDIR/configure --help;
+$SRCDIR/configure --target=$ARCH --with-platform=$PLATFORM \
+ --prefix=${INSTALLDIR} $CONFIGURE_OPTS || RET=$?;
+end_log -n "configure-$TARGET";
+
+if [ "$RET" -ne 0 ]; then
+ start_log -c -n "showlogs-$TARGET" "Configuring $TARGET failed, showing
logs";
+ cat ${BUILDDIR}/config.log;
+ end_log -n "showlogs-$TARGET";
+ exit $RET;
+fi;
+
+start_log -c -n "build-$TARGET" "Building $TARGET";
+make -j$JOBS || RET=$?;
+end_log -n "build-$TARGET";
+[ "$RET" -ne 0 ] && exit $RET;
+
+start_log -c -n "install-$TARGET" "Installing $TARGET";
+make -j$JOBS install || RET=$?;
+end_log -n "install-$TARGET";
+exit $RET;
\ No newline at end of file
diff --git a/.ci/functions.gitlab.sh b/.ci/functions.gitlab.sh
new file mode 100644
index 0..e9cd2f9be
--- /dev/null
+++ b/.ci/functions.gitlab.sh
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+TXT_RED="\e[31m"
+TXT_YELLOW="\e[33m"
+TXT_CLEAR="\e[0m"
+
+function start_log() {
+ while [ "$#" -gt 0 ]; do
+case "$1" in
+ -c) LOG_COLLAPSE=1; shift;;
+ -n) LOG_NAME="$2"; shift;;
+ *) [ "$#" -eq 1 ] && LOG_MSG="$1"; shift;;
+esac
+ done
+
+ echo -e "section_start:`date
+%s`:${LOG_NAME}${LOG_COLLAPSE:+[collapsed=true]}\r\e[0K${LOG_MSG}"
+}
+
+function end_log() {
+ while [ "$#" -gt 0 ]; do
+case "$1" in
+ -n) LOG_NAME=$2; shift;;
+ *) shift;;
+esac
+ done
+
+ echo -e "section_end:`date +%s`:${LOG_NAME}\r\e[0K"
+}
+
+:;
diff --git a/.ci/functions.sh b/.ci/functions.sh
new file mode 100644
index 0..bba33fb5f
--- /dev/null
+++ b/.ci/functions.sh
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+TXT_RED="\e[31m"
+TXT_YELLOW="\e[33m"
+TXT_CLEAR="\e[0m"
+
+function start_log() {
+ while [ "$#" -gt 0 ]; do
+case "$1" in
+ -c) LOG_COLLAPSE=1; shift;;
+ -n) LOG_NAME="$2"; shift;;
+ *) [ "$#" -eq 1 ] && LOG_MSG="$1"; shift;;
+esac
+ done
+
+ echo -e "Start:${LOG_NAME} ${LOG_MSG}"
+}
+
+function end_log() {
+ while [ "$#" -gt 0 ]; do
+case "$1" in
+ -n) LOG_NAME=$2; shift;;
+ *) shift;;
+esac
+ done
+
+ echo -e "End:${LOG_NAME}"
+}
+
+:;
diff --git a/.ci/make-images.sh b/.ci/make-images.sh
new file mode 100755
index 0..3c7d2f88c
--- /
[CI 17/17] keep: remove a lot of cruft that we probably want to reuse at some point
Signed-off-by: Glenn Washburn
---
.ci/build.sh | 10 -
.ci/process-tests.sh | 11 -
.ci/test.sh | 8 -
.gitlab-ci.yml | 552 ---
4 files changed, 581 deletions(-)
diff --git a/.ci/build.sh b/.ci/build.sh
index 14dd7f7c5..55af74a1b 100755
--- a/.ci/build.sh
+++ b/.ci/build.sh
@@ -31,16 +31,6 @@ mkdir -pv ${BUILDDIR};
RET=0;
cd ${BUILDDIR};
-# echo -e "#!$SHELL\nREAL_SHELL=\${REAL_SHELL:-\$(readlink
/proc/\$\$/exe)}\nexec \$REAL_SHELL \$SHELL_OPTS \"\$@\""
>${BUILDDIR}/shell-wrapper.sh;
-# cat <${BUILDDIR}/shell-wrapper.sh
-# #!$SHELL
-# REAL_SHELL=\${REAL_SHELL:-$(readlink /proc/\$\$/exe)}
-# exec \$REAL_SHELL \$SHELL_OPTS \"\$@\"
-# EOF
-
-# chmod +x ${BUILDDIR}/shell-wrapper.sh;
-# export CONFIG_SHELL=${BUILDDIR}/shell-wrapper.sh;
-
start_log -c -n "configure-$TARGET" "Configuring $TARGET";
ls -la $SRCDIR;
[ -x "$SRCDIR/configure" ] && $SRCDIR/configure --help;
diff --git a/.ci/process-tests.sh b/.ci/process-tests.sh
index ab030cfc2..635436e39 100755
--- a/.ci/process-tests.sh
+++ b/.ci/process-tests.sh
@@ -25,8 +25,6 @@ set -eo pipefail
[ -f "$(dirname "$0")/functions.$CI_TYPE.sh" ] &&
. "$(dirname "$0")/functions.$CI_TYPE.sh"
-# TARGET="${ARCH}-${PLATFORM}"
-
test "x${SHELL_TRACE}" = "xy" && set -x;
if [ -f ${BUILDDIR}/test.success ]; then
echo "Not processing test logs because make check ran successfully";
@@ -96,13 +94,4 @@ fi;
RET=1;
fi;
end_log -n "process-test-$TARGET";
-# if [ -f $CI_PROJECT_DIR/${TARGET}.timedout-failures ]; then
-# sed 's/^/${TARGET}:/' $CI_PROJECT_DIR/${TARGET}.timedout-failures
-# >> $CI_PROJECT_DIR/build/test.timedout.log;
-# fi;
-# if [ -f $CI_PROJECT_DIR/${TARGET}.unexpected-failures ]; then
-# sed 's/^/${TARGET}:/' $CI_PROJECT_DIR/${TARGET}.unexpected-failures
-# >> $CI_PROJECT_DIR/build/test.failed.log;
-# exit 1;
-# fi;
exit ${RET:-0};
diff --git a/.ci/test.sh b/.ci/test.sh
index 3f51de58b..b780ae51f 100755
--- a/.ci/test.sh
+++ b/.ci/test.sh
@@ -85,16 +85,8 @@ if [ -n "$GRUB_SHELL_DEFAULT_DEBUG" ]; then
fi;
TIMEOUT=${TESTS_TIMEOUT};
-function func_strace () {
- strace -y -yy -f -v -s4096 -o >($COMP -9 >
${BUILDDIR}/test.strace.$RANDOM.$COMP) "$@";
-};
-':' STRACE="func_strace";
-
-':' $STRACE ${TIMEOUT:+timeout $TIMEOUT} make -C ${BUILDDIR} -j$JOBS SUBDIRS=
check || RET=$?;
${TIMEOUT:+timeout $TIMEOUT} make -C ${BUILDDIR} -j$JOBS SUBDIRS= check ||
RET=$?;
-':' timeout $TIMEOUT $STRACE make -C ${BUILDDIR} -j$JOBS SUBDIRS=
LOG_COMPILER="${BUILDDIR}/log-tester.sh" check || RET=$?;
[ "$RET" -eq 124 -o "$RET" -eq 137 ] && echo "ERROR":" make check timed out";
-':' gzip -9 ${BUILDDIR}/test.strace.*;
end_log -n "test-$TARGET";
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d4fc2337e..b3f854f61 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -13,76 +13,20 @@ workflow:
- if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH
- when: never
-# This allows running CI/CD pipelines for merge requests
-# include:
-# - template: 'Workflows/MergeRequest-Pipelines.gitlab-ci.yml'
-
-# Variables that can be set by runners:
-# CONFIGURE_OPTS - Extra configure options
-# TESTS_TIMEOUT - Set timeout for make check tests (see man timeout)
-# FAIL_ON_HARD_ERRORS - If set, Hard errors will cause a failure
-# DISABLE_ALL_TESTS - If set, make check tests will be disabled
-# TEST_ALL_TARGETS - If set, failing targets which have been marked disabled
will be enabled
-# TEST_VERBOSITY - Verbosity level when running tests: 1-3
-# STRACE_TESTS - If set, strace individual tests. WARNING: This will cause
testing to take much longer
-#which can cause some test to fail by timing out or having
incorrect timing
-# SHELL_TRACE - If set, turn on shell tracing of everything. NOTE:
TEST_VERBOSITY=3 turns on more targeted
-# shell tracing
-
variables:
CI_TYPE: gitlab
- # CI_DEBUG_TRACE: 'true'
- # GIT_CLONE_PATH: $CI_BUILDS_DIR/$CI_CONCURRENT_ID/grub-$CI_COMMIT_SHORT_SHA
GIT_CLONE_PATH: $CI_BUILDS_DIR/grub-${CI_COMMIT_SHORT_SHA}.git
GIT_STRATEGY: fetch
- # Include all cross toolchain paths, so we can just call them later down.
- #PATH:
"/tmp/qemu-install/bin:/usr/bin:/bin:$CROSS_DIR/gcc-8.1.0-nolibc/aarch64-linux/bin:$CROSS_DIR/gcc-8.1.0-nolibc/arm-linux-gnueabi/bin:$CROSS_DIR/gcc-8.1.0-nolibc/ia64-linux/bin:$CROSS_DIR/gcc-8.1.0-nolibc/mips64-linux/bin:$CROSS_DIR/gcc-8.1.0-nolibc/powerpc64-linux/bin:$CROSS_DIR/gcc-8.1.0-nolibc/riscv32-linux/bin:$CROSS_DIR/gcc-8.1.0-nolibc/riscv64-linux/bin:$CROSS_DIR/gcc-8.1.0-nolibc/sparc64-linux/bin"
-# PACKAGE_CACHE: $CI_BUILDS_DIR/pkgs
PACKAGE_CACHE: $CI_PROJECT_DIR/pkgs
CROSS_DIR: $CI
[CI 14/17] grub-shell: Use malta qemu-mips machine type instead off non-existant indy
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 211bfdb95..332fae704 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -119,7 +119,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}"
in
mips-arc)
boot=cd
qemu=qemu-system-mips64
- qemuopts="-M indy $qemuopts"
+ qemuopts="-M malta $qemuopts"
serial_port=arc/serial0/line0
console=
trim=1
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 15/17] grub-shell: Update qemu UEFI firmware names to be more generic
The current qemu firmware paths for arm-efi and arm64-efi are hardcoded
and not available on Ubuntu/Debian. Make these paths relative and basenames
generic. The idea is that regardless where or what names firmware images
are installed under they can be copied and renamed into a temporary
directory. Then use the -L argument to qemu to point to that directory.
For convenience, the GRUB source directory which build grub-shell will be
added to the list of firmware paths that qemu will search. So all that is
needed is to copy the firmwares to the source directory with potential
renames.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 332fae704..33d39d4dd 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -74,7 +74,7 @@ exec_show_error () {
work_directory=${WORKDIR:-`mktemp -d "${TMPDIR:-/tmp}/grub-shell.XX"`}
|| exit 1
. "${builddir}/grub-core/modinfo.sh"
-qemuopts="${GRUB_QEMU_OPTS}"
+qemuopts="${GRUB_QEMU_OPTS} -L ${srcdir}"
serial_port=com0
serial_null=
halt_cmd=halt
@@ -194,7 +194,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}"
in
boot=hd
console=console
trim=1
- qemuopts="-machine virt -cpu cortex-a57 -bios
/usr/share/qemu-efi/QEMU_EFI.fd $qemuopts"
+ qemuopts="-machine virt -cpu cortex-a57 -bios OVMF-aarch64.fd $qemuopts"
disk="device virtio-blk-device,drive=hd1 -drive if=none,id=hd1,file="
serial_port=
;;
@@ -203,7 +203,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}"
in
boot=hd
console=console
trim=1
- qemuopts="-machine virt -bios /usr/share/ovmf-arm/QEMU_EFI.fd $qemuopts"
+ qemuopts="-machine virt -bios OVMF-arm.fd $qemuopts"
disk="device virtio-blk-device,drive=hd1 -drive if=none,id=hd1,file="
serial_port=efi0
;;
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 12/17] tests: Allow turning on shell tracing from environment variables
This allows turning on shell tracing when its not practical or not possible
to use commandline arguments. Turn on tracing when the envvar is an integer
greater than 1, since these can log a lot of messages.
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 2 ++
tests/util/grub-shell.in | 5 -
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 4213b7bfc..9a5f409e9 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -2,6 +2,8 @@
set -e
+[ "${GRUB_TEST_DEFAULT_DEBUG:-0}" -gt 1 ] && set -x
+
fs="$1"
GRUBFSTEST="@builddir@/grub-fstest"
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 40c62b9fd..3b6bce4c2 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -215,6 +215,7 @@ esac
timeout=${GRUB_SHELL_DEFAULT_TIMEOUT:-60}
mkimage_extra_arg=
+debug=${GRUB_SHELL_DEFAULT_DEBUG}
# Check the arguments.
for option in "$@"; do
@@ -234,7 +235,7 @@ for option in "$@"; do
--no-trim)
trim=0 ;;
--debug)
-debug=1 ;;
+debug=$((debug+1)) ;;
--modules=*)
ms=`echo "$option" | sed -e 's/--modules=//' -e 's/,/ /g'`
modules="$modules $ms" ;;
@@ -319,6 +320,8 @@ for option in "$@"; do
esac
done
+[ "${debug:-0}" -gt 1 ] && set -x
+
if [ "x${source}" = x ] ; then
tmpfile="$work_directory/testcase.cfg"
while read REPLY; do
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 01/13] misc: Format string for grub_error should be a literal
Signed-off-by: Glenn Washburn
---
grub-core/loader/efi/chainloader.c | 2 +-
grub-core/net/tftp.c | 2 +-
grub-core/script/lexer.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/grub-core/loader/efi/chainloader.c
b/grub-core/loader/efi/chainloader.c
index 7b31c3fb9..2e98947a0 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -90,7 +90,7 @@ grub_chainloader_boot (void)
*grub_utf16_to_utf8 ((grub_uint8_t *) buf,
exit_data, exit_data_size) = 0;
- grub_error (GRUB_ERR_BAD_OS, buf);
+ grub_error (GRUB_ERR_BAD_OS, "%s", buf);
grub_free (buf);
}
}
diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
index 7e659a78b..592bfee34 100644
--- a/grub-core/net/tftp.c
+++ b/grub-core/net/tftp.c
@@ -252,7 +252,7 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__
((unused)),
case TFTP_ERROR:
data->have_oack = 1;
grub_netbuff_free (nb);
- grub_error (GRUB_ERR_IO, (char *) tftph->u.err.errmsg);
+ grub_error (GRUB_ERR_IO, "%s", tftph->u.err.errmsg);
grub_error_save (>save_err);
return GRUB_ERR_NONE;
default:
diff --git a/grub-core/script/lexer.c b/grub-core/script/lexer.c
index 57778f881..52004d059 100644
--- a/grub-core/script/lexer.c
+++ b/grub-core/script/lexer.c
@@ -349,7 +349,7 @@ void
grub_script_yyerror (struct grub_parser_param *state, const char *err)
{
if (err)
-grub_error (GRUB_ERR_INVALID_COMMAND, err);
+grub_error (GRUB_ERR_INVALID_COMMAND, "%s", err);
grub_print_error ();
state->err++;
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 02/13] error: grub_error missing format string argument
Its obvious from the error message that the variable named "type" was
accidentally omitted.
Signed-off-by: Glenn Washburn
---
grub-core/parttool/msdospart.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/parttool/msdospart.c b/grub-core/parttool/msdospart.c
index dcbf74e3b..2e2e9d527 100644
--- a/grub-core/parttool/msdospart.c
+++ b/grub-core/parttool/msdospart.c
@@ -127,8 +127,8 @@ static grub_err_t grub_pcpart_type (const grub_device_t dev,
{
dev->disk->partition = part;
return grub_error (GRUB_ERR_BAD_ARGUMENT,
-N_("the partition type 0x%x isn't "
- "valid"));
+N_("the partition type 0x%x isn't valid"),
+ type);
}
mbr.entries[index].type = type;
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 03/13] error: grub_error format string add missing format code
Signed-off-by: Glenn Washburn --- grub-core/video/bochs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grub-core/video/bochs.c b/grub-core/video/bochs.c index 7a249eb21..30ea1bd82 100644 --- a/grub-core/video/bochs.c +++ b/grub-core/video/bochs.c @@ -249,11 +249,11 @@ grub_video_bochs_setup (unsigned int width, unsigned int height, } if (width > BOCHS_MAX_WIDTH) -return grub_error (GRUB_ERR_IO, "width must be at most", +return grub_error (GRUB_ERR_IO, "width must be at most %d", BOCHS_MAX_WIDTH); if (height > BOCHS_MAX_HEIGHT) -return grub_error (GRUB_ERR_IO, "height must be at most", +return grub_error (GRUB_ERR_IO, "height must be at most %d", BOCHS_MAX_HEIGHT); if (width & (BOCHS_WIDTH_ALIGN - 1)) -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 09/13] error: Use format code PRIxGRUB_UINT64_T for 64-bit uint argument in grub_error
Signed-off-by: Glenn Washburn --- grub-core/disk/ata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grub-core/disk/ata.c b/grub-core/disk/ata.c index 685f33a19..c8f350ed3 100644 --- a/grub-core/disk/ata.c +++ b/grub-core/disk/ata.c @@ -219,7 +219,7 @@ grub_ata_setaddress (struct grub_ata *dev, if (dev->sectors_per_track == 0 || dev->heads == 0) return grub_error (GRUB_ERR_OUT_OF_RANGE, -"sector %d cannot be addressed " +"sector %"PRIxGRUB_UINT64_T" cannot be addressed " "using CHS addressing", sector); /* Calculate the sector, cylinder and head to use. */ @@ -232,7 +232,7 @@ grub_ata_setaddress (struct grub_ata *dev, || cylinder > dev->cylinders || head > dev->heads) return grub_error (GRUB_ERR_OUT_OF_RANGE, -"sector %d cannot be addressed " +"sector %"PRIxGRUB_UINT64_T" cannot be addressed " "using CHS addressing", sector); parms->taskfile.disk = 0xE0 | head; -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 10/13] error: Use format code PRIxGRUB_UINT64_T for 64-bit arg in grub_error
The macro ELF_R_TYPE does not change the underlying type. Here its argument
is a 64-bit Elf64_Xword. Make sure the format code matches.
For the riscv architecture, rel->r_info could be either Elf32_Xword or
Elf64_Xword depending on if 32 or 64-bit risc is being built. So cast to
64-bit value regardless.
Signed-off-by: Glenn Washburn
---
grub-core/efiemu/i386/loadcore64.c | 3 ++-
grub-core/kern/arm64/dl.c | 3 ++-
grub-core/kern/ia64/dl.c | 3 ++-
grub-core/kern/riscv/dl.c | 5 +++--
grub-core/kern/sparc64/dl.c| 3 ++-
grub-core/kern/x86_64/dl.c | 3 ++-
6 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/grub-core/efiemu/i386/loadcore64.c
b/grub-core/efiemu/i386/loadcore64.c
index 18facf47f..3e9a71cfd 100644
--- a/grub-core/efiemu/i386/loadcore64.c
+++ b/grub-core/efiemu/i386/loadcore64.c
@@ -122,7 +122,8 @@ grub_arch_efiemu_relocate_symbols64 (grub_efiemu_segment_t
segs,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
- N_("relocation 0x%x is not implemented
yet"),
+ N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/arm64/dl.c b/grub-core/kern/arm64/dl.c
index fb0337319..b04fed18a 100644
--- a/grub-core/kern/arm64/dl.c
+++ b/grub-core/kern/arm64/dl.c
@@ -184,7 +184,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/ia64/dl.c b/grub-core/kern/ia64/dl.c
index ebcf31629..ddf240976 100644
--- a/grub-core/kern/ia64/dl.c
+++ b/grub-core/kern/ia64/dl.c
@@ -137,7 +137,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/riscv/dl.c b/grub-core/kern/riscv/dl.c
index 6fb8385ef..77f5b7648 100644
--- a/grub-core/kern/riscv/dl.c
+++ b/grub-core/kern/riscv/dl.c
@@ -331,8 +331,9 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
-ELF_R_TYPE (rel->r_info));
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
+(grub_uint64_t)ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/sparc64/dl.c b/grub-core/kern/sparc64/dl.c
index 739be4717..60ce183fb 100644
--- a/grub-core/kern/sparc64/dl.c
+++ b/grub-core/kern/sparc64/dl.c
@@ -177,7 +177,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
break;
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
diff --git a/grub-core/kern/x86_64/dl.c b/grub-core/kern/x86_64/dl.c
index 3a73e6e6c..8d2800c9e 100644
--- a/grub-core/kern/x86_64/dl.c
+++ b/grub-core/kern/x86_64/dl.c
@@ -107,7 +107,8 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
default:
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-N_("relocation 0x%x is not implemented yet"),
+N_("relocation 0x%"PRIxGRUB_UINT64_T
+ " is not implemented yet"),
ELF_R_TYPE (rel->r_info));
}
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 10/17] grub-shell: Add grub output logfile with grub-shell --debug
This allows seeing full qemu output of grub-shell, which can be invaluable
when debugging failing tests.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 8c6ed76d7..c6d7860e9 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -379,6 +379,9 @@ echo "${halt_cmd}" >>${cfgfile}
test -z "$debug" || echo "GRUB script: ${cfgfile}" >&2
test -z "$debug" || echo "GRUB testcase script: ${tmpfile}" >&2
+
+goutfile="$work_directory/grub-qemu.log"
+test -z "$debug" || echo "GRUB output log: ${goutfile}" >&2
test -z "$debug" || echo "Boot device: ${boot}" >&2
isofile="$work_directory/grub.iso"
@@ -502,7 +505,7 @@ if [ x$boot = xnet ]; then
cp "${cfgfile}" "$netdir/boot/grub/grub.cfg"
cp "${source}" "$netdir/boot/grub/testcase.cfg"
[ -z "$files" ] || copy_extra_files "$netdir" $files
-timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -boot n -net
"user,tftp=$netdir,bootfile=/boot/grub/${grub_modinfo_target_cpu}-${grub_modinfo_platform}/core.$netbootext"
-net nic | cat | tr -d "\r" | do_trim
+timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -boot n -net
"user,tftp=$netdir,bootfile=/boot/grub/${grub_modinfo_target_cpu}-${grub_modinfo_platform}/core.$netbootext"
-net nic | cat | tr -d "\r" | tee "${goutfile}" | do_trim
elif [ x$boot = xemu ]; then
rootdir="$work_directory/rootdir"
grubdir="$rootdir/boot/grub"
@@ -521,11 +524,11 @@ elif [ x$boot = xemu ]; then
[ -z "$files" ] || copy_extra_files "$rootdir" $files
roottar="$work_directory/root.tar"
(cd "$rootdir"; tar cf "$roottar" .)
-@builddir@/grub-core/grub-emu -m "$device_map" --memdisk "$roottar" -r
memdisk -d "/boot/grub" | tr -d "\r" | do_trim
+@builddir@/grub-core/grub-emu -m "$device_map" --memdisk "$roottar" -r
memdisk -d "/boot/grub" | tr -d "\r" | tee "${goutfile}" | do_trim
test -n "$debug" || rm -rf "$rootdir"
test -n "$debug" || rm -f "$roottar"
else
-timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -${device}"${isofile}" ${bootdev} | cat | tr -d "\r" | do_trim
+timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -${device}"${isofile}" ${bootdev} | cat | tr -d "\r" | tee
"${goutfile}" | do_trim
fi
if [ x$boot = xcoreboot ]; then
test -n "$debug" || rm -f "${imgfile}"
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 03/17] tests: When checking squashfs fstime, use superblock last modified time
Currently, the filesystem timestamp check in grub-fs-tester uses the
squashfs image file's last modified timestamp and checks to see if that
time stamp is within 3 seconds of the superblock timestamp as determined by
grub. The image file's timestamp could be more than 3 seconds off if
mksquashfs takes more than 3 seconds to generate the image, as is the case
on a low power machine. Instead use squashfs tools to get the filesystem
timestamp directly.
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index bfc425e1f..4213b7bfc 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -1351,6 +1351,12 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
# With some abstractions like mdraid flushing to disk
# may be delayed for a long time.
FSTIME="$UMOUNT_TIME";;
+ xsquash*)
+ # Creating the squash image may take more than a few
+ # seconds. Use the more accurate timestamp from the
+ # superblock.
+ FSTIME="$(unsquashfs -s "${FSIMAGEP}0.img" | grep
^Creation | awk '{print $6 " " $7 " " $8 " " $9 " " $10; }')"
+ FSTIME="$(date -d "$FSTIME" -u '+%Y-%m-%d %H:%M:%S')";;
*)
FSTIME="$(TZ=UTC ls --time-style="+%Y-%m-%d_%H:%M:%S"
-l -d "${FSIMAGEP}0.img"|awk '{print $6; }'|sed 's,_, ,g')";;
esac
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 13/17] grub-shell: Add --verbose to mkrescue when $debug is greater than 2
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 3b6bce4c2..211bfdb95 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -404,7 +404,9 @@ fi
if [ x$boot != xnet ] && [ x$boot != xemu ]; then
pkgdatadir="@builddir@" \
-exec_show_error "@builddir@/grub-mkrescue" "--output=${isofile}" \
+exec_show_error "@builddir@/grub-mkrescue" \
+ ${debug:+$([ "$debug" -gt 2 ] && echo -n "--verbose")} \
+ "--output=${isofile}" \
"--override-directory=${builddir}/grub-core" \
--rom-directory="${rom_directory}" \
--locale-directory="@srcdir@/po" \
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 11/17] grub-shell: Set exit status to qemu exit status
This allows us to test if unexpected output in test scripts is because of a
bug in grub, because there was an error in qemu, or qemu was killed due to a
timeout.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index c6d7860e9..40c62b9fd 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -498,6 +498,9 @@ copy_extra_files() {
done
}
+ret=0
+mkfifo "$work_directory/qemu-pipe"
+cat < "$work_directory/qemu-pipe" | tr -d "\r" | tee "${goutfile}" | do_trim &
if [ x$boot = xnet ]; then
netdir="$work_directory/netdir"
mkdir -p "$netdir"
@@ -505,7 +508,7 @@ if [ x$boot = xnet ]; then
cp "${cfgfile}" "$netdir/boot/grub/grub.cfg"
cp "${source}" "$netdir/boot/grub/testcase.cfg"
[ -z "$files" ] || copy_extra_files "$netdir" $files
-timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -boot n -net
"user,tftp=$netdir,bootfile=/boot/grub/${grub_modinfo_target_cpu}-${grub_modinfo_platform}/core.$netbootext"
-net nic | cat | tr -d "\r" | tee "${goutfile}" | do_trim
+timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -boot n -net
"user,tftp=$netdir,bootfile=/boot/grub/${grub_modinfo_target_cpu}-${grub_modinfo_platform}/core.$netbootext"
-net nic > "$work_directory/qemu-pipe" || ret=$?
elif [ x$boot = xemu ]; then
rootdir="$work_directory/rootdir"
grubdir="$rootdir/boot/grub"
@@ -524,11 +527,11 @@ elif [ x$boot = xemu ]; then
[ -z "$files" ] || copy_extra_files "$rootdir" $files
roottar="$work_directory/root.tar"
(cd "$rootdir"; tar cf "$roottar" .)
-@builddir@/grub-core/grub-emu -m "$device_map" --memdisk "$roottar" -r
memdisk -d "/boot/grub" | tr -d "\r" | tee "${goutfile}" | do_trim
+@builddir@/grub-core/grub-emu -m "$device_map" --memdisk "$roottar" -r
memdisk -d "/boot/grub" > "$work_directory/qemu-pipe" || ret=$?
test -n "$debug" || rm -rf "$rootdir"
test -n "$debug" || rm -f "$roottar"
else
-timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -${device}"${isofile}" ${bootdev} | cat | tr -d "\r" | tee
"${goutfile}" | do_trim
+timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial
file:/dev/stdout -${device}"${isofile}" ${bootdev} >
"$work_directory/qemu-pipe" || ret=$?
fi
if [ x$boot = xcoreboot ]; then
test -n "$debug" || rm -f "${imgfile}"
@@ -536,6 +539,7 @@ fi
test -n "$debug" || rm -f "${isofile}"
test -n "$debug" || rm -rf "${rom_directory}"
test -n "$debug" || rm -f "${tmpfile}" "${cfgfile}"
-exit 0
+rm -f "$work_directory/qemu-pipe"
+exit $ret
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 08/17] grub-shell: Allow setting default timeout via GRUB_SHELL_DEFAULT_TIMEOUT envvar
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 9d8c417da..e80471126 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -211,7 +211,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}"
in
console=console;;
esac
-timeout=60
+timeout=${GRUB_SHELL_DEFAULT_TIMEOUT:-60}
mkimage_extra_arg=
# Check the arguments.
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 07/13] efi: Format string error in grub_error
The second format string argument, GRUB_EFI_MAX_USABLE_ADDRESS, is a macro
to a number literal. However depending on what the target architecture, the
type can be 32 or 64 bits. Cast to a 64-bit integer. Also, change the
format string literals %llx to use PRIxGRUB_UINT64_T.
Signed-off-by: Glenn Washburn
---
grub-core/kern/efi/mm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
index 457772d57..9d3d279fd 100644
--- a/grub-core/kern/efi/mm.c
+++ b/grub-core/kern/efi/mm.c
@@ -127,8 +127,9 @@ grub_efi_allocate_pages_real (grub_efi_physical_address_t
address,
if (address > GRUB_EFI_MAX_USABLE_ADDRESS)
{
grub_error (GRUB_ERR_BAD_ARGUMENT,
- N_("invalid memory address (0x%llx > 0x%llx)"),
- address, GRUB_EFI_MAX_USABLE_ADDRESS);
+ N_("invalid memory address (0x%"PRIxGRUB_UINT64_T
+ " > 0x%"PRIxGRUB_UINT64_T")"),
+ address, (grub_efi_uint64_t) GRUB_EFI_MAX_USABLE_ADDRESS);
return NULL;
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 04/13] dmraid_nvidia: Format string error in grub_error
The grub_error has a format string expecting two arguments, but only one
provided. According to the comments in the struct grub_nv_super definition,
the version field looks like a version number where major.minor is encoded
as each a byte in the two-byte short.
Signed-off-by: Glenn Washburn
---
grub-core/disk/dmraid_nvidia.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/disk/dmraid_nvidia.c b/grub-core/disk/dmraid_nvidia.c
index 4d2fb04d1..6372663e6 100644
--- a/grub-core/disk/dmraid_nvidia.c
+++ b/grub-core/disk/dmraid_nvidia.c
@@ -122,7 +122,7 @@ grub_dmraid_nv_detect (grub_disk_t disk,
if (sb.version != NV_VERSION)
{
grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
- "unknown version: %d.%d", sb.version);
+ "unknown version: %d.%d", sb.version >> 8, sb.version & 0xFF);
return NULL;
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 00/13] error: Do compile-time format string checking on grub_error
This patch series fixes all compile errors due to format string issues on grub_error. This was tested against nearly all supported platforms successfully. This is important because earlier versions of these changes compiled successfully on x86 platforms, but had issues on other ones not tested. All patches except the last fix actual format string issues. The last patch turns format string issues into errors. This is a good idea because it will help to prevent introducing new format string issues into the code. Since, I presume, Daniel does not do not do a build test for all architectures before committing to master, this will not ensure that no format string issues get introduced into the code. However, it should flush out any format string issues when the CI build is done. Many of these changes are fairly obvious. I tried to use the PRI*GRUB_*_T macros as much as I could and to not cast arguments. There are some notable exceptions. There is some code that uses the same grub_error code in both 32 and 64 bit architectures (riscv), so casting was needed to force the larger storage type. The second to last patch for zfs is still confounding to me as to why the macro PRIuGRUB_UINT64_T was not expanding to llu, and yet the compiler was saying the argument was a long long unsigned. The tests results can be found here: https://gitlab.com/gwashburn/grub/-/pipelines/255133408 Glenn Glenn Washburn (13): misc: Format string for grub_error should be a literal error: grub_error missing format string argument error: grub_error format string add missing format code dmraid_nvidia: Format string error in grub_error grub_error: Use format code PRIuGRUB_SIZE for variables of type grub_size_t pgp: Format code for grub_error is incorrect efi: Format string error in grub_error error: Use %p format code for pointer types error: Use format code PRIxGRUB_UINT64_T for 64-bit uint argument in grub_error error: Use format code PRIxGRUB_UINT64_T for 64-bit arg in grub_error error: Use format code PRIuGRUB_UINT64_T for 64-bit typed fileblock in grub_error error: Use format code llu for 64-bit uint bp->blk_prop in grub_error error: Do compile-time format string checking on grub_error grub-core/commands/pgp.c | 2 +- grub-core/disk/ata.c | 4 ++-- grub-core/disk/cryptodisk.c| 12 grub-core/disk/dmraid_nvidia.c | 2 +- grub-core/efiemu/i386/loadcore64.c | 3 ++- grub-core/fs/hfsplus.c | 3 ++- grub-core/fs/zfs/zfs.c | 4 ++-- grub-core/kern/arm64/dl.c | 3 ++- grub-core/kern/efi/efi.c | 2 +- grub-core/kern/efi/mm.c| 5 +++-- grub-core/kern/ia64/dl.c | 3 ++- grub-core/kern/riscv/dl.c | 5 +++-- grub-core/kern/sparc64/dl.c| 3 ++- grub-core/kern/x86_64/dl.c | 3 ++- grub-core/loader/efi/chainloader.c | 4 ++-- grub-core/loader/i386/bsd.c| 3 ++- grub-core/loader/i386/pc/linux.c | 4 ++-- grub-core/net/tftp.c | 2 +- grub-core/parttool/msdospart.c | 4 ++-- grub-core/script/lexer.c | 2 +- grub-core/video/bochs.c| 4 ++-- include/grub/err.h | 3 ++- 22 files changed, 47 insertions(+), 33 deletions(-) -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 06/13] pgp: Format code for grub_error is incorrect
The format code is for a 32-bit int, but the argument, keyid, is declared as
a 64 bit int. The comment above says keyid is 32-bit. I'm not sure if the
comment or declaration is wrong, so force the display of a 64-bit int for
now.
Signed-off-by: Glenn Washburn
---
grub-core/commands/pgp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
index bbf6871fe..150cb6fe9 100644
--- a/grub-core/commands/pgp.c
+++ b/grub-core/commands/pgp.c
@@ -633,7 +633,7 @@ grub_verify_signature_real (struct grub_pubkey_context
*ctxt,
if (!sk)
{
/* TRANSLATORS: %08x is 32-bit key id. */
- grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key %08x not found"),
+ grub_error (GRUB_ERR_BAD_SIGNATURE, N_("public key
%08"PRIxGRUB_UINT64_T" not found"),
keyid);
goto fail;
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 11/13] error: Use format code PRIuGRUB_UINT64_T for 64-bit typed fileblock in grub_error
Signed-off-by: Glenn Washburn
---
grub-core/fs/hfsplus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 9c4e4c88c..b538f52e0 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -188,7 +188,8 @@ grub_hfsplus_read_block (grub_fshelp_node_t node,
grub_disk_addr_t fileblock)
|| !nnode)
{
grub_error (GRUB_ERR_READ_ERROR,
- "no block found for the file id 0x%x and the block offset
0x%x",
+ "no block found for the file id 0x%x and the block"
+ " offset 0x%"PRIuGRUB_UINT64_T,
node->fileid, fileblock);
break;
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 12/13] error: Use format code llu for 64-bit uint bp->blk_prop in grub_error
For some reason PRIuGRUB_UINT64_T is not expanding to llu, but to lu, which
causes the format string check to fail. Use literal and force cast until
this is debugged.
Signed-off-by: Glenn Washburn
---
grub-core/fs/zfs/zfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index b6e1e178d..a3691d220 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -1869,8 +1869,8 @@ zio_read (blkptr_t *bp, grub_zfs_endian_t endian, void
**buf,
{
if (BPE_GET_ETYPE(bp) != BP_EMBEDDED_TYPE_DATA)
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
- "unsupported embedded BP (type=%u)\n",
- BPE_GET_ETYPE(bp));
+ "unsupported embedded BP (type=%llu)\n",
+ (long long unsigned int)BPE_GET_ETYPE(bp));
lsize = BPE_GET_LSIZE(bp);
psize = BF64_GET_SB(grub_zfs_to_cpu64 ((bp)->blk_prop, endian), 25, 7,
0, 1);
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 13/13] error: Do compile-time format string checking on grub_error
This should help prevent format string errorsand thus improve the quality of error reporting. Signed-off-by: Glenn Washburn --- include/grub/err.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/grub/err.h b/include/grub/err.h index 24ba9f5f5..b08d5d0de 100644 --- a/include/grub/err.h +++ b/include/grub/err.h @@ -85,7 +85,8 @@ struct grub_error_saved extern grub_err_t EXPORT_VAR(grub_errno); extern char EXPORT_VAR(grub_errmsg)[GRUB_MAX_ERRMSG]; -grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...); +grub_err_t EXPORT_FUNC(grub_error) (grub_err_t n, const char *fmt, ...) +__attribute__ ((format (GNU_PRINTF, 2, 3))); void EXPORT_FUNC(grub_fatal) (const char *fmt, ...) __attribute__ ((noreturn)); void EXPORT_FUNC(grub_error_push) (void); int EXPORT_FUNC(grub_error_pop) (void); -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 05/13] grub_error: Use format code PRIuGRUB_SIZE for variables of type grub_size_t
Signed-off-by: Glenn Washburn
---
grub-core/disk/cryptodisk.c| 12
grub-core/kern/efi/efi.c | 2 +-
grub-core/loader/efi/chainloader.c | 2 +-
grub-core/loader/i386/bsd.c| 3 ++-
4 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index b62835acc..4ff6654a0 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -484,13 +484,15 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const
char *ciphername, cons
}
if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
{
- ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size:
%d",
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+ "Unsupported XTS block size: %"PRIuGRUB_SIZE,
cipher->cipher->blocksize);
goto err;
}
if (secondary_cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
{
- ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported XTS block size:
%d",
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+ "Unsupported XTS block size: %"PRIuGRUB_SIZE,
secondary_cipher->cipher->blocksize);
goto err;
}
@@ -501,7 +503,8 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const
char *ciphername, cons
cipheriv = ciphermode + sizeof ("lrw-") - 1;
if (cipher->cipher->blocksize != GRUB_CRYPTODISK_GF_BYTES)
{
- ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported LRW block size:
%d",
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT,
+ "Unsupported LRW block size: %"PRIuGRUB_SIZE,
cipher->cipher->blocksize);
goto err;
}
@@ -523,7 +526,8 @@ grub_cryptodisk_setcipher (grub_cryptodisk_t crypt, const
char *ciphername, cons
{
if (cipher->cipher->blocksize & (cipher->cipher->blocksize - 1)
|| cipher->cipher->blocksize == 0)
- grub_error (GRUB_ERR_BAD_ARGUMENT, "Unsupported benbi blocksize: %d",
+ grub_error (GRUB_ERR_BAD_ARGUMENT,
+ "Unsupported benbi blocksize: %"PRIuGRUB_SIZE,
cipher->cipher->blocksize);
/* FIXME should we return an error here? */
for (benbi_log = 0;
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 2942b8e35..d45cf67a9 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -503,7 +503,7 @@ grub_efi_duplicate_device_path (const
grub_efi_device_path_t *dp)
if (len < 4)
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
- "malformed EFI Device Path node has length=%d", len);
+ "malformed EFI Device Path node has
length=%"PRIuGRUB_SIZE, len);
return NULL;
}
diff --git a/grub-core/loader/efi/chainloader.c
b/grub-core/loader/efi/chainloader.c
index 2e98947a0..b8d35f41e 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -164,7 +164,7 @@ make_file_path (grub_efi_device_path_t *dp, const char
*filename)
if (len < 4)
{
grub_error (GRUB_ERR_OUT_OF_RANGE,
- "malformed EFI Device Path node has length=%d", len);
+ "malformed EFI Device Path node has
length=%"PRIuGRUB_SIZE, len);
return NULL;
}
diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index f5bf7f89e..fe20b339b 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -2110,7 +2110,8 @@ grub_cmd_openbsd_ramdisk (grub_command_t cmd
__attribute__ ((unused)),
{
grub_file_close (file);
return grub_error (GRUB_ERR_BAD_OS, "your kOpenBSD supports ramdisk only"
-" up to %u bytes, however you supplied a %u bytes one",
+" up to %"PRIuGRUB_SIZE" bytes, however you supplied"
+" a %"PRIuGRUB_SIZE" bytes one",
openbsd_ramdisk.max_size, size);
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 08/13] error: Use %p format code for pointer types
Signed-off-by: Glenn Washburn
---
grub-core/loader/i386/pc/linux.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/grub-core/loader/i386/pc/linux.c b/grub-core/loader/i386/pc/linux.c
index 814988ab9..0bc5d6807 100644
--- a/grub-core/loader/i386/pc/linux.c
+++ b/grub-core/loader/i386/pc/linux.c
@@ -230,9 +230,9 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
&& GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size
> grub_linux_real_target)
{
- grub_error (GRUB_ERR_BAD_OS, "too big zImage (0x%x > 0x%x), use bzImage
instead",
+ grub_error (GRUB_ERR_BAD_OS, "too big zImage (%p > %p), use bzImage
instead",
(char *) GRUB_LINUX_ZIMAGE_ADDR + grub_linux16_prot_size,
- (grub_size_t) grub_linux_real_target);
+ (void *) grub_linux_real_target);
goto fail;
}
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 01/17] tests: Make sure LANG is set properly for iso9660_test
LANG must be set to something that supports international characters,
otherwise xorriso will refuse to include the file with name having
international characters, causing the test to fail. So if LANG is not set,
set it to en_US.UTF-8, a very common UTF-8 locale. And if it is set, but
does not look like a UTF-8 locale, print a warning so the user will have a
clue as to why the iso9660_test might be failing.
Signed-off-by: Glenn Washburn
---
tests/iso9660_test.in | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/iso9660_test.in b/tests/iso9660_test.in
index 571b938d7..37b1d30af 100644
--- a/tests/iso9660_test.in
+++ b/tests/iso9660_test.in
@@ -7,6 +7,12 @@ if ! which xorriso >/dev/null 2>&1; then
exit 77
fi
+if [ -z "$LANG" ]; then
+ export LANG=en_US.UTF-8
+elif [ -n "${LANG##*UTF*}" ]; then
+ echo "WARNING: LANG=$LANG appears to not be unicode, international file
test may fail."
+fi
+
"@builddir@/grub-fs-tester" joliet
"@builddir@/grub-fs-tester" rockridge
"@builddir@/grub-fs-tester" rockridge_joliet
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 00/17] Gitlab CI and test framework improvements
This patch series is divided into roughly three sections: modifications of tests, improvements to grub-shell, and the CI patch itself. I have other improvements to testing coming, but I consider these more essential to the quality of the CI tests, or highly desirable. Aside from modification of qemu firmware paths to make them more generic, the changes should not be visible to current testers. The CI change is the addition of a .gitlab-ci.yml file which is the GitLab specific CI config file and a .ci directory with scripts used by the CI config. The scripts in the .ci dir are meant to be generic and thus usable by other CI platforms. There is probably more that can be pulled out of the .gitlab-ci.yml file for use in common with other CI configs, but I decided those can be pulled out as needed. The successful CI run can be viewed here: https://gitlab.com/gnu-grub/grub/-/pipelines/258485608 Glenn Glenn Washburn (17): tests: Make sure LANG is set properly for iso9660_test tests: Fix partmap_test for arm*-efi, disk numbering has changed tests: When checking squashfs fstime, use superblock last modified time tests: Fail immediately when grub-shell fails and do not occlude the error code grub-shell: Allow specifying non-default trim line contents grub-shell: Trim line should always be matched from the beginning of the line grub-shell: Only show grub-mkrescue output if it returns an error grub-shell: Allow setting default timeout via GRUB_SHELL_DEFAULT_TIMEOUT envvar grub-shell: Put all generated files into working dir and use better file names grub-shell: Add grub output logfile with grub-shell --debug grub-shell: Set exit status to qemu exit status tests: Allow turning on shell tracing from environment variables grub-shell: Add --verbose to mkrescue when $debug is greater than 2 grub-shell: Use malta qemu-mips machine type instead off non-existant indy grub-shell: Update qemu UEFI firmware names to be more generic ci: Add .gitlab-ci.yml for continuous integration with Gitlab CI keep: remove a lot of cruft that we probably want to reuse at some point .ci/build.sh | 56 +++ .ci/functions.gitlab.sh| 30 ++ .ci/functions.sh | 30 ++ .ci/make-images.sh | 71 .ci/process-tests.sh | 97 + .ci/test.sh| 98 + .gitlab-ci.yml | 632 + tests/ahci_test.in | 6 +- tests/cdboot_test.in | 3 +- tests/core_compress_test.in| 6 +- tests/ehci_test.in | 6 +- tests/fddboot_test.in | 3 +- tests/grub_cmd_date.in | 3 +- tests/grub_cmd_test.in | 1 + tests/grub_script_blockarg.in | 2 +- tests/grub_script_expansion.in | 3 +- tests/gzcompress_test.in | 3 +- tests/hddboot_test.in | 3 +- tests/iso9660_test.in | 6 + tests/lzocompress_test.in | 3 +- tests/netboot_test.in | 3 +- tests/ohci_test.in | 6 +- tests/partmap_test.in | 6 +- tests/pata_test.in | 3 +- tests/test_sha512sum.in| 1 + tests/uhci_test.in | 6 +- tests/util/grub-fs-tester.in | 8 + tests/util/grub-shell.in | 87 +++-- tests/xzcompress_test.in | 3 +- 29 files changed, 1138 insertions(+), 47 deletions(-) create mode 100755 .ci/build.sh create mode 100644 .ci/functions.gitlab.sh create mode 100644 .ci/functions.sh create mode 100755 .ci/make-images.sh create mode 100755 .ci/process-tests.sh create mode 100755 .ci/test.sh create mode 100644 .gitlab-ci.yml -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[CI 04/17] tests: Fail immediately when grub-shell fails and do not occlude the error code
If grub-shell fails, that means that whatever was being tested was not
actually tested. So fail immediately. Sometimes grub-shell is not the last
command in a pipeline of several commands, and in this case the failed error
code can be hidden by a later failing command or hidden when 'set -e' is not
set and there are subsequent successful commands. When the test script fails
because of a failure in grub-shell, then test script should exit with the
failed exit code of grub-shell.
Signed-off-by: Glenn Washburn
---
tests/ahci_test.in | 6 +-
tests/cdboot_test.in | 3 ++-
tests/core_compress_test.in| 6 --
tests/ehci_test.in | 6 +-
tests/fddboot_test.in | 3 ++-
tests/grub_cmd_date.in | 3 ++-
tests/grub_cmd_test.in | 1 +
tests/grub_script_blockarg.in | 2 +-
tests/grub_script_expansion.in | 3 ++-
tests/gzcompress_test.in | 3 ++-
tests/hddboot_test.in | 3 ++-
tests/lzocompress_test.in | 3 ++-
tests/netboot_test.in | 3 ++-
tests/ohci_test.in | 6 +-
tests/partmap_test.in | 4 ++--
tests/pata_test.in | 3 ++-
tests/test_sha512sum.in| 1 +
tests/uhci_test.in | 6 +-
tests/xzcompress_test.in | 3 ++-
19 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/tests/ahci_test.in b/tests/ahci_test.in
index 7df560462..3fb7f54a2 100644
--- a/tests/ahci_test.in
+++ b/tests/ahci_test.in
@@ -41,7 +41,11 @@ echo "hello" > "$outfile"
tar cf "$imgfile" "$outfile"
-if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}"
--qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci -device
ide-drive,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(ahci0)/$outfile';" |
+"${grubshell}" --qemu-opts="-drive id=disk,file=$imgfile,if=none
+ -device ahci,id=ahci
+ -device ide-drive,drive=disk,bus=ahci.0"
>$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
rm "$imgfile"
rm "$outfile"
exit 1
diff --git a/tests/cdboot_test.in b/tests/cdboot_test.in
index 75acdfedb..7229f79fb 100644
--- a/tests/cdboot_test.in
+++ b/tests/cdboot_test.in
@@ -34,6 +34,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-if [ "$(echo hello | "${grubshell}" --boot=cd)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --boot=cd`
+if [ "$v" != "Hello World" ]; then
exit 1
fi
diff --git a/tests/core_compress_test.in b/tests/core_compress_test.in
index 9d216ebcf..90dd00607 100644
--- a/tests/core_compress_test.in
+++ b/tests/core_compress_test.in
@@ -27,10 +27,12 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}"
in
esac
-if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz)" !=
"Hello World" ]; then
+v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz`
+if [ "$v" != "Hello World" ]; then
exit 1
fi
-if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=none)" !=
"Hello World" ]; then
+v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=none`
+if [ "$v" != "Hello World" ]; then
exit 1
fi
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index b197f8cdc..64bd80070 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -41,7 +41,11 @@ echo "hello" > "$outfile"
tar cf "$imgfile" "$outfile"
-if [ "$(echo "nativedisk; source '(usb0)/$outfile';" | "${grubshell}"
--qemu-opts="-device ich9-usb-ehci1 -drive id=my_usb_disk,file=$imgfile,if=none
-device usb-storage,drive=my_usb_disk" | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(usb0)/$outfile';" |
+"${grubshell}" --qemu-opts="-device ich9-usb-ehci1
+ -drive id=my_usb_disk,file=$imgfile,if=none
+ -device usb-storage,drive=my_usb_disk" >$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
rm "$imgfile"
rm "$outfile"
exit 1
diff --git a/tests/fddboot_test.in b/tests/fddboot_test.in
index 2d7dfc889..1bbe60ee5 100644
--- a/tests/fddboot_test.in
+++ b/tests/fddboot_test.in
@@ -46,6 +46,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-if [ "$(echo hello | "${grubshell}" --boot=fd --mkrescue-arg="
[CI 06/17] grub-shell: Trim line should always be matched from the beginning of the line
When turning on shell tracing the trim line will be output before we
actually want to start the trim. However, in this case the trim line never
starts from the beginning of the line. So start trimming from the correct
line by matching from the beginning of the line.
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index b409962f1..602b16f3e 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -343,7 +343,7 @@ terminal_output ${term}
EOF
if [ $trim = 1 ]; then
-echo "echo $trim_head" >>${cfgfile}
+echo "echo; echo $trim_head" >>${cfgfile}
fi
rom_directory=`mktemp -d "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
@@ -457,7 +457,7 @@ fi
do_trim ()
{
if [ $trim = 1 ] || [ $trim = 2 ]; then
- awk '{ if (have_head == 1) print $0; } /'"$trim_head"'/ { have_head=1;
}'
+ awk '{ if (have_head == 1) print $0; } /^'"$trim_head"'/ { have_head=1;
}'
else
cat
fi
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: RFC: Grub project management
On Sun, 14 Feb 2021 13:58:40 +1100 Daniel Axtens wrote: > > Reading more about patchwork, it seems to have its own set of > > issues, partly revolving around using a mailing list of development > > as we do. see: https://lwn.net/Articles/773456/ > > I'm a patchwork maintainer, happy to discuss how Patchwork might be > helpful. It certainly isn't perfect (and alternatives like patchew and > the freedesktop.org fork of patchwork should be seriously considered) > but it certainly could be part of a solution. That's great. I'm thinking it would be better to use somebody else's infrastructure instead of maintaining our own, like maintaining our own patchwork server. I'm a little surprised that there's not a centralized multiproject patchwork server with lots of projects that use MLs for sending patches. Or have I just not seen it? Although, having a patchwork maintainer in the community might make maintaining our own infrastructure less costly. > It doesn't directly host CI, but provides some API access that can be > used to drive CI. The snowpatch project provides a link to Jenkins, > although Jenkins comes with its own set of problems. > > A nice thing about all of upstream patchwork/FDO patchwork/patchew is > that they can all be bolted on to the existing ML and be used as much > or as little as desired. That's great, but we're still maintaining the infrastructure. But maybe we've got people in our community that are willing to contribute those resources. > Another option, and something I have been considering but so far have > lacked the time to do, is to set up something like one of the various > bots that lurks on the linux kernel mailing lists and builds and tests > patches without a web interface - just posting the results back to the > ML as a reply. This could be done without any buy-in from maintainers. I think having automated building/testing of patches would be great. And the bot could reply to the first email in the patch thread with success/failure and some links to logs or other artifacts. I don't really like the custom solution aspect cause that's more overall maintenance. This is why I like the potential of the sourcehut suggestion. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/7] Add xHCI USB support
Hi Patrick, Thanks for the contribution. I think this would be a great addition to GRUB. However, there are a few issues I see at the moment. On Mon, 7 Dec 2020 08:41:20 +0100 Patrick Rudolph wrote: > Add basic support for xHCI USB controllers and non root xHCI hubs. > The motivation is to use this code on platforms that do not provide > user input by runtime services (like BIOS or UEFI platform) do. > This is the case when GRUB is used as coreboot payload for example. > > The code is based on seabios implementation, but has been heavily > modified to match grubs internals. Are these the same changes as in https://github.com/Nitrokey/grub.git referenced in a previous email? I'm asking because there were some comments that were unaddressed. So, if it is, could you please address Thomas's email (I haven't checked to see if they are still relevant if so)? (see: https://marc.info/?l=grub-devel=159769164332293) Searching in the mailing list, I found this attempt in early 2017. Having not looked at the code at all, could this help in addressing some of Thomas's concerns? https://github.com/bjornfor/grub/tree/add-coreboot-xhci-driver-2nd-attempt-v2 I've run your code through the GitLab CI I've configured, and there was a build failure on x86_64-efi. Looks like some implicit type casting issues. I believe it failed for both gcc 8.1 and 10.1. Here is the error log when using gcc 10.1: https://gitlab.com/gwashburn/grub/-/jobs/1025317154#L594 Also, I think that any xhci support should be accompanied by passing make check tests. See uhci_test for an example of how this might be done. Since you've already tested on qemu, I think you're 90% of the way there in making some tests. Both of the ones listed before would be nice. If you'd like some guidance or help, let me know. Glenn > Changes done in version 2: > * Code cleanup > * Code style fixes > * Don't leak memory buffers > * Compile without warnings > * Add more defines > * Add more helper functions > * Don't assume a 1:1 virtual to physical mapping > * Flush cachelines after writing buffers > * Don't use hardcoded page size > * Proper scratchpad register setup > * xHCI bios ownership handoff > > Changes done in version 3: > * Several bug fixes for real hardware > * Fixed a race condition detecting events, which doesn't appear on >qemu based xHCI controllers > * Don't accidently disable USB3.0 devices after first command > * Support arbitrary protocol speed IDs > * Coding style cleanup > * Support for non root SuperSpeed hubs > > Changes Tested: > * Qemu system x86_64 >* virtual USB HID keyboard (usb-kbd) >* virtual USB HID mass storage (usb-storage) > * Intel C246 integrated xHCI (Supermicro X11SSH-F) >* iKVM HID keyboard >* USB3 HID mass storage (controller root port) >* External USB HID keyboard > > TODO: > * Test on more hardware > * Test on USB3 hubs > * Support for USB 3.1 and USB 3.2 controllers > > Patrick Rudolph (7): > grub-core/bus/usb: Parse SuperSpeed companion descriptors > usb: Add enum for xHCI > usbtrans: Set default maximum packet size > grub-core/bus/usb: Add function pointer for attach/detach events > grub-core/bus/usb/usbhub: Add new private fields for xHCI controller > grub-core/bus/usb: Add xhci support > grub-core/bus/usb/usbhub: Add xHCI non root hub support > > Makefile.am |2 +- > grub-core/Makefile.core.def |7 + > grub-core/bus/usb/ehci.c |2 + > grub-core/bus/usb/ohci.c |2 + > grub-core/bus/usb/serial/common.c |2 +- > grub-core/bus/usb/uhci.c |2 + > grub-core/bus/usb/usb.c | 44 +- > grub-core/bus/usb/usbhub.c| 95 +- > grub-core/bus/usb/usbtrans.c |6 +- > grub-core/bus/usb/xhci-pci.c | 195 +++ > grub-core/bus/usb/xhci.c | 2496 > + grub-core/commands/usbtest.c | > 2 +- grub-core/disk/usbms.c|2 +- > grub-core/term/usb_keyboard.c |2 +- > include/grub/usb.h| 18 +- > include/grub/usbdesc.h| 12 +- > include/grub/usbtrans.h |4 + > 17 files changed, 2852 insertions(+), 41 deletions(-) > create mode 100644 grub-core/bus/usb/xhci-pci.c > create mode 100644 grub-core/bus/usb/xhci.c > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
RFC: Grub project management
Hi fellow GRUB developers, I want to start by recognizing that most people who are involved with the project maintenance are doing so on a voluntary basis (or at least that's my assumption) and people have busy lives. Its a mostly thankless job, so thank you guys for all the time spent keeping this essential project alive. I hope that the following will not be construed as judging anyone, but merely taking account of where we've been, where we are, and how to move forward. TLDR; Check out https://gitlab.com/gnu-grub/grub and do merge requests with changes rebased on to feature/gitlab-testing against feature/gitlab-testing to get automatic CI testing. I believe I speak for more than just myself when I say that the current development process leaves much to be desired. GRUB has been in a feature freeze since last March, with the release being pushed rescheduled several times and now looks like its projected to be in March (assuming its not pushed back again). I get the impression that Daniel is overloaded with non-GRUB responsibilities. I also wonder if part of the issue is concerned about making a release which he believes hasn't had sufficient testing. Daniel, could you please clarify what needs to be done to get the release out the door and what the community can do to help expedite this process? In the meantime, patches are being sent to the list which will not be considered for acceptance until the release is out and feature freeze lifted. Does anyone believe that someone is going to go back to last March and check all the submitted patches to see about including them? I judge the answer to be no. So patches will be continually falling into a blackhole. What we need is an issue/merge management system so things don't get lost, or can easily be found. GRUB currently uses the Savannah GNU project hosting site. And while I love GNU philosophy and the role that Savannah has played for free software projects, it is showing its age and not ideal for a modern development process. My two big pain points is that it does not do merge requests and the bug tracker is an unpleasant experience when compared to modern ones. If you've ever tried to search for a bug, you'll know what I mean. So unfortunately, I think its time to say goodbye Savannah. Another issue, as I see it, is the testing. The only testing that I'm aware of is the travis CI, which is only a build test, and Adrian's testing for debian, which does do the make check tests but is missing filesystem testing and qemu tests for most architectures. Ideally, the make check tests should be done within travis, but we're not there yet. This process feels closed and non-transparent to me, which I find undesirable in general, but especially so for a free software project which is the corner-stone of the linux community. And I'm not saying that that is intentional on the part of those working on those tests. So, for one, it would be nice to have publicly accessible the status of Travis runs. And may be it is, but why isn't it obvious? Based on what I've gathered from the list and private conversations (which isn't much), the travis is only really run as a prep for making a release. If that's true, the process fails to use the main point of a continuous integration system. I would like GRUB to get to a place where every commit is run through CI (at least the build test). In the short term, it would be nice to kick off a CI job every time master changes. It would be even more nice if we could integrate the merge tracking system with the CI system so that merge requests automatically kick off a CI job to see if the proposed changes pass both the build test and functional make check tests. I believe, and my sincere hope, is that by resolving these issues we can speed up the development cycle. I have a lot of changes that I've been waiting months to submit to the list. I don't think it should take a mountain of patience to get changes accepted. The GRUB project and its users are worse off for it. It is in consideration of all the above that I've setup a system on GitLab that addresses all these concerns and more. I also believe that GitLab was founded as a more ethical company that would be more compatible with a GNU software project than most other project hosting sites. I chose to use GitLab's builtin CI because it has what appears to be unlimited free resources (although resource constrained to an extent) for open source accounts. I've setup an unofficial GNU GRUB group and project repository at https://gitlab.com/gnu-grub/grub. It is currently configured to mirror the official repository. I have created a few merge requests that are real examples to show what the process would look like. The most important merge request is the one adding the GitLab CI feature (https://gitlab.com/gnu-grub/grub/-/merge_requests/2). This is a merge request created from a branch in the repo, as opposed to one from a fork. Because it has the .gitlab-ci.yml file configured
Re: [PATCH 00/12] Grub-shell improvements
Hi Daniel, On Thu, 26 Aug 2021 20:00:32 +0200 Daniel Kiper wrote: > Hi Glenn, > > On Wed, Aug 25, 2021 at 06:06:30PM -0500, Glenn Washburn wrote: > > Hi Daniel, > > > > What are the chances this patch series can be reviewed in the near > > future? Some feedback would be greatly appreciated. > > I can see the following patches from you waiting in my review queue: > - [CRYPTO-LUKS v1 00/19] Fixes and improvements for > cryptodisks+luks2 and a few other things. > https://lists.gnu.org/archive/html/grub-devel/2020-07/msg00088.html > - [CRYPTOMOUNT-TEST 0/7] Add LUKS1/2 tests for cryptomount > https://lists.gnu.org/archive/html/grub-devel/2020-08/msg00010.html > - [PATCH 0/5] Testing improvements > https://lists.gnu.org/archive/html/grub-devel/2020-12/msg00246.html > - [CI 00/17] Gitlab CI and test framework improvements > https://lists.gnu.org/archive/html/grub-devel/2021-02/msg00071.html > - [PROCFS 0/5] Add and improve (proc) entries > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00264.html > - [PATCH 0/4] Various LUKS2 improvements > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00272.html > - [PATCH 0/4] Miscellaneous changes to aid in troubleshooting > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00279.html > - [PATCH] fs: Allow number of blocks in block list to be optional, > defaulting length to device length > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00286.html > - [PATCH v2 0/2] cryptodisk: Allows UUIDs to be compared in a > dash-insensitive manner > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00344.html > - [PATCH] command: Add silent mode to read command to suppress > input echo > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00291.html > - [PATCH 0/2] Allow overriding commands > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00292.html > - [PATCH 00/12] Grub-shell improvements > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00390.html > - [PATCH v2 0/8] Various fixes/improvements for tests > https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00110.html > - [PATCH 0/3] Refactor/improve cryptomount data passing to crypto > modules > https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00129.html > > Please sort them in the order of importance/preference/... Then I > will be looking at them (more or less) in that order, one patch set > at a time. > > I hope I did not miss any of your patches. As far as I can tell, this is the full list. My order preference is as follows: These two patches are only first because it should be a quick review. - [PATCH] command: Add silent mode to read command to suppress input echo https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00291.html - [PATCH] fs: Allow number of blocks in block list to be optional, defaulting length to device length https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00286.html This is a high priority because I think it should be merged before the keyfile and detached header support patch series. If this is merged, I'll submit and updated keyfile and detached header patch series that works with this patch series. - [PATCH 0/3] Refactor/improve cryptomount data passing to crypto modules https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00129.html Based on discussion with Thomas Schmitt, I think the first patch of this series should be dropped. Should I make a v3, or would you review it and if acceptable drop the first patch? - [PATCH v2 0/8] Various fixes/improvements for tests https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00110.html Then these in this order. - [PATCH 00/12] Grub-shell improvements https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00390.html - [PATCH 0/4] Various LUKS2 improvements https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00272.html - [PROCFS 0/5] Add and improve (proc) entries https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00264.html - [PATCH v2 0/2] cryptodisk: Allows UUIDs to be compared in a dash-insensitive manner https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00344.html - [PATCH 0/4] Miscellaneous changes to aid in troubleshooting https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00279.html - [PATCH 0/2] Allow overriding commands https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00292.html Patch series with dependencies This patch series requires keyfile support via the -k argument for cryptomount. So it must be reviewed after a patch series implementing this. Also, iirc, this series relies on "Grub-shell improvements". Ideally this would get reviewe
Re: [grub-fs-tester.in] zisofs test looks unsuitable
On Fri, 27 Aug 2021 00:14:20 +0200 "Thomas Schmitt" wrote: > Hi. > > Glenn Washburn wrote: > > I think the changes to get the test > > working are worthy of inclusion so that the tests are ready when > > this feature gets implemented. > > Do you think my draft of a commit message is ok ? Seems good to me. > Probably i should mention that this test will fail until zisofs is > implemented. Yes, but as I noted before, the ziso9660 is not actually run by any "make check" tests, which makes sense if its not actually implemented. Also the ziso9660 fs test fails currently anyway, so I don't see a problem here. > > I'm not familiar with that code nor the > > format details, but it sounds like you might be able to determine if > > its something somewhat easy to add to GRUB. > > Well, i wrote an implementation as libisofs filter (which differs much > from a filesystem driver) and a documentation of the format: > https://sources.debian.org/src/libisofs/1.5.2-1/doc/zisofs_format.txt > (Since noon our project web server is down. I need to pester the web > master to check which useless piece of software crashed it this time.) > > Implementing SUSP entry "ZF" would be a substantial change to > grub-core/fs/iso9660.c including a new dependency on zlib. There wouldn't be a new dependency on zlib. Grub already supports zlib decompression using grub_zlib_decompress in grub-core/io/gzio.c. I suspect this would work fine. > The adventure would probably begin at function susp_iterate_dir() > which interprets some Rock Ridge SUSP entries. > Then i'd need to know which functions tell the file size and do the > reading of file content data and how to make the info from the ZF > entry known to them. Functions for dealing with files can be found in include/grub/file.h and grub-core/kern/file.c > The Linux kernel has an implementation from the inventor, H.Peter > Anvin. (With a bug for machines with PAGE_SIZE > 32 KB > > https://lore.kernel.org/linux-scsi/20201120140633.1673-1-scdbac...@gmx.net/T/#u > and a dire need for a readahead implementation.) > > And there is the successor format zisofs2, not yet in the kernel > > https://fossies.org/linux/misc/xorriso-1.5.4.pl02.tar.gz/xorriso-1.5.4/doc/zisofs2_format.txt?m=t > (Thanks to all the sites which store what i write.) > Last november a zisofs2 patch of ~200 lines enabled the then actual > 5.10.0 release candidate to read the new format. But nearly a year of > bitrot already devalued my readahead patch. So my local test kernel > is the only one in the world which can read giant zisofs2 compressed > files at high speed. I'm sure support for the new format would be welcome as well and it sounds like you're well-positioned to make that happen. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [grub-fs-tester.in] zisofs test looks unsuitable
On Thu, 26 Aug 2021 22:07:29 +0200
"Thomas Schmitt" wrote:
> Hi,
>
> i managed to get the ziso9660 test running with actual compression.
>
> ./grub-fs-tester ziso9660
>
> now produces an ISO image but seems not to be happy with it.
>
> If i get it right then it sees the compressed size 1124519 of file
> "1.img" but expects its uncompressed size 5242879.
> Linux mount sees
> -rw-r--r-- 2 thomas thomas 5242879 Aug 26 21:52 1.img
>
> So grub-fstest.c CMD_LS seems not yet ready for dealing with zisofs.
This confirms my suspicions. I think the changes to get the test
working are worthy of inclusion so that the tests are ready when this
feature gets implemented. I'm not familiar with that code nor the
format details, but it sounds like you might be able to determine if
its something somewhat easy to add to GRUB.
> This is the change which brought me that far, including a draft for a
> commit message:
>
>
> The grub-fs-tester test "ziso9660" failed early with a xorriso error
> because it was not in the list of tests which tolerate at most 32
> bytes of FSLABEL.
> The xorriso run of ziso9660 did not actually compress the data files
> because the xorriso command for installing zisofs filters was applied
> before any files had populated the emerging ISO image.
> So add "ziso9660" where other ISO 9660 tests get a shorter FSLABEL and
> re-arrange the commands of the xorriso run to bring -add and -zisofs
> before -set_filter_r.
>
> Signed-off-by: Thomas Schmitt
>
> diff --git a/tests/util/grub-fs-tester.in
> b/tests/util/grub-fs-tester.in index bfc425e1f..834c05728 100644
> --- a/tests/util/grub-fs-tester.in
> +++ b/tests/util/grub-fs-tester.in
> @@ -314,7 +314,8 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
> "$MAXLOGSECSIZE" 1); do FSLABEL="grub_;/testéтi
> u劣蓮кириrewfceniuewruevrewnuuireurevueurnievrewfne";; # FS
> LIMITATION: afs and iso9660 label is at most 32 UTF-8 characters
> x"afs" | xiso9660 | xrockridge | xrockridge_joliet\
> - | xiso9660_1999 | xrockridge_1999 |
> xrockridge_joliet_1999)
> + | xiso9660_1999 | xrockridge_1999\
> + | xrockridge_joliet_1999 | xziso9660)
>FSLABEL="gr_;/é鍊劣кирит u";;
> # FS LIMITATION: bfs label is at most 32 UTF-8
> characters # OS LIMITATION: bfs label can't contain ; or /
> @@ -1020,7 +1021,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
> "$MAXLOGSECSIZE" 1); do (cd "$MASTER"; find . | cpio -o -H "$(echo
> ${fs} | sed 's@^cpio_@@')" > "${FSIMAGEP}0.img" ) ;; x"ziso9660")
> FSUUID=$(date -u +%Y-%m-%d-%H-%M-%S-00);
> - xorriso -compliance rec_mtime -set_filter_r
> --zisofs -- -zisofs default -as mkisofs -iso-level 3 -graft-points -R
> -J -joliet-long -V "$FSLABEL" --modification-date=$(echo ${FSUUID} |
> sed 's/-//g;') -o "${FSIMAGEP}0.img" -- -set_filter_r --zisofs --
> -zisofs default -add /="$MASTER" ;;
> + xorriso -compliance rec_mtime -as mkisofs
> -iso-level 3 -graft-points -R -J -joliet-long -V "$FSLABEL"
> --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o
> "${FSIMAGEP}0.img" -- -add /="$MASTER" -- -zisofs default
> -set_filter_r --zisofs / -- ;; x"iso9660") FSUUID=$(date -u
> +%Y-%m-%d-%H-%M-%S-00); xorriso --rockridge off -compliance rec_mtime
> -as mkisofs -iso-level 3 -graft-points -V "$FSLABEL"
> --modification-date=$(echo ${FSUUID} | sed 's/-//g;') -o
> "${FSIMAGEP}0.img" /="$MASTER" ;;
>
>
>
> Here is a run with this change:
>
>
> $ ./grub-fs-tester ziso9660
> GNU xorriso 1.5.4.pl02 : RockRidge filesystem manipulator, libburnia
> project.
>
> Drive current: -outdev
> 'stdio:/tmp/tmp.iIXC5zIfEd/ziso9660_512_512_1_0.img' Media current:
> stdio file, overwriteable Media status : is blank
> Media summary: 0 sessions, 0 data blocks, 0 data, 400g free
> xorriso : WARNING : -volid text problematic as automatic mount point
> name xorriso : WARNING : -volid text is too long for Joliet (31 > 16)
> xorriso : WARNING : -volid text does not comply to ISO 9660 / ECMA
> 119 rules Added to ISO image: directory
> '/'='/tmp/tmp.iIXC5zIfEd/master' xorriso : UPDATE : 227 files
> added in 1 seconds xorriso : UPDATE : 10 file filters processed
> in 1 seconds libisofs: WARNING : Cannot add /longsym to Joliet tree.
> Symlinks can only be added to a Rock Ridge tree. libisofs: WARNING :
> Cannot add /psym to Joliet tree. Symlinks can only be added to a Rock
> Ridge tree. libisofs: WARNING : Cannot add /sdir/ssym to Joliet tree.
> Symlinks can only be added to a Rock Ridge tree. libisofs: WARNING :
> Cannot add /sdir/usym to Joliet tree. Symlinks can only be added to a
> Rock Ridge tree. libisofs: WARNING : Cannot add /sym to Joliet tree.
> Symlinks can only be added to a Rock
Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test
On Thu, 26 Aug 2021 19:49:37 +0200
"Thomas Schmitt" wrote:
> Hi,
>
> now i know that i should have asked for the test-suite.log file.
>
> The problem turned out to be in Joliet, not in ISO 9660.
> Because Joliet gets its file names and other texts encoded as UCS-2
> 16-bit characters it is indeed inavoidable to define the meaning of
> the bytes in a Unix file name.
> The UTF-8 sequence \303\251 has no meaning as ANSI_X3.4-1968
> characters (which are what we learned to love as 7-bit ASCII). So
> iconv fails.
>
> My remedy proposal
>
> xorriso ... -as mkisofs -input-charset UTF-8 -output-charset UTF-8
> ...
Do there need to be any UTF-8 locales installed (or any locales for
that matter) for this to work? My guess is no.
> turned out to be a valid alternative to
>
> LANG=en_US.UTF-8
>
> Using above charset options unconditionally would make obsolete the
> warning in Glenn Washburn's patch at least for the xorriso runs:
>
> +elif [ -n "${LANG##*UTF*}" ]; then
> + echo "WARNING: LANG=$LANG appears to not be unicode,
> international file test may fail."
>
> It would still be beneficial to default LANG="" to LANG=en_US.UTF-8
> because 7-bit ASCII is really an odd character interpretation for Unix
> filenames. (HP-UX from 1986 maybe ...)
Correct me if I'm wrong, but this logic about defaulting LANG is more
general than the iso9660_test. Would you agree that this should be
applied to the tests a a whole?
If yes, then I should probably remove this patch altogether and put
this code elsewhere.
--
>
> Noob question:
> How can i force "make check" to do something at its second run ?
I might be answering this question below.
--
> Long story:
>
> On a Debian 10 with a freshly made GRUB git clone i see indeed a
> failure of iso9660_test with mutilated volume id and a mutilated
> exotic filename after
>
> make SUBDIRS= LANG= TESTS=iso9660_test check
>
> On the same machine with the same xorriso, the same file name from the
> log and the volume id taken from the script, i get with
>
> LANG= xorriso -as mkisofs -o test.iso -V "...exotic.characters..."
> test_tree
>
> a somewhat confused xterm (or libreadline) but
>
> xorriso -indev test.iso -find / --
>
> reports the exotic volume id and the exotic filename.
>
> The "make ... check" run says on the second try
> make[3]: 'iso9660_test' is up to date.
> but i can reproduce the problem by
>
> LANG= ./grub-fs-tester joliet
Yes the iso9660_test is up to date in the sense that it does not need
to be generated from iso9660_test.in. However, "make check" should
still be running the test. IOW, the tests are always rerun, there is no
make rule that does not run tests based on them having been perviously
run (that I know of).
> The trigger for the file name mutilation seems to be the mkisofs
> emulation option for creating a Joliet tree
> -J
> With default settings of mount(8) or xorriso's file name display the
> problem becomes visible only if no Rock Ridge info was added to the
> ISO 9660 tree. This is caused by xorriso command
> --rockridge off
>
> Nevertheless file(1) and xorriso still see the exotic volume id.
> Binary inspection shows that the ISO 9660 volume id (at offset 32768
> + 40) is originally exotic, but the Joliet volume id (at offset 34816
> + 40) was mutilated.
>
> So obviously the problem sits in the Joliet name conversion to UCS-2.
> It is inavoidable if the file names do not match the input character
> set (defaulted from locale) because the Unix file names cannot be
> just copied to Joliet directory records.
>
> Now that i can reproduce the problem i can also confirm that my remedy
> proposal would work as expected. It preservies the exotic names in the
> Joliet tree and the exotic volume id in the Joliet supplementary
> volume descriptor.
As i understand it, your proposed change is entirely with in
grub-fs-tester. Would you submit a patch with the changes you've
outlined that would fix this issue for when LANG is empty? Based on the
work you've done, I think it makes most sense for you to author the
patch. And I'll test/review it.
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test
On Thu, 26 Aug 2021 23:28:04 +0200
"Thomas Schmitt" wrote:
> > If yes, then I should probably remove this patch altogether and put
> > this code elsewhere.
>
> I think it can be justified to hardcode UTF-8 in grub-fs-tester
> regardless whether higher levels of the test empire or the user have
> own interests in locale settings.
> The code in grub-fs-tester is a good example for the situation that
> the file names on disk might belong to users with differing locales.
> We expect UTF-8 to be the common base of all those nowadays.
>
> So i'd vote for something like
>
> if [ -n "${LANG##*UTF*}" ]; then
> echo "NOTE: LANG=$LANG appears to not be UTF-8."
> echo "NOTE: Setting LANG=en_US.UTF-8 to match the test file
> names." export LANG=en_US.UTF-8
> fi
>
> at the start of grub-fs-tester.in.
Sounds good to me, please add this to the patch.
> > Would you submit a patch with the changes you've
> > outlined that would fix this issue for when LANG is empty?
>
> I'd go farer and smack down everything that does not claim to be
> UTF-8.
>
> Is a git diff enough or has it to be git format-patch from a local
> commit ? Is git send-email necessary ?
I'm not sure that it has to be git format-patch then git send-email,
but that's the desired method. The following commands may help you get
to speed quickly (since its only one patch things are less complicated).
git format-patch -s
Now you should have a patch file ready for sending. Setting up git
send-email isn't that hard, but I'd refer you to the man page since it
depends on your setup. Daniel might find it acceptable to send that
patch as an attachment since its just a single patch.
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test
On Fri, 27 Aug 2021 21:13:17 +0200 "Thomas Schmitt" wrote: > Hi, > > it turns out that it might be contraproductive to set > LANG=en_US.UTF-8. > > If the LANG locale is not listed by > locale -a > then nl_langinfo(3) returns "ANSI_X3.4-1968". > I stumbled over this when testing LANG=de_DE.UTF8, which despite of > my location is not an available locale on my machine. My patch only sets LANG if empty, so in this case I don't think its counterproductive, just not productive. > So i think that setting any locale by LANG makes daring assumptions > about its availability, unless brain is added to inspect the installed > locals and to choose one that is based on UTF-8. > This reaches quite far beyond the problem which the patch shall solve. > > What route to go ? > > - Choose the first UTF-8 locale offered by locale -a ? > (What to do if none is offered ?) > > - Abandon the idea of setting LANG and rather use xorrisofs charset > options to enforce UTF-8 as origin of the conversion ? > > I will for now strive for the second alternative but am ready to > change to number one. I think that regardless we should be using the xorrisofs charset. The issue of LANG may or may not be an issue, I'm inclined to believe not. I was only using this method to get the test working. The configuration of locales and LANG should be a user problem, but ideally should not affect how the tests are run. So in my judgement option 2 is the right way forward. If there are other LANG related issues we may have to mess with it in the future, but we can cross that bridge if we need to. I'm looking forward to your patch and will make sure this one gets dropped. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 00/12] Grub-shell improvements
Hi Daniel, What are the chances this patch series can be reviewed in the near future? Some feedback would be greatly appreciated. Glenn On Wed, 31 Mar 2021 20:57:39 -0500 Glenn Washburn wrote: > I've extracted this patch series from the GitLab CI patch series, > which some minor modifications. Mostly these are improvements to > grub-shell to facilitate debugging. > > The only patch that might be considered a fix, as opposed to an > improvement, would be patch #11, which fixes the issue where > qemu-mips is given a non-existant machine type. And while this was > discussed here on the list as a possible solution, I couldn't get the > mips tests working, so I'm not sure it's the right fix. > > Glenn > > Glenn Washburn (12): > grub-shell: Allow specifying non-default trim line contents > grub-shell: Trim line should always be matched from the beginning of > the line > grub-shell: Only show grub-mkrescue output if it returns an error > grub-shell: Allow setting default timeout via > GRUB_SHELL_DEFAULT_TIMEOUT envvar > grub-shell: Put all generated files into working dir and use better > file names > grub-shell: Add grub output logfile with grub-shell --debug > grub-shell: Set exit status to qemu exit status > tests: Allow turning on shell tracing from environment variables > grub-shell: Add --verbose to mkrescue when $debug is greater than 2 > grub-shell: Only turn on qemu head when large debug value is > specified grub-shell: Use malta qemu-mips machine type instead off > non-existant indy > grub-shell: Update qemu UEFI firmware names to be more generic > > tests/util/grub-fs-tester.in | 2 + > tests/util/grub-shell.in | 89 > 2 files changed, 63 > insertions(+), 28 deletions(-) > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test
LANG must be set to something that supports international characters,
otherwise xorriso will refuse to include the file with name having
international characters, causing the test to fail. So if LANG is not set,
set it to en_US.UTF-8, a very common UTF-8 locale. And if it is set, but
does not look like a UTF-8 locale, print a warning so the user will have a
clue as to why the iso9660_test might be failing.
Signed-off-by: Glenn Washburn
---
tests/iso9660_test.in | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/iso9660_test.in b/tests/iso9660_test.in
index 571b938d7..37b1d30af 100644
--- a/tests/iso9660_test.in
+++ b/tests/iso9660_test.in
@@ -7,6 +7,12 @@ if ! which xorriso >/dev/null 2>&1; then
exit 77
fi
+if [ -z "$LANG" ]; then
+ export LANG=en_US.UTF-8
+elif [ -n "${LANG##*UTF*}" ]; then
+ echo "WARNING: LANG=$LANG appears to not be unicode, international file
test may fail."
+fi
+
"@builddir@/grub-fs-tester" joliet
"@builddir@/grub-fs-tester" rockridge
"@builddir@/grub-fs-tester" rockridge_joliet
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 3/8] tests: When checking squashfs fstime, use superblock last modified time
Currently, the filesystem timestamp check in grub-fs-tester uses the
squashfs image file's last modified timestamp and checks to see if that
time stamp is within 3 seconds of the superblock timestamp as determined by
grub. The image file's timestamp could be more than 3 seconds off if
mksquashfs takes more than 3 seconds to generate the image, as is the case
on a virtual machine. Instead use squashfs tools to get the filesystem
timestamp directly.
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index bfc425e1f..4213b7bfc 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -1351,6 +1351,12 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
# With some abstractions like mdraid flushing to disk
# may be delayed for a long time.
FSTIME="$UMOUNT_TIME";;
+ xsquash*)
+ # Creating the squash image may take more than a few
+ # seconds. Use the more accurate timestamp from the
+ # superblock.
+ FSTIME="$(unsquashfs -s "${FSIMAGEP}0.img" | grep
^Creation | awk '{print $6 " " $7 " " $8 " " $9 " " $10; }')"
+ FSTIME="$(date -d "$FSTIME" -u '+%Y-%m-%d %H:%M:%S')";;
*)
FSTIME="$(TZ=UTC ls --time-style="+%Y-%m-%d_%H:%M:%S"
-l -d "${FSIMAGEP}0.img"|awk '{print $6; }'|sed 's,_, ,g')";;
esac
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 2/8] tests: Fix partmap_test for arm*-efi, disk numbering has changed
Perhaps using a newer UEFI firmware is the reason for the created test disk
showing up as hd2 instead of hd3.
Signed-off-by: Glenn Washburn
---
tests/partmap_test.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 6ef518b0a..7353dc70e 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -89,7 +89,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
disk=arc/scsi0/disk0/rdisk0
;;
arm*-efi)
- disk=hd3
+ disk=hd2
;;
*)
disk=hd0
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 7/8] tests: Exit with skipped exit code when test not performed
These tests were not performed and therefore did not pass, nor fail. This
fixes misleading test exit code where, for instance, the pseries_test will
pass on i386-pc, which is not a pseries architecture.
Signed-off-by: Glenn Washburn
---
tests/ahci_test.in | 8
tests/cdboot_test.in| 8
tests/core_compress_test.in | 2 +-
tests/ehci_test.in | 8
tests/fddboot_test.in | 16
tests/grub_cmd_date.in | 2 +-
tests/grub_cmd_set_date.in | 6 +++---
tests/hddboot_test.in | 6 +++---
tests/netboot_test.in | 12 ++--
tests/ohci_test.in | 8
tests/partmap_test.in | 8
tests/pata_test.in | 6 +++---
tests/pseries_test.in | 2 +-
tests/uhci_test.in | 8
14 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/tests/ahci_test.in b/tests/ahci_test.in
index a2bcff6b9..0e1c3679b 100644
--- a/tests/ahci_test.in
+++ b/tests/ahci_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
# PLATFORM: Don't mess with real devices when OS is active
*-emu)
- exit 0;;
+ exit 77;;
# FIXME: qemu gets bonito DMA wrong
mipsel-loongson)
- exit 0;;
+ exit 77;;
# PLATFORM: no AHCI on ARC and qemu-mips platforms
mips*-arc | mips*-qemu_mips)
- exit 0;;
+ exit 77;;
# FIXME: No native drivers are available for those
powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
- exit 0;;
+ exit 77;;
esac
imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
diff --git a/tests/cdboot_test.in b/tests/cdboot_test.in
index 7229f79fb..c0a5212ff 100644
--- a/tests/cdboot_test.in
+++ b/tests/cdboot_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
# PLATFORM: emu is different
*-emu)
- exit 0;;
+ exit 77;;
# PLATFORM: Flash targets
i386-qemu | i386-coreboot | mips-qemu_mips | mipsel-qemu_mips)
- exit 0;;
+ exit 77;;
# FIXME: currently grub-shell uses only -kernel for loongson
mipsel-loongson)
- exit 0;;
+ exit 77;;
# FIXME: OFW fails to open CD-ROM
i386-ieee1275)
- exit 0;;
+ exit 77;;
esac
v=`echo hello | "${grubshell}" --boot=cd`
diff --git a/tests/core_compress_test.in b/tests/core_compress_test.in
index 90dd00607..72d2eca7d 100644
--- a/tests/core_compress_test.in
+++ b/tests/core_compress_test.in
@@ -22,7 +22,7 @@ grubshell=@builddir@/grub-shell
case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
# FIXME: Only mips currently supports configurable core compression
*-emu | i386-* | x86_64-* | sparc64-* | ia64-*)
- exit 0
+ exit 77
;;
esac
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index da7652bd3..bd80f93d4 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -22,16 +22,16 @@ grubshell=@builddir@/grub-shell
case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
# PLATFORM: Don't mess with real devices when OS is active
*-emu)
- exit 0;;
+ exit 77;;
# FIXME: qemu gets bonito DMA wrong
mipsel-loongson)
- exit 0;;
+ exit 77;;
# PLATFORM: no USB on ARC and qemu-mips platforms
mips*-arc | mips*-qemu_mips)
- exit 0;;
+ exit 77;;
# FIXME: No native drivers are available for those
powerpc-ieee1275 | sparc64-ieee1275 | arm*-efi)
- exit 0;;
+ exit 77;;
esac
imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
diff --git a/tests/fddboot_test.in b/tests/fddboot_test.in
index 1bbe60ee5..5348ac56b 100644
--- a/tests/fddboot_test.in
+++ b/tests/fddboot_test.in
@@ -22,28 +22,28 @@ grubshell=@builddir@/grub-shell
case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
# PLATFORM: emu is different
*-emu)
- exit 0;;
+ exit 77;;
# PLATFORM: Flash targets
i386-qemu | i386-coreboot | mips-qemu_mips | mipsel-qemu_mips)
- exit 0;;
+ exit 77;;
# FIXME: currently grub-shell uses only -kernel for loongson
mipsel-loongson)
- exit 0;;
+ exit 77;;
# FIXME: We don't support EFI floppy boot in grub-mkrescue
*-efi)
- exit 0;;
+ exit 77;;
# FIXME: no floppy support
i386-multiboot)
- exit 0;;
+ exit 77;;
# FIXME: QEMU firmware crashes when trying to boot from floppy
sparc64-ieee1275)
- exit 0;;
+ exit 77;;
# FIXME: QEMU doesn't emulate SCSI floppies
mipsel-arc | mips-arc)
- exit 0;;
+ exit 77;;
# PLATFORM: powerpc doesn't boot from floppy except OldWorld Macs which we
don't support anyway
powerpc-ieee1275)
- exit 0;;
+ exit 77;;
[PATCH v2 0/8] Various fixes/improvements for tests
This is an update to the first version of this patch series to account for
changes in tests/ahci_test.in which cause a conflict in the previous patch
series. Nothing of substance has changed which would invalidate the previous
cover letter, to which I'll refer for comments on the patches.
Glenn
Glenn Washburn (8):
tests: Make sure LANG is set properly for iso9660_test
tests: Fix partmap_test for arm*-efi, disk numbering has changed
tests: When checking squashfs fstime, use superblock last modified
time
tests: Fail immediately when grub-shell fails and do not occlude the
error code
tests: Make setup errors in grub-fs-tester hard errors
tests: A failure of mktemp should cause the test script to exit with
code 99
tests: Exit with skipped exit code when test not performed
tests: Use @BUILD_SHEBANG@ autoconf var instead of literal shell
tests/ahci_test.in | 18 +++---
tests/cdboot_test.in | 11 ++-
tests/core_compress_test.in| 8 +---
tests/ehci_test.in | 18 +++---
tests/f2fs_test.in | 2 +-
tests/fddboot_test.in | 19 ++-
tests/gettext_strings_test.in | 2 +-
tests/grub_cmd_date.in | 5 +++--
tests/grub_cmd_set_date.in | 6 +++---
tests/grub_cmd_test.in | 7 ---
tests/grub_script_blockarg.in | 4 ++--
tests/grub_script_expansion.in | 3 ++-
tests/gzcompress_test.in | 3 ++-
tests/hddboot_test.in | 9 +
tests/iso9660_test.in | 6 ++
tests/lzocompress_test.in | 3 ++-
tests/netboot_test.in | 15 ---
tests/ohci_test.in | 18 +++---
tests/partmap_test.in | 18 +-
tests/pata_test.in | 13 +++--
tests/pseries_test.in | 2 +-
tests/syslinux_test.in | 2 +-
tests/test_sha512sum.in| 7 ---
tests/uhci_test.in | 18 +++---
tests/util/grub-fs-tester.in | 17 -
tests/xzcompress_test.in | 3 ++-
26 files changed, 140 insertions(+), 97 deletions(-)
Range-diff against v1:
1: c1ff77eec = 1: 17ddccb2e tests: Make sure LANG is set properly for
iso9660_test
2: eb4bf8cb6 = 2: 62ed781c8 tests: Fix partmap_test for arm*-efi, disk
numbering has changed
3: b00c1df00 = 3: 066486e22 tests: When checking squashfs fstime, use
superblock last modified time
4: 73f81a5b4 ! 4: 562d74a49 tests: Fail immediately when grub-shell fails and
do not occlude the error code
@@ tests/ahci_test.in: echo "hello" > "$outfile"
tar cf "$imgfile" "$outfile"
--if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}"
--qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci -device
ide-drive,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; then
+-if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}"
--qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci -device
ide-hd,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(ahci0)/$outfile';" |
+"${grubshell}" --qemu-opts="-drive id=disk,file=$imgfile,if=none
+ -device ahci,id=ahci
-+ -device ide-drive,drive=disk,bus=ahci.0"
>$outfile
++ -device ide-hd,drive=disk,bus=ahci.0" >$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
rm "$imgfile"
rm "$outfile"
5: 946730f71 = 5: af85ae97f tests: Make setup errors in grub-fs-tester hard
errors
6: 0e49be59e = 6: fcb2b7a06 tests: A failure of mktemp should cause the test
script to exit with code 99
7: f08c84e58 = 7: 47afc3569 tests: Exit with skipped exit code when test not
performed
8: dca75ed56 = 8: 40f77d41b tests: Use @BUILD_SHEBANG@ autoconf var instead
of literal shell
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 5/8] tests: Make setup errors in grub-fs-tester hard errors
When a test program fails because it failed to setup the test properly, this
does not indicate a failure in what is attempting to be tested because the
test is never run. So exit with a hard error exit status to note this
difference. This will allow easier detection of tests that are not actually
being run and those that are really failing.
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 4213b7bfc..eacb1e0a7 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -6,7 +6,8 @@ fs="$1"
GRUBFSTEST="@builddir@/grub-fstest"
-tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XX"` || exit 1
+tempdir=`mktemp -d "${TMPDIR:-/tmp}/tmp.XX"` ||
+{ echo "Failed to make temporary directory"; exit 99; }
# This wrapper is to ease insertion of valgrind or time statistics
run_it () {
@@ -273,7 +274,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
done
if test "$CFILESRC" = "" ; then
echo "Couldn't find compressible file" >&2
- exit 1
+ exit 99
fi
case x"$fs" in
# FS LIMITATION: 8.3 names
@@ -616,7 +617,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
mkdir -p "$MNTPOINTRO"
for i in $(range 0 $((NDEVICES-1)) 1); do
dd if=/dev/zero of="$FSIMAGEP${i}.img" count=1 bs=1
seek=$((DISKSIZE-1)) &> /dev/null
- LODEVICE=$(losetup --find --show "$FSIMAGEP${i}.img")
+ LODEVICE=$(losetup --find --show "$FSIMAGEP${i}.img")
|| exit 99
LODEVICES="$LODEVICES $LODEVICE"
if test "$i" = 0; then
MOUNTDEVICE="$LODEVICE"
@@ -820,7 +821,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
"mkfs.xfs" -m crc=1 -b size=$BLKSIZE -s size=$SECSIZE -L
"$FSLABEL" -q "${MOUNTDEVICE}" ;;
*)
echo "Add appropriate mkfs command here"
- exit 1
+ exit 99
;;
esac
BASEFILE="1.img"
@@ -926,7 +927,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
for i in $(range 0 $((NDEVICES-1)) 1); do
rm "$FSIMAGEP${i}.img"
done
- exit 1;
+ exit 99;
fi
;;
esac
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 6/8] tests: A failure of mktemp should cause the test script to exit with code 99
A test exiting with code 99 means that there was an error in the test itself
and not a failure in the thing being tested (also known as a hard error).
Signed-off-by: Glenn Washburn
---
tests/ahci_test.in| 4 ++--
tests/ehci_test.in| 4 ++--
tests/gettext_strings_test.in | 2 +-
tests/grub_cmd_test.in| 6 +++---
tests/grub_script_blockarg.in | 2 +-
tests/ohci_test.in| 4 ++--
tests/partmap_test.in | 4 ++--
tests/pata_test.in| 4 ++--
tests/syslinux_test.in| 2 +-
tests/test_sha512sum.in | 6 +++---
tests/uhci_test.in| 4 ++--
11 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/tests/ahci_test.in b/tests/ahci_test.in
index 30dc9d31a..a2bcff6b9 100644
--- a/tests/ahci_test.in
+++ b/tests/ahci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
echo "hello" > "$outfile"
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index 64bd80070..da7652bd3 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
echo "hello" > "$outfile"
diff --git a/tests/gettext_strings_test.in b/tests/gettext_strings_test.in
index 813999ebe..1c37fe41b 100644
--- a/tests/gettext_strings_test.in
+++ b/tests/gettext_strings_test.in
@@ -2,7 +2,7 @@
cd '@srcdir@'
-tdir="$(mktemp -d "${TMPDIR:-/tmp}/tmp.XX")"
+tdir="$(mktemp -d "${TMPDIR:-/tmp}/tmp.XX")" || exit 99
xgettext -f po/POTFILES.in -L C -o "$tdir/"skip.pot -x po/grub.pot
--keyword=grub_util_info --keyword=grub_dprintf:1,2
--keyword=grub_register_command --keyword=grub_register_extcmd
--keyword=grub_efiemu_resolve_symbol --keyword=grub_efiemu_register_symbol
--keyword=grub_dl_load --keyword=grub_crypto_lookup_md_by_name
--keyword=grub_crypto_lookup_cipher_by_name
--keyword=grub_efiemu_resolve_symbol --keyword=alias
--keyword=grub_ieee1275_get_property:2 --keyword=grub_ieee1275_find_device
--keyword=grub_ieee1275_get_integer_property:2 --keyword=INIT_IEEE1275_COMMON:2
--keyword=grub_boot_time --keyword=grub_env_get --keyword=grub_env_set
--keyword=grub_register_variable_hook --keyword=grub_fatal --keyword=__asm__
--keyword=volatile --keyword=__volatile__ --keyword=grub_error:2
--from-code=iso-8859-1
xgettext -f po/POTFILES.in -L C -o "$tdir/"skip2.pot -x po/grub.pot
--keyword=volatile:2 --keyword=__volatile__:2 --keyword=grub_dprintf:2
--from-code=iso-8859-1
diff --git a/tests/grub_cmd_test.in b/tests/grub_cmd_test.in
index dac6f7d6a..043c3a634 100644
--- a/tests/grub_cmd_test.in
+++ b/tests/grub_cmd_test.in
@@ -2,8 +2,8 @@
set -e
# create a randome file
-empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-non_empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+non_empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
cat >$non_empty <$outfile <$tmp
error_if_not "`head -n1 $tmp|tail -n1`" '{ test_blockarg { true } }'
diff --git a/tests/ohci_test.in b/tests/ohci_test.in
index cc35f67a9..c55aad4ad 100644
--- a/tests/ohci_test.in
+++ b/tests/ohci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
echo "hello" > "$outfile"
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 5f18ab51c..7906db43d 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -101,8 +101,8 @@ if ! which parted >/dev/null 2>&1; then
exit 77
fi
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp &q
[PATCH v2 4/8] tests: Fail immediately when grub-shell fails and do not occlude the error code
If grub-shell fails, that means that whatever was being tested was not
actually tested. So fail immediately. Sometimes grub-shell is not the last
command in a pipeline of several commands, and in this case the failed error
code can be hidden by a later failing command or hidden when 'set -e' is not
set and there are subsequent successful commands. When the test script fails
because of a failure in grub-shell, then test script should exit with the
failed exit code of grub-shell.
Signed-off-by: Glenn Washburn
---
tests/ahci_test.in | 6 +-
tests/cdboot_test.in | 3 ++-
tests/core_compress_test.in| 6 --
tests/ehci_test.in | 6 +-
tests/fddboot_test.in | 3 ++-
tests/grub_cmd_date.in | 3 ++-
tests/grub_cmd_test.in | 1 +
tests/grub_script_blockarg.in | 2 +-
tests/grub_script_expansion.in | 3 ++-
tests/gzcompress_test.in | 3 ++-
tests/hddboot_test.in | 3 ++-
tests/lzocompress_test.in | 3 ++-
tests/netboot_test.in | 3 ++-
tests/ohci_test.in | 6 +-
tests/partmap_test.in | 4 ++--
tests/pata_test.in | 3 ++-
tests/test_sha512sum.in| 1 +
tests/uhci_test.in | 6 +-
tests/xzcompress_test.in | 3 ++-
19 files changed, 49 insertions(+), 19 deletions(-)
diff --git a/tests/ahci_test.in b/tests/ahci_test.in
index d844fe680..30dc9d31a 100644
--- a/tests/ahci_test.in
+++ b/tests/ahci_test.in
@@ -41,7 +41,11 @@ echo "hello" > "$outfile"
tar cf "$imgfile" "$outfile"
-if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}"
--qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci -device
ide-hd,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(ahci0)/$outfile';" |
+"${grubshell}" --qemu-opts="-drive id=disk,file=$imgfile,if=none
+ -device ahci,id=ahci
+ -device ide-hd,drive=disk,bus=ahci.0" >$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
rm "$imgfile"
rm "$outfile"
exit 1
diff --git a/tests/cdboot_test.in b/tests/cdboot_test.in
index 75acdfedb..7229f79fb 100644
--- a/tests/cdboot_test.in
+++ b/tests/cdboot_test.in
@@ -34,6 +34,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-if [ "$(echo hello | "${grubshell}" --boot=cd)" != "Hello World" ]; then
+v=`echo hello | "${grubshell}" --boot=cd`
+if [ "$v" != "Hello World" ]; then
exit 1
fi
diff --git a/tests/core_compress_test.in b/tests/core_compress_test.in
index 9d216ebcf..90dd00607 100644
--- a/tests/core_compress_test.in
+++ b/tests/core_compress_test.in
@@ -27,10 +27,12 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}"
in
esac
-if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz)" !=
"Hello World" ]; then
+v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz`
+if [ "$v" != "Hello World" ]; then
exit 1
fi
-if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=none)" !=
"Hello World" ]; then
+v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=none`
+if [ "$v" != "Hello World" ]; then
exit 1
fi
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index b197f8cdc..64bd80070 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -41,7 +41,11 @@ echo "hello" > "$outfile"
tar cf "$imgfile" "$outfile"
-if [ "$(echo "nativedisk; source '(usb0)/$outfile';" | "${grubshell}"
--qemu-opts="-device ich9-usb-ehci1 -drive id=my_usb_disk,file=$imgfile,if=none
-device usb-storage,drive=my_usb_disk" | tail -n 1)" != "Hello World" ]; then
+echo "nativedisk; source '(usb0)/$outfile';" |
+"${grubshell}" --qemu-opts="-device ich9-usb-ehci1
+ -drive id=my_usb_disk,file=$imgfile,if=none
+ -device usb-storage,drive=my_usb_disk" >$outfile
+if [ "$(cat "$outfile" | tail -n 1)" != "Hello World" ]; then
rm "$imgfile"
rm "$outfile"
exit 1
diff --git a/tests/fddboot_test.in b/tests/fddboot_test.in
index 2d7dfc889..1bbe60ee5 100644
--- a/tests/fddboot_test.in
+++ b/tests/fddboot_test.in
@@ -46,6 +46,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-if [ "$(echo hello | "${grubshell}" --boot=fd --mkrescue-arg="
[PATCH v2 8/8] tests: Use @BUILD_SHEBANG@ autoconf var instead of literal shell
This bring this test in line with the rest of the test scripts. Signed-off-by: Glenn Washburn --- tests/f2fs_test.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in index 1ea77c826..8c415db61 100644 --- a/tests/f2fs_test.in +++ b/tests/f2fs_test.in @@ -1,4 +1,4 @@ -#!/bin/sh +#!@BUILD_SHEBANG@ set -e -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 1/3] cryptodisk: Add infrastructure to pass data from cryptomount to cryptodisk modules
As an example, passing a password as a cryptomount argument is implemented.
However, the backends are not implemented, so testing this will return a not
implemented error.
Signed-off-by: Glenn Washburn
---
grub-core/disk/cryptodisk.c | 31 ++-
grub-core/disk/geli.c | 4
grub-core/disk/luks.c | 4
grub-core/disk/luks2.c | 4
include/grub/cryptodisk.h | 8
5 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 90f82b2d3..b966b19ab 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -41,6 +41,7 @@ static const struct grub_arg_option options[] =
/* TRANSLATORS: It's still restricted to cryptodisks only. */
{"all", 'a', 0, N_("Mount all."), 0, 0},
{"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0},
+{"password", 'p', 0, N_("Password to open volumes."), 0, ARG_TYPE_STRING},
{0, 0, 0, 0, 0, 0}
};
@@ -996,7 +997,9 @@ cryptodisk_close (grub_cryptodisk_t dev)
}
static grub_err_t
-grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
+grub_cryptodisk_scan_device_real (const char *name,
+ grub_disk_t source,
+ grub_cryptomount_args_t cargs)
{
grub_err_t err;
grub_cryptodisk_t dev;
@@ -1015,7 +1018,9 @@ grub_cryptodisk_scan_device_real (const char *name,
grub_disk_t source)
if (!dev)
continue;
+*dev->cargs = *cargs;
err = cr->recover_key (source, dev);
+*dev->cargs = NULL;
if (err)
{
cryptodisk_close (dev);
@@ -1080,10 +1085,11 @@ grub_cryptodisk_cheat_mount (const char *sourcedev,
const char *cheat)
static int
grub_cryptodisk_scan_device (const char *name,
-void *data __attribute__ ((unused)))
+void *data)
{
grub_err_t err;
grub_disk_t source;
+ grub_cryptomount_args_t cargs = data;
/* Try to open disk. */
source = grub_disk_open (name);
@@ -1093,7 +1099,7 @@ grub_cryptodisk_scan_device (const char *name,
return 0;
}
- err = grub_cryptodisk_scan_device_real (name, source);
+ err = grub_cryptodisk_scan_device_real (name, source, cargs);
grub_disk_close (source);
@@ -1106,12 +1112,19 @@ static grub_err_t
grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
{
struct grub_arg_list *state = ctxt->state;
+ struct grub_cryptomount_args cargs = {0};
if (argc < 1 && !state[1].set && !state[2].set)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
+ if (state[3].set) /* password */
+{
+ cargs.key_data = (grub_uint8_t *) state[3].arg;
+ cargs.key_len = grub_strlen(state[3].arg);
+}
+
have_it = 0;
- if (state[0].set)
+ if (state[0].set) /* uuid */
{
grub_cryptodisk_t dev;
@@ -1125,18 +1138,18 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
argc, char **args)
check_boot = state[2].set;
search_uuid = args[0];
- grub_device_iterate (_cryptodisk_scan_device, NULL);
+ grub_device_iterate (_cryptodisk_scan_device, );
search_uuid = NULL;
if (!have_it)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk found");
return GRUB_ERR_NONE;
}
- else if (state[1].set || (argc == 0 && state[2].set))
+ else if (state[1].set || (argc == 0 && state[2].set)) /* -a|-b */
{
search_uuid = NULL;
check_boot = state[2].set;
- grub_device_iterate (_cryptodisk_scan_device, NULL);
+ grub_device_iterate (_cryptodisk_scan_device, );
search_uuid = NULL;
return GRUB_ERR_NONE;
}
@@ -1178,7 +1191,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
argc, char **args)
return GRUB_ERR_NONE;
}
- err = grub_cryptodisk_scan_device_real (diskname, disk);
+ err = grub_cryptodisk_scan_device_real (diskname, disk, );
grub_disk_close (disk);
if (disklast)
@@ -1317,7 +1330,7 @@ GRUB_MOD_INIT (cryptodisk)
{
grub_disk_dev_register (_cryptodisk_dev);
cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
- N_("SOURCE|-u UUID|-a|-b"),
+ N_("[-p password] "),
N_("Mount a crypto device."), options);
grub_procfs_register ("luks_script", _script);
}
diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
index 2f34a35e6..0a7bd90da 100644
--- a/grub-core/disk/geli.c
+++ b/grub-core/disk/geli.c
@@ -414,6 +414,10 @@ recover_key (grub_disk_t source, grub_cryptodisk_t dev)
grub_disk_addr_t sector;
grub_err_t err;
+ /* Keyfiles are not implemented yet */
+ if
[PATCH 0/3] Refactor/improve cryptomount data passing to crypto modules
This patch series refactors the way cryptomount passes data to the crypto modules. Currently, the method has been by global variable and function call argument, neither of which are ideal. This method passes data via a grub_cryptomount_args struct, which can be added to over time as opposed to continually adding arguments to the cryptodisk recover_key (as is being proposed in the keyfile and detached header patches). The infrastructure is implemented in patch #1 along with adding a new -p parameter to cryptomount partly as an example to show how a password would be passed to the crypto module backends. The backends do nothing with this data in this patch, but print a message saying that sending a password is unimplemented. Patch #2 takes advantage of this new data passing mechanism to refactor the essentially duplicated code in each crypto backend module for inputting the password and puts that functionality in the cryptodisk code. Conceptually, the crypto backends should not be getting user input anyway. Finally patch #3, gets rid of some long time globals in cryptodisk, moving them into the passed struct. My intention is for this patch series to lay the foundation for an improved patch series providing detached header and keyfile support (I already have the series updated and ready to send once this is accepted). I also believe tha this will somewhat simplify the patch series by James Bottomley in passing secrets to the crypto backends. Glenn Glenn Washburn (3): cryptodisk: Add infrastructure to pass data from cryptomount to cryptodisk modules cryptodisk: Refactor password input out of crypto dev modules into cryptodisk cryptodisk: Move global variables into grub_cryptomount_args struct grub-core/disk/cryptodisk.c | 109 grub-core/disk/geli.c | 24 ++-- grub-core/disk/luks.c | 25 ++--- grub-core/disk/luks2.c | 24 ++-- include/grub/cryptodisk.h | 12 5 files changed, 102 insertions(+), 92 deletions(-) -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 3/3] cryptodisk: Move global variables into grub_cryptomount_args struct
Signed-off-by: Glenn Washburn
---
grub-core/disk/cryptodisk.c | 26 +-
include/grub/cryptodisk.h | 3 +++
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index b6cf1835d..00a671a59 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -984,9 +984,6 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
#endif
-static int check_boot, have_it;
-static char *search_uuid;
-
static void
cryptodisk_close (grub_cryptodisk_t dev)
{
@@ -1014,7 +1011,7 @@ grub_cryptodisk_scan_device_real (const char *name,
FOR_CRYPTODISK_DEVS (cr)
{
-dev = cr->scan (source, search_uuid, check_boot);
+dev = cr->scan (source, cargs->search_uuid, cargs->check_boot);
if (grub_errno)
return grub_errno;
if (!dev)
@@ -1051,7 +1048,7 @@ grub_cryptodisk_scan_device_real (const char *name,
grub_cryptodisk_insert (dev, name, source);
-have_it = 1;
+cargs->found_uuid = 1;
goto cleanup;
}
@@ -1093,7 +1090,7 @@ grub_cryptodisk_cheat_mount (const char *sourcedev, const
char *cheat)
FOR_CRYPTODISK_DEVS (cr)
{
-dev = cr->scan (source, search_uuid, check_boot);
+dev = cr->scan (source, NULL, 0);
if (grub_errno)
return grub_errno;
if (!dev)
@@ -1137,7 +1134,7 @@ grub_cryptodisk_scan_device (const char *name,
if (err)
grub_print_error ();
- return have_it && search_uuid ? 1 : 0;
+ return (cargs->found_uuid && cargs->search_uuid) ? 1 : 0;
}
static grub_err_t
@@ -1155,7 +1152,6 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
argc, char **args)
cargs.key_len = grub_strlen(state[3].arg);
}
- have_it = 0;
if (state[0].set) /* uuid */
{
grub_cryptodisk_t dev;
@@ -1168,21 +1164,18 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
argc, char **args)
return GRUB_ERR_NONE;
}
- check_boot = state[2].set;
- search_uuid = args[0];
+ cargs.check_boot = state[2].set;
+ cargs.search_uuid = args[0];
grub_device_iterate (_cryptodisk_scan_device, );
- search_uuid = NULL;
- if (!have_it)
+ if (!cargs.found_uuid)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk found");
return GRUB_ERR_NONE;
}
else if (state[1].set || (argc == 0 && state[2].set)) /* -a|-b */
{
- search_uuid = NULL;
- check_boot = state[2].set;
+ cargs.check_boot = state[2].set;
grub_device_iterate (_cryptodisk_scan_device, );
- search_uuid = NULL;
return GRUB_ERR_NONE;
}
else
@@ -1194,8 +1187,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int
argc, char **args)
char *disklast = NULL;
grub_size_t len;
- search_uuid = NULL;
- check_boot = state[2].set;
+ cargs.check_boot = state[2].set;
diskname = args[0];
len = grub_strlen (diskname);
if (len && diskname[0] == '(' && diskname[len - 1] == ')')
diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
index 1070140d9..11062f43a 100644
--- a/include/grub/cryptodisk.h
+++ b/include/grub/cryptodisk.h
@@ -69,6 +69,9 @@ typedef gcry_err_code_t
struct grub_cryptomount_args
{
+ grub_uint32_t check_boot : 1;
+ grub_uint32_t found_uuid : 1;
+ char *search_uuid;
grub_uint8_t *key_data;
grub_size_t key_len;
};
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 2/3] cryptodisk: Refactor password input out of crypto dev modules into cryptodisk
The crypto device modules should only be setting up the crypto devices and
not getting user input. This has the added benefit of simplifying the code
such that three essentially duplicate pieces of code are merged into one.
Signed-off-by: Glenn Washburn
---
grub-core/disk/cryptodisk.c | 52 ++---
grub-core/disk/geli.c | 26 ---
grub-core/disk/luks.c | 27 +++
grub-core/disk/luks2.c | 26 ---
include/grub/cryptodisk.h | 1 +
5 files changed, 57 insertions(+), 75 deletions(-)
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index b966b19ab..b6cf1835d 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -1001,9 +1001,11 @@ grub_cryptodisk_scan_device_real (const char *name,
grub_disk_t source,
grub_cryptomount_args_t cargs)
{
- grub_err_t err;
+ grub_err_t ret = GRUB_ERR_NONE;
grub_cryptodisk_t dev;
grub_cryptodisk_dev_t cr;
+ int askpass = 0;
+ char *part = NULL;
dev = grub_cryptodisk_get_by_source_disk (source);
@@ -1017,23 +1019,53 @@ grub_cryptodisk_scan_device_real (const char *name,
return grub_errno;
if (!dev)
continue;
-
+
+if (cargs->key_len == 0)
+ {
+ /* Get the passphrase from the user, if no key data. */
+ askpass = 1;
+ if (source->partition)
+ part = grub_partition_get_name (source->partition);
+ grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
+source->partition ? "," : "", part ? : "",
+dev->uuid);
+ grub_free (part);
+
+ cargs->key_data = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
+ if (!cargs->key_data)
+ return grub_errno;
+
+ if (!grub_password_get ((char *) cargs->key_data,
GRUB_CRYPTODISK_MAX_PASSPHRASE))
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
+ goto error;
+ }
+ cargs->key_len = grub_strlen((char *) cargs->key_data);
+ }
+
*dev->cargs = *cargs;
-err = cr->recover_key (source, dev);
+ret = cr->recover_key (source, dev);
*dev->cargs = NULL;
-if (err)
-{
- cryptodisk_close (dev);
- return err;
-}
+if (ret)
+ goto error;
grub_cryptodisk_insert (dev, name, source);
have_it = 1;
-return GRUB_ERR_NONE;
+goto cleanup;
}
- return GRUB_ERR_NONE;
+ goto cleanup;
+
+error:
+ cryptodisk_close (dev);
+cleanup:
+ if (askpass)
+{
+ cargs->key_len = 0;
+ grub_free(cargs->key_data);
+}
+ return ret;
}
#ifdef GRUB_UTIL
diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
index 0a7bd90da..b4525ed48 100644
--- a/grub-core/disk/geli.c
+++ b/grub-core/disk/geli.c
@@ -135,8 +135,6 @@ const char *algorithms[] = {
[0x16] = "aes"
};
-#define MAX_PASSPHRASE 256
-
static gcry_err_code_t
geli_rekey (struct grub_cryptodisk *dev, grub_uint64_t zoneno)
{
@@ -406,17 +404,14 @@ recover_key (grub_disk_t source, grub_cryptodisk_t dev)
grub_uint8_t verify_key[GRUB_CRYPTO_MAX_MDLEN];
grub_uint8_t zero[GRUB_CRYPTO_MAX_CIPHER_BLOCKSIZE];
grub_uint8_t geli_cipher_key[64];
- char passphrase[MAX_PASSPHRASE] = "";
unsigned i;
gcry_err_code_t gcry_err;
struct grub_geli_phdr header;
- char *tmp;
grub_disk_addr_t sector;
grub_err_t err;
- /* Keyfiles are not implemented yet */
- if (dev->cargs->key_data || dev->cargs->key_len)
- return GRUB_ERR_NOT_IMPLEMENTED_YET;
+ if (dev->cargs->key_data == NULL || dev->cargs->key_len == 0)
+return grub_error (GRUB_ERR_BAD_ARGUMENT, "No key data");
if (dev->cipher->cipher->blocksize > GRUB_CRYPTO_MAX_CIPHER_BLOCKSIZE)
return grub_error (GRUB_ERR_BUG, "cipher block is too long");
@@ -438,23 +433,12 @@ recover_key (grub_disk_t source, grub_cryptodisk_t dev)
grub_puts_ (N_("Attempting to decrypt master key..."));
- /* Get the passphrase from the user. */
- tmp = NULL;
- if (source->partition)
-tmp = grub_partition_get_name (source->partition);
- grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
- source->partition ? "," : "", tmp ? : "",
- dev->uuid);
- grub_free (tmp);
- if (!grub_password_get (passphrase, MAX_PASSPHRASE))
-return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
-
/* Calculate the PBKDF2 of the user supplied passphrase. */
if (grub_le_to_cpu32 (header.niter) != 0)
{
grub_uint8_t pbkdf_key[64];
- gcry_err = grub_crypto_pbkdf2 (dev->hash, (grub_uint8_t
Re: [PATCH v2 1/8] tests: Make sure LANG is set properly for iso9660_test
Hi Thomas, On Wed, 25 Aug 2021 11:34:47 +0200 "Thomas Schmitt" wrote: > Hi, > > Glenn Washburn wrote: > > LANG must be set to something that supports international > > characters, otherwise xorriso will refuse to include the file with > > name having international characters, causing the test to fail. > > Can you tell me the exact error message from xorriso ? > I have some difficulties with reproducing the situation. There is no error message from xorriso, it will do as you noticed below, which is to default to replacing non-ASCII characters with underscores. The problem is that that behavior causes the tests to fail (rightly so because non-ASCII names are not being tested). The following causes the test to fail, but removing the LANG= causes success (because my system has LANG set to a UTF8 encoding). make SUBDIRS= LANG= TESTS=iso9660_test check > Your description is plausible, because libisofs takes its character > set default from nl_langinfo(3) with item CODESET and uses iconv(3) to > convert the file names to the output charset. > > I would like to bring up for discussion an alternative remedy by these > xorrisofs options: > > -input-charset UTF-8 -output-charset UTF-8 > > --- > > I am quite sure that it will work, but have problems with creating the > situation where it shall be the remedy. > On a system with configured LANG=en_US.UTF-8 i fail to reproduce > the issue with LANG=C or LANG="" unless i add the explicit demand to > convert to UTF-8. > > My disappointingly unproblematic xorriso test run is > > ( LANG= xorrisofs -o test.iso 'ÄÖÜß' ) > > with various attempts to set the locale charset away from UTF-8: > LANG, LANGUAGE, LC_ALL set to empty text or to "C". > LANG= causes nl_langinfo(CODESET) to return "ANSI_X3.4-1968" but > iconv(3) throws no error when it shall convert > "\303\204\303\226\303\234\303\237". > > I get warning messages if i tell xorriso to aim for output charset > UTF-8 while LANG is empty and the file name contains non-ASCII > characters: > > (LANG= xorrisofs -o test.iso -output-charset UTF-8 'ÄÖÜß') > > yields three times > > libisofs: WARNING : Charset conversion error. Cannot convert ÄÖÜß > from ANSI_X3.4-1968 to UTF-8 libisofs: NOTE : > Caused by: Charset > conversion error > > Inspection of test.iso shows that the file name 'ÄÖÜß' was defaulted > to name ''. > > Adding the other charset option > > (LANG= xorrisofs -o test.iso \ > -input-charset UTF-8 -output-charset UTF-8 'ÄÖÜß') > > yields a run without warning messages resulting in Rock Ridge file > name 'ÄÖÜß'. > > So i assume that my proposal is a valid alternative to setting > LANG=UTF-8. Whether it is preferrable over setting LANG to UTF-8 > would have to be discussed. > At least it would be nice for me to know why LANG= causes trouble in > the GRUB tests but not on my command line. > (I see nothing in the xorriso runs of tests/util/grub-fs-tester.in > which would be equivalent to the run which fails to convert 'ÄÖÜß' > here.) The takeaway here is that 'ÄÖÜß' is being converted and its is precisely that which is causing the test to fail. So unless I'm mistaken, you are confirming the issue this patch resolves. It looks like you have a viable alternative to my proposed patch, though I've not tested it myself. I really don't care about the implementation as long as the test succeeds when LANG is unset or empty. Despite having an alternative approach, do you have concerns or objections to my patch? Also would you like to create a patch that I would be happy to test? Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [grub-fs-tester.in] zisofs test looks unsuitable
On Wed, 25 Aug 2021 12:29:48 +0200 "Thomas Schmitt" wrote: > Hi, > > the xorriso run for testing zisofs in > > tests/util/grub-fs-tester.in > > looks not like it would cause any zisofs compression in the ISO. > > Line 1024: > > xorriso -compliance rec_mtime -set_filter_r --zisofs -- -zisofs > default -as mkisofs ... some options ... -- > -set_filter_r --zisofs -- -zisofs default -add > /="$MASTER" ;; > > should probably become > > xorriso -compliance rec_mtime \ > -as mkisofs ... same options as above ... -- \ > -add /="$MASTER" -- \ > -zisofs default \ > -set_filter_r --zisofs / -- > ;; > > In the current run, both times when "-set_filter_r --zisofs" is > executed in xorriso's native command mode, the emerging root > directory is still empty. So -set_filter_r cannot yet affect the > files of "$MASTER" which get added only later. > > Command -zisofs sets parameters for -set_filter_r. So it must be > executed before any successful -set_filter_r. Only the lack of any > installed compression filters keeps the existing run from failing with > libisofs: FAILURE : Cannot set global zisofs parameters while > filters exist > > Further: > Command -set_filter_r expects one or more paths in the ISO as > parameters. The empty path list defaults to "/" indeed, but that's > not a documented feature. > Command -add has a variable length parameter list and should thus be > terminated by '--'. In my proposal it has to be terminated because > more commands follow. > > > Experiment: > > Currently the run is essentially like > > xorriso -compliance rec_mtime \ > -as mkisofs -o test.iso --graft-points -- \ > -add /=test -- -set_filter_r --zisofs / -- > > My test directory contains > > -rw-r--r-- 1 thomas thomas 51200 Aug 25 11:51 zeros > -rw-r--r-- 1 thomas thomas 6 Aug 25 09:37 ÄÖÜß > > No zisofs compression gets applied by above run to the files, as can > be seen by a subsequent run of > > xorriso -indev test.iso -find / -exec show_stream -- > > which reports > > '/zeros' < image:'/zeros' > '/ÄÖÜß' < image:'/ÄÖÜß' > > In contrast, my proposed xorriso run causes compression of file > "zeros", which then causes the xorriso run with -indev to install a > decompression filter to that file: > > '/zeros' < --zisofs-decode:pz:32k < image:'/zeros' > '/ÄÖÜß' < image:'/ÄÖÜß' > > The small "ÄÖÜß" staid uncompressed, because compression yielded no > gain in terms of 2048 byte blocks. If I'm not mistaken, there is currently no test which actually exercises that code, ie no test runs "grub-fs-tester ziso9660". I think it could be a good idea to have one though. However, when attempting to run the test manually, I'm getting a failure probably related to what you've found. $ ./grub-fs-tester ziso9660 xorriso 1.5.2 : RockRidge filesystem manipulator, libburnia project. Drive current: -outdev 'stdio:/tmp/tmp.82yFdjU5NO/ziso9660_512_512_1_0.img' Media current: stdio file, overwriteable Media status : is blank Media summary: 0 sessions, 0 data blocks, 0 data, 4621m free xorriso : FAILURE : -volid: Text too long (1650 > 32) xorriso : aborting : -abort_on 'FAILURE' encountered 'FAILURE' I haven't dug very far, but I'm unsure if GRUB currently is supposed to support this. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Where is the testing? (was: Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock)
On Wed, 8 Sep 2021 18:03:50 +0200 Daniel Kiper wrote: > On Wed, Sep 08, 2021 at 01:22:20AM +0000, Glenn Washburn wrote: > > It looks like the xfs_test test succeeds with tag grub-2.06-rc1a, > > fails with tag grub-2.06, and succeeds with current master. Yes, as > > expected. However, what this tells me is that no "make check" tests > > were done before finalizing the 2.06 release. I was under the > > impression that that was part of the release procedure. If its not, > > it would seem that we're not using the tests at a time when they > > would have the most impact. > > Currently I run build tests for all architectures and platforms and > Coverity for x86_64 EFI before every push and release. I know this is > not enough but I tried "make check" at least once and got the > impression that the tests are not reliable. Sadly I have not time to > dive deeper and fix this and that. If someone, you?, wants to verify > all tests and fix broken ones I will be more than happy to start > using it (it seems to me your "[PATCH v2 0/8] Various > fixes/improvements for tests" patch set fixes some issues but I am > not sure if all of them). I suspect the problem you're running into with the tests is more a matter of setting up the expected environment than the tests themselves. From my experience, most of the tests work once everything is setup right. My gitlab patch shows how to do this on Ubuntu 20.04. Due to not being able to load kernel modules on the shared gitlab runners, I was not able to run all the tests either, and there are some tests that fail. The failures that come to mind are on Sparc and some of the functional tests, and I've notified the list, but no one came forward to fix them. So what I've done is have a list of known failing tests and ignore those in determining whether the test suite as a whole has succeeded. I'm not convinced that that behavior is something we should include in the test framework itself. So to my mind, I have verified (nearly) all tests on most platforms. I have fixed the ones I could. I think the biggest low hanging fruit (for me), is to get the tests running on a system with all the needed kernel fs drivers loaded. Perhaps to your point, the filesystem tests failing due to not having a kernel module loaded should be marked as skipped because the test isn't really run. Here is an example of a successfully completed pipeline https://gitlab.com/gwashburn/grub/-/pipelines/258561619 And here is the raw log for the x86_64-efi build and test (search for "Testsuite summary" and look at the lines above for individual test pass/fail): https://storage.googleapis.com/gitlab-gprd-artifacts/0e/0d/0e0d2ccc27a1e9a121b3d4ef831c56b5fd81cc8b565a3dbc5fa09b9a58edbcb7/2021_02_19/1042905700/1137549157/job.log?response-content-type=text%2Fplain%3B%20charset%3Dutf-8=inline=gitlab-object-storage-...@gitlab-production.iam.gserviceaccount.com=ewpRWBqWWorOXak6hFkb5kUEfefU1biAtu6xY2Rtyds3%2BduM6q0kiFIKe4A5%0A8wS%2FPbd8Al3AwF45Q22KcpYyZ87UBkryQHjispAj%2B3tBQrnnyOYSRKGNV%2Bbz%0A4iaKAdYn4onIcJM9Ro4RkJ%2FCAY2tBxWmmLoEZ%2FQzWLvbhH%2BJSmuNqtEZXfuD%0AI1tXD1ZKgoLcYCCLNz8iaMFpgB32NfR2W6sDDuFb24ZFSy0X7H02KRVAMIor%0Akhj5QXdXGqoqFFXXMM9r8ZGv34Kh4GdDbVjMmJ%2FaDhvLPgmZF3UKnhcDYJoB%0AiMj%2BbimYNoiQW0SLg4hHA11PkKNn4KPAFqhLVscsCw%3D%3D=1631130462 This is all to show that the tests are (mostly) working. There's some post-processing on the pass/fail status of some tests to ignore expected failures (eg. zfs_test). Occasionally there are spurious test failures, I think due to the VM getting starved at times (eg. why grub_cmd_sleep fails and is ignored). No as far as you personally using the tests, based on the above what do we need to get that to happen? I don't know the setup that you plan on doing the tests on (is it debian based?). Do you want a script that sets up the environment for you? Perhaps a docker or lxd setup (probably better in the long run)? A word about why I haven't verified the fs tests that require kernel modules not loaded in gitlab shared runner, I could run these on my system. However, I've had some strange experiences in the past with the fs tests not cleaning up well and causing some issues with the rest of the system. So I'm not keen on running all the fs tests on my daily driver. Perhaps in a container (a virtual machine would work, but I suspect really slow, because many of the tests use virtual machines for testing, so there would be VMs in a VM). I'd be curious to know what specific issues you had with the test, if you can remember or reproduce them. > > It is my understanding that we have travis-ci tests that get run (at > > some point?), however they are only build tests and so would not > > have caught this. It was precisely this scenario that I hoped to > > avoid by doing more thorough continuous integration, which runs the > > extensive
Re: [SECURITY PATCH 05/28] malloc: Use overflow checking primitives where we do complex allocations
On Wed, 29 Jul 2020 19:00:18 +0200
Daniel Kiper wrote:
> From: Peter Jones
>
> This attempts to fix the places where we do the following where
> arithmetic_expr may include unvalidated data:
>
> X = grub_malloc(arithmetic_expr);
>
> It accomplishes this by doing the arithmetic ahead of time using grub_add(),
> grub_sub(), grub_mul() and testing for overflow before proceeding.
...
> diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
> index a83761674..21ac7f446 100644
> --- a/grub-core/fs/udf.c
> +++ b/grub-core/fs/udf.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -890,9 +891,19 @@ read_string (const grub_uint8_t *raw, grub_size_t sz,
> char *outbuf)
> utf16[i] = (raw[2 * i + 1] << 8) | raw[2*i + 2];
> }
>if (!outbuf)
> -outbuf = grub_malloc (utf16len * GRUB_MAX_UTF8_PER_UTF16 + 1);
> +{
> + grub_size_t size;
> +
> + if (grub_mul (utf16len, GRUB_MAX_UTF8_PER_UTF16, ) ||
> + grub_add (size, 1, ))
> + goto fail;
> +
> + outbuf = grub_malloc (size);
> +}
>if (outbuf)
> *grub_utf16_to_utf8 ((grub_uint8_t *) outbuf, utf16, utf16len) = '\0';
> +
> + fail:
>grub_free (utf16);
>return outbuf;
> }
> @@ -1005,7 +1016,7 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
>grub_size_t sz = U64 (node->block.fe.file_size);
>grub_uint8_t *raw;
>const grub_uint8_t *ptr;
> - char *out, *optr;
> + char *out = NULL, *optr;
>
>if (sz < 4)
> return NULL;
> @@ -1013,14 +1024,16 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
>if (!raw)
> return NULL;
>if (grub_udf_read_file (node, NULL, NULL, 0, sz, (char *) raw) < 0)
> -{
> - grub_free (raw);
> - return NULL;
> -}
> +goto fail_1;
>
> - out = grub_malloc (sz * 2 + 1);
> + if (grub_mul (sz, 2, ) ||
> + grub_add (sz, 1, ))
The old code did not change the value of sz, but the new code does.
This is a problem because sz is used further down. This is causing udf
symlinks to not be accessible via grub and is causing the udf tests to
fail on the symlink test.
> +goto fail_0;
> +
> + out = grub_malloc (sz);
>if (!out)
> {
> + fail_0:
>grub_free (raw);
>return NULL;
> }
> @@ -1031,17 +1044,17 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
> {
>grub_size_t s;
>if ((grub_size_t) (ptr - raw + 4) > sz)
> - goto fail;
> + goto fail_1;
>if (!(ptr[2] == 0 && ptr[3] == 0))
> - goto fail;
> + goto fail_1;
>s = 4 + ptr[1];
>if ((grub_size_t) (ptr - raw + s) > sz)
> - goto fail;
> + goto fail_1;
>switch (*ptr)
> {
> case 1:
> if (ptr[1])
> - goto fail;
> + goto fail_1;
> /* Fallthrough. */
> case 2:
> /* in 4 bytes. out: 1 byte. */
> @@ -1066,11 +1079,11 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
> if (optr != out)
> *optr++ = '/';
> if (!read_string (ptr + 4, s - 4, optr))
> - goto fail;
> + goto fail_1;
> optr += grub_strlen (optr);
> break;
> default:
> - goto fail;
> + goto fail_1;
> }
>ptr += s;
> }
> @@ -1078,7 +1091,7 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
>grub_free (raw);
>return out;
>
> - fail:
> + fail_1:
>grub_free (raw);
>grub_free (out);
>grub_error (GRUB_ERR_BAD_FS, "invalid symlink");
I actually noticed in my CI tests that the UDF tests were failing before
the 2.06 release, but I assumed it was something long standing and
didn't look into what was causing it. Here's another example of how my
CI and testing improvements could've made a more solid release.
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] udf: Fix regression which is preventing symlink access
This code was broken by commit 3f05d693 ("malloc: Use overflow checking
primitives where we do complex allocations"), which added overflow checking
in many areas. The problem here is that the changes update the local
variable sz, which was already in use and which was not updated before the
change. So the code using sz was getting a different value of than it would
have previously for the same UDF image. This causes the logic getting the
destination of the symlink to not realize that its gotten the full
destination, but keeps trying to read past the end of the destination. The
bytes after the end are generally NULL padding bytes, but that's not a
valid component type (ECMA-167 14.16.1.1). So grub_udf_read_symlink branches
to error logic, returning NULL, instead of the symlink destination path.
The result of this bug is that the UDF filesystem tests were failing in the
symlink test with the grub-fstest error message:
grub-fstest: error: cannot open `(loop0)/sym': invalid symlink.
This change stores the result of doubling sz in another local variable s,
so as not to modify sz. Also remove unnecessary grub_add, which increased
the output by 1 to account for a NULL byte. This isn't needed because an
output buffer of size twice sz is already guaranteed to be more than enough
to contain the path components converted to UTF-8. The worst case upper-
bound for the needed output buffer size is (sz-4)*1.5, where 4 is the size
of a path component header and 1.5 is the maximum growth in bytes when
converting from 2-byte unicode code-points to UTF-8 (from 2 bytes to 3).
Signed-off-by: Glenn Washburn
---
grub-core/fs/udf.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
index 2ac5c1d00..197e60d12 100644
--- a/grub-core/fs/udf.c
+++ b/grub-core/fs/udf.c
@@ -1022,7 +1022,7 @@ grub_udf_iterate_dir (grub_fshelp_node_t dir,
static char *
grub_udf_read_symlink (grub_fshelp_node_t node)
{
- grub_size_t sz = U64 (node->block.fe.file_size);
+ grub_size_t s, sz = U64 (node->block.fe.file_size);
grub_uint8_t *raw;
const grub_uint8_t *ptr;
char *out = NULL, *optr;
@@ -1035,11 +1035,19 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
if (grub_udf_read_file (node, NULL, NULL, 0, sz, (char *) raw) < 0)
goto fail_1;
- if (grub_mul (sz, 2, ) ||
- grub_add (sz, 1, ))
+ /*
+ * Local sz is the size of the symlink file data, which contains a sequence
+ * of path components (ECMA-167 14.16.1) representing the link destination.
+ * This size is an upper-bound on the number of bytes of a contained and
+ * potentially compressed 2-byte character string. Allocate 2*sz for the
+ * output buffer contained the string converted to UTF-8 because the
+ * resulting string can not be more than double the size (2-byte unicode
+ * points will be converted to a maximum of 3 bytes in UTF-8).
+ */
+ if (grub_mul (sz, 2, ))
goto fail_0;
- out = grub_malloc (sz);
+ out = grub_malloc (s);
if (!out)
{
fail_0:
@@ -1051,7 +1059,6 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
for (ptr = raw; ptr < raw + sz; )
{
- grub_size_t s;
if ((grub_size_t) (ptr - raw + 4) > sz)
goto fail_1;
if (!(ptr[2] == 0 && ptr[3] == 0))
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] udf: Fix regression which is preventing symlink access
On Tue, 14 Sep 2021 16:27:55 +0200
Daniel Kiper wrote:
> On Fri, Sep 10, 2021 at 04:03:23PM +0000, Glenn Washburn wrote:
> > This code was broken by commit 3f05d693 ("malloc: Use overflow
> > checking primitives where we do complex allocations"), which added
> > overflow checking in many areas. The problem here is that the
> > changes update the local variable sz, which was already in use and
> > which was not updated before the change. So the code using sz was
> > getting a different value of than it would have previously for the
> > same UDF image. This causes the logic getting the destination of
> > the symlink to not realize that its gotten the full destination,
> > but keeps trying to read past the end of the destination. The bytes
> > after the end are generally NULL padding bytes, but that's not a
> > valid component type (ECMA-167 14.16.1.1). So grub_udf_read_symlink
> > branches to error logic, returning NULL, instead of the symlink
> > destination path.
> >
> > The result of this bug is that the UDF filesystem tests were
> > failing in the symlink test with the grub-fstest error message:
> >
> > grub-fstest: error: cannot open `(loop0)/sym': invalid symlink.
> >
> > This change stores the result of doubling sz in another local
> > variable s, so as not to modify sz. Also remove unnecessary
> > grub_add, which increased the output by 1 to account for a NULL
> > byte. This isn't needed because an output buffer of size twice sz
> > is already guaranteed to be more than enough to contain the path
> > components converted to UTF-8. The worst case upper- bound for the
> > needed output buffer size is (sz-4)*1.5, where 4 is the size
>
> I think 4 comes from ECMA-167 spec. Could you add a reference to it
> here? The number of paragraph would be perfect...
Its 14.16.1 basically in the same place as the reference earlier, which
is why I didn't include it. But, yes, I can include it.
> > of a path component header and 1.5 is the maximum growth in bytes
> > when converting from 2-byte unicode code-points to UTF-8 (from 2
> > bytes to 3).
>
> Could you explain how did you come up with the 1.5 value? It would be
> nice if you refer to a spec or something like that.
There is no spec that I know of (but would be happy to know of one). Its
something I've deduced based on my understanding of Unicode, UTF-8, and
UTF-16. All unicode code points less than or equal to 2 bytes (code
points <0x1) can be represented in UTF-8 by a maximum of 3 bytes
[1]. Longer UTF-16 encodings don't matter because those will be 4 bytes
or longer. The maximum number of bytes for a UTF-8 encoding of a unicode
code point is 4 bytes. So the longer UTF-16 encodings can only be equal
to or longer than the UTF-8 encoding, thus the UTF-16 -> UTF-8 would be
shrinking or maintaining the length of the original byte string. Since
the worst case growth is 2 bytes to 3, that's 1.5 times the original
string size. QED.
Do you want all that in there? I could just remove that part about the
1.5 too.
Here's an SO question that addresses this. Yes, unofficial, but I think
adds some weight as to the correctness of the logic above.
https://stackoverflow.com/questions/55056322/maximum-utf-8-string-size-given-utf-16-size
Glenn
[1] https://en.wikipedia.org/wiki/UTF-8#Encoding
>
> Daniel
>
> > Signed-off-by: Glenn Washburn
> > ---
> > grub-core/fs/udf.c | 17 -
> > 1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
> > index 2ac5c1d00..197e60d12 100644
> > --- a/grub-core/fs/udf.c
> > +++ b/grub-core/fs/udf.c
> > @@ -1022,7 +1022,7 @@ grub_udf_iterate_dir (grub_fshelp_node_t dir,
> > static char *
> > grub_udf_read_symlink (grub_fshelp_node_t node)
> > {
> > - grub_size_t sz = U64 (node->block.fe.file_size);
> > + grub_size_t s, sz = U64 (node->block.fe.file_size);
> >grub_uint8_t *raw;
> >const grub_uint8_t *ptr;
> >char *out = NULL, *optr;
> > @@ -1035,11 +1035,19 @@ grub_udf_read_symlink (grub_fshelp_node_t
> > node) if (grub_udf_read_file (node, NULL, NULL, 0, sz, (char *)
> > raw) < 0) goto fail_1;
> >
> > - if (grub_mul (sz, 2, ) ||
> > - grub_add (sz, 1, ))
> > + /*
> > + * Local sz is the size of the symlink file data, which contains
> > a sequence
> > + * of path components (ECMA-167 14.16.1) representing the link
> > destination.
> > + * This size is an upper-bound on the number of bytes of a
> > contained and
> > + * potentially compressed 2-byte character string. Allocate 2*sz
> > for the
Re: [PATCH] udf: Fix regression which is preventing symlink access
On Wed, 15 Sep 2021 16:52:28 +0200
Daniel Kiper wrote:
> On Tue, Sep 14, 2021 at 06:19:03PM +0000, Glenn Washburn wrote:
> > On Tue, 14 Sep 2021 16:27:55 +0200
> > Daniel Kiper wrote:
> >
> > > On Fri, Sep 10, 2021 at 04:03:23PM +, Glenn Washburn wrote:
> > > > This code was broken by commit 3f05d693 ("malloc: Use overflow
> > > > checking primitives where we do complex allocations"), which
> > > > added overflow checking in many areas. The problem here is that
> > > > the changes update the local variable sz, which was already in
> > > > use and which was not updated before the change. So the code
> > > > using sz was getting a different value of than it would have
> > > > previously for the same UDF image. This causes the logic
> > > > getting the destination of the symlink to not realize that its
> > > > gotten the full destination, but keeps trying to read past the
> > > > end of the destination. The bytes after the end are generally
> > > > NULL padding bytes, but that's not a valid component type
> > > > (ECMA-167 14.16.1.1). So grub_udf_read_symlink branches to
> > > > error logic, returning NULL, instead of the symlink destination
> > > > path.
> > > >
> > > > The result of this bug is that the UDF filesystem tests were
> > > > failing in the symlink test with the grub-fstest error message:
> > > >
> > > > grub-fstest: error: cannot open `(loop0)/sym': invalid
> > > > symlink.
> > > >
> > > > This change stores the result of doubling sz in another local
> > > > variable s, so as not to modify sz. Also remove unnecessary
> > > > grub_add, which increased the output by 1 to account for a NULL
> > > > byte. This isn't needed because an output buffer of size twice
> > > > sz is already guaranteed to be more than enough to contain the
> > > > path components converted to UTF-8. The worst case upper- bound
> > > > for the needed output buffer size is (sz-4)*1.5, where 4 is the
> > > > size
> > >
> > > I think 4 comes from ECMA-167 spec. Could you add a reference to
> > > it here? The number of paragraph would be perfect...
> >
> > Its 14.16.1 basically in the same place as the reference earlier,
> > which is why I didn't include it. But, yes, I can include it.
>
> Yes, please.
Ok, will do.
> > > > of a path component header and 1.5 is the maximum growth in
> > > > bytes when converting from 2-byte unicode code-points to UTF-8
> > > > (from 2 bytes to 3).
> > >
> > > Could you explain how did you come up with the 1.5 value? It
> > > would be nice if you refer to a spec or something like that.
> >
> > There is no spec that I know of (but would be happy to know of
> > one). Its something I've deduced based on my understanding of
> > Unicode, UTF-8, and UTF-16. All unicode code points less than or
> > equal to 2 bytes (code points <0x1) can be represented in UTF-8
> > by a maximum of 3 bytes [1]. Longer UTF-16 encodings don't matter
> > because those will be 4 bytes or longer. The maximum number of
> > bytes for a UTF-8 encoding of a unicode
>
> The [1] says: Since Errata DCN-5157, the range of code points was
> expanded to all code points from Unicode 4.0 (or any newer or older
> version), which includes Plane 1-16 characters such as Emoji.
>
> So, I think your assumption about longer encodings is incorrect for
> the UDF.
No, I don't believe so. The "assumption", which was actually a logical
conclusion, that I understand you to be saying is incorrect is "Longer
UTF-16 encodings don't matter because those will be 4 bytes or longer".
This was perhaps a little confusing as there are no UTF-16 encoded
codepoints longer than 4 bytes. Be that as it may, I go on to explain
that they do not matter because those UTF-16 bytes strings will never
occupy more bytes when encoded in UTF-8. The question of debate is "how
much can a UTF-16 byte string grow when re-encoded as UTF-8?". Thus
codepoints that can not grow (most in fact strink!) can be disregarded
in the analysis. You talk about unicode Planes 1-16 as relevant, but
remember those Planes are the ones where the UTF-16 is 4 bytes, and
thus the set of code words we just disregarded as not relevant.
Perhaps you do not believe that all codepoints outside of Plane 0
require no more bytes in UTF-16 than in UTF-8. If so, please provide one
counter example, that is one codepoint satisfying the condition.
Likewise, if you believe that there exists a 2-byte co
Where is the testing? (was: Re: [PATCH] fs/xfs: Avoid unreadble filesystem if V4 superblock)
On Thu, 2 Sep 2021 10:56:49 +0200 Carlos Maiolino wrote: > On Wed, Sep 01, 2021 at 02:40:57PM +0200, Daniel Kiper wrote: > > CC-ing Javier... > > > > On Mon, Aug 30, 2021 at 01:48:50PM +0200, Carlos Maiolino wrote: > > > Hi. > > > On Mon, Aug 30, 2021 at 11:18:31AM +0200, Erwan Velu wrote: > > > >Good day list, > > > >Le jeu. 26 août 2021 à 15:26, Carlos Maiolino > > > > <[1]cmaiol...@redhat.com> a écrit : > > > > > > > > [..] > > > > Thanks for spotting this! > > > > > > > >I'm adding the maintainers in CC. Carlos who commit the > > > > patch I'm fixing, agreed on the content. > > > > > > I didn't test the patch itself yet, but I've reproduced the > > > issue. I was quite sure I had tested this patch on a V4 fs, but > > > looks like I miscalculated the sizing. Thanks again. I'll try to > > > test the patch here asap. > > > > Did you test this patch? If yes may I add your Tested-by to it? > > Yup, patch works fine, just finished testing it, I was just trying to > understand where/why I miscalculated the inode size on V4 > filesystems, and the reason was the same why Erwan split the > last/first members of inode v2/v3 in two different unused structs. > > Feel free to add to the patch: > > Tested-by: Carlos Maiolino It looks like the xfs_test test succeeds with tag grub-2.06-rc1a, fails with tag grub-2.06, and succeeds with current master. Yes, as expected. However, what this tells me is that no "make check" tests were done before finalizing the 2.06 release. I was under the impression that that was part of the release procedure. If its not, it would seem that we're not using the tests at a time when they would have the most impact. It is my understanding that we have travis-ci tests that get run (at some point?), however they are only build tests and so would not have caught this. It was precisely this scenario that I hoped to avoid by doing more thorough continuous integration, which runs the extensive array of "make check" tests, when I submitted the Gitlab-CI patch series (which would've caught this automatically if it had been merged). To me this scenario is the poster child for taking action on this front. Can we make this a priority? Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] tests: Let xorriso fixely assume UTF-8 as local character set
On Wed, 1 Sep 2021 15:11:38 +0200 Daniel Kiper wrote: > On Fri, Aug 27, 2021 at 11:05:06PM +0200, Thomas Schmitt wrote: > > iso9660_test fails if the effective locale is not UTF-8. This > > happens because xorriso needs to convert file names and FSLABEL to > > UCS-2 when preparing a Joliet tree. grub-fs-tester obviously > > intends to use UTF-8 as character set, but xorriso assumes by > > default the result of nl_langinfo(3) with item CODESET. > > So override the result of nl_langinfo(CODESET) by options of > > xorriso -as mkisofs. > > > > Signed-off-by: Thomas Schmitt > > Thomas, Glenn, thank you for doing such deep investigation! > > Reviewed-by: Daniel Kiper > > Glenn, did you test this patch? Coming to this late, had some issues which kept me away. I notice that the patch is already in master, but for what its worth, I have now tested with the patch and LANG unset and the test now succeeds. Thanks Thomas. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 3/3] cryptodisk: Move global variables into grub_cryptomount_args struct
On Mon, 30 Aug 2021 20:02:26 +0200
Patrick Steinhardt wrote:
> On Thu, Aug 26, 2021 at 12:08:52AM -0500, Glenn Washburn wrote:
> > Signed-off-by: Glenn Washburn
> > ---
> > grub-core/disk/cryptodisk.c | 26 +-
> > include/grub/cryptodisk.h | 3 +++
> > 2 files changed, 12 insertions(+), 17 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c
> > b/grub-core/disk/cryptodisk.c index b6cf1835d..00a671a59 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -984,9 +984,6 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
> >
> > #endif
> >
> > -static int check_boot, have_it;
> > -static char *search_uuid;
> > -
> > static void
> > cryptodisk_close (grub_cryptodisk_t dev)
> > {
> > @@ -1014,7 +1011,7 @@ grub_cryptodisk_scan_device_real (const char
> > *name,
> >FOR_CRYPTODISK_DEVS (cr)
> >{
> > -dev = cr->scan (source, search_uuid, check_boot);
> > +dev = cr->scan (source, cargs->search_uuid, cargs->check_boot);
> > if (grub_errno)
> >return grub_errno;
> > if (!dev)
> > @@ -1051,7 +1048,7 @@ grub_cryptodisk_scan_device_real (const char
> > *name,
> > grub_cryptodisk_insert (dev, name, source);
> >
> > -have_it = 1;
> > +cargs->found_uuid = 1;
> >
> > goto cleanup;
> >}
> > @@ -1093,7 +1090,7 @@ grub_cryptodisk_cheat_mount (const char
> > *sourcedev, const char *cheat)
> >FOR_CRYPTODISK_DEVS (cr)
> >{
> > -dev = cr->scan (source, search_uuid, check_boot);
> > +dev = cr->scan (source, NULL, 0);
> > if (grub_errno)
> >return grub_errno;
> > if (!dev)
> > @@ -1137,7 +1134,7 @@ grub_cryptodisk_scan_device (const char *name,
> >
> >if (err)
> > grub_print_error ();
> > - return have_it && search_uuid ? 1 : 0;
> > + return (cargs->found_uuid && cargs->search_uuid) ? 1 : 0;
> > }
> >
> > static grub_err_t
> > @@ -1155,7 +1152,6 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args) cargs.key_len =
> > grub_strlen(state[3].arg); }
> >
> > - have_it = 0;
> >if (state[0].set) /* uuid */
> > {
> >grub_cryptodisk_t dev;
> > @@ -1168,21 +1164,18 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args) return GRUB_ERR_NONE;
> > }
> >
> > - check_boot = state[2].set;
> > - search_uuid = args[0];
> > + cargs.check_boot = state[2].set;
> > + cargs.search_uuid = args[0];
> >grub_device_iterate (_cryptodisk_scan_device, );
> > - search_uuid = NULL;
> >
> > - if (!have_it)
> > + if (!cargs.found_uuid)
> > return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > cryptodisk found"); return GRUB_ERR_NONE;
> > }
> >else if (state[1].set || (argc == 0 && state[2].set)) /* -a|-b */
> > {
> > - search_uuid = NULL;
> > - check_boot = state[2].set;
> > + cargs.check_boot = state[2].set;
> >grub_device_iterate (_cryptodisk_scan_device, );
> > - search_uuid = NULL;
> >return GRUB_ERR_NONE;
> > }
> >else
> > @@ -1194,8 +1187,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args) char *disklast = NULL;
> >grub_size_t len;
> >
> > - search_uuid = NULL;
> > - check_boot = state[2].set;
> > + cargs.check_boot = state[2].set;
> >diskname = args[0];
> >len = grub_strlen (diskname);
> >if (len && diskname[0] == '(' && diskname[len - 1] == ')')
> > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> > index 1070140d9..11062f43a 100644
> > --- a/include/grub/cryptodisk.h
> > +++ b/include/grub/cryptodisk.h
> > @@ -69,6 +69,9 @@ typedef gcry_err_code_t
> >
> > struct grub_cryptomount_args
> > {
> > + grub_uint32_t check_boot : 1;
> > + grub_uint32_t found_uuid : 1;
> > + char *search_uuid;
> >grub_uint8_t *key_data;
> >grub_size_t key_len;
> > };
>
> Aren't these parameters in a different scope than the key data? These
> are only used for device discovery via `scan()`, while the other ones
> are for decrypting the key. Do we want to split those up into two
> different structs?
This struct is meant to be used for any data passed to the crypto
backend from cryptomount. All of those members are affected by
cryptomount options. So this struct isn't about anything in particular,
just a common set of data passed to the crypto backends via
cryptomount. So I don't think two structs would improve anything here.
Am I missing something?
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/3] cryptodisk: Add infrastructure to pass data from cryptomount to cryptodisk modules
On Mon, 30 Aug 2021 19:55:59 +0200
Patrick Steinhardt wrote:
> On Thu, Aug 26, 2021 at 12:08:50AM -0500, Glenn Washburn wrote:
> > As an example, passing a password as a cryptomount argument is
> > implemented. However, the backends are not implemented, so testing
> > this will return a not implemented error.
> >
> > Signed-off-by: Glenn Washburn
> > ---
> > grub-core/disk/cryptodisk.c | 31 ++-
> > grub-core/disk/geli.c | 4
> > grub-core/disk/luks.c | 4
> > grub-core/disk/luks2.c | 4
> > include/grub/cryptodisk.h | 8
> > 5 files changed, 42 insertions(+), 9 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c
> > b/grub-core/disk/cryptodisk.c index 90f82b2d3..b966b19ab 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -41,6 +41,7 @@ static const struct grub_arg_option options[] =
> > /* TRANSLATORS: It's still restricted to cryptodisks only. */
> > {"all", 'a', 0, N_("Mount all."), 0, 0},
> > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag
> > set."), 0, 0},
> > +{"password", 'p', 0, N_("Password to open volumes."), 0,
> > ARG_TYPE_STRING}, {0, 0, 0, 0, 0, 0}
> >};
> >
> > @@ -996,7 +997,9 @@ cryptodisk_close (grub_cryptodisk_t dev)
> > }
> >
> > static grub_err_t
> > -grub_cryptodisk_scan_device_real (const char *name, grub_disk_t
> > source) +grub_cryptodisk_scan_device_real (const char *name,
> > + grub_disk_t source,
> > + grub_cryptomount_args_t cargs)
> > {
> >grub_err_t err;
> >grub_cryptodisk_t dev;
> > @@ -1015,7 +1018,9 @@ grub_cryptodisk_scan_device_real (const char
> > *name, grub_disk_t source) if (!dev)
> >continue;
> >
> > +*dev->cargs = *cargs;
> > err = cr->recover_key (source, dev);
> > +*dev->cargs = NULL;
> > if (err)
> > {
> >cryptodisk_close (dev);
>
> Is there any specific reason why you choose to set `cargs` as a struct
> field? Seeing uses like this makes me wonder whether it wouldn't be
> better to pass in `cargs` as explicit arguments whenever required.
> This would also automatically answer any lifetime questions which
> arise.
I'm not opposed to this. One of my original goals was to try and not
change the function interfaces between cryptomount and the backends. I
also was originally going to have the dev->cargs stick around for the
lifetime of the dev, but I'm not remembering a good use case for that.
I'll send another series with cargs being passed as an argument.
> [snip]
> > diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> > index 13103ea6a..e2a4a3bf5 100644
> > --- a/grub-core/disk/luks.c
> > +++ b/grub-core/disk/luks.c
> > @@ -165,6 +165,10 @@ luks_recover_key (grub_disk_t source,
> >grub_size_t max_stripes = 1;
> >char *tmp;
> >
> > + /* Keyfiles are not implemented yet */
> > + if (dev->cargs->key_data || dev->cargs->key_len)
> > + return GRUB_ERR_NOT_IMPLEMENTED_YET;
> > +
> >err = grub_disk_read (source, 0, 0, sizeof (header), );
> >if (err)
> > return err;
>
> Hum. This makes me wonder whether we'll have to adjust all backends
> whenever we add a new parameter to `cargs` to return
> `NOT_IMPLEMENTED`. I fear that it won't scale nicely, and that it is
> a recipe for passing unsupported arguments to backends even though
> they don't know to handle them.
>
> Not sure about a nice alternative though. The only idea I have right
> now is something like a capabilities bitfield exposed by backends:
> only if a specific bit is set will we pass the corresponding field to
> such a backend. This would allow us to easily centralize the logic
> into a single function which only receives both the capabilities
> bitset and the `cargs` struct.
>
> Other than that I really like where this is going.
I see your concern, and it would be nice to have an elegant solution to
it. The capability bitfield idea seems workable. I don't think this
needs to be solved right now though. This is a problem with all
proposed approaches. I think that this *could* lead to scalability
issues, but that's likely way down the road (considering the rate at
which we're adding args to cryptomount). Also, I don't think this patch
series hampers any such solution. So I think we can punt on this for
now. Does that sound reasonable?
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 02/10] tests: mkreiserfs only supports 4096 block size
Signed-off-by: Glenn Washburn --- tests/util/grub-fs-tester.in | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in index 9f6f26f26..ab58a96c8 100644 --- a/tests/util/grub-fs-tester.in +++ b/tests/util/grub-fs-tester.in @@ -126,8 +126,10 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do MAXBLKSIZE=4096 ;; xreiserfs*) - MINBLKSIZE=512 - # OS LIMITATION: 8192 isn't supported. + # OS LIMITATION: mkreiserfs says block sizes smaller than 4k are + # not supported and bails. Its been this way since at least 2014. + MINBLKSIZE=4096 + # OS LIMITATION: 8192 isn't supported. MAXBLKSIZE=4096 ;; x"mdraid"*) -- 2.32.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 00/10] More test fixes/improvements
Here is another patch series of changes needed to get the make check tests passing on fairly recent Ubuntu/Debian systems, though most of the fixes are for issues on fairly old systems as well (eg. reiser fixes). The FAT volume label change is only needed on fairly recent systems because up until very recently a fairly old version of dosfstools has been used which didn't do invalid character checking. Glenn Glenn Washburn (10): tests: Rename variable filtime -> filetime as its meant to be tests: mkreiserfs only supports 4096 block size tests: Disable reiserfs tests for old format because newer kernels do not support them tests: mkfs.btrfs now supports only 4k sector sizes and above tests: Only test minix3 volumes of 1k block size tests: Change FAT volume label to be with in the valid character range tests: Skip hfs test only when mac_roman module is not loaded and not loadable grub-shell: Boot PowerPC using PMU instead of CUDA for power management tests: Output list of devices when partmap fails tests: Do not delete filesystem images on error tests/hfs_test.in| 2 +- tests/minixfs_test.in| 5 - tests/partmap_test.in| 2 +- tests/reiserfs_test.in | 4 +++- tests/util/grub-fs-tester.in | 28 ++-- tests/util/grub-shell.in | 1 + 6 files changed, 20 insertions(+), 22 deletions(-) -- 2.32.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 07/10] tests: Skip hfs test only when mac_roman module is not loaded and not loadable
Allow the hfs tests to not be skipped if the mac_roman modules is loaded in the kernel, but not accessible to modprobe. Signed-off-by: Glenn Washburn --- tests/hfs_test.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/hfs_test.in b/tests/hfs_test.in index d7ec56bef..5b0c5e33e 100644 --- a/tests/hfs_test.in +++ b/tests/hfs_test.in @@ -15,7 +15,7 @@ if ! which mkfs.hfs >/dev/null 2>&1; then exit 77 fi -if ! modprobe mac-roman; then +if ! grep -q mac_roman /proc/modules && ! modprobe mac_roman; then echo "no mac-roman support; cannot test HFS." exit 77 fi -- 2.32.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 10/10] tests: Do not delete filesystem images on error
The filesystem images created for the filesystem test can be useful when
debugging why a filesystem test failed. So keep them around and let the user
clean them up.
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 3 ---
1 file changed, 3 deletions(-)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 537cdd48b..62f814dc4 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -932,9 +932,6 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
sleep 1
done
done
- for i in $(range 0 $((NDEVICES-1)) 1); do
- rm "$FSIMAGEP${i}.img"
- done
exit 99;
fi
;;
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 04/10] tests: mkfs.btrfs now supports only 4k sector sizes and above
Signed-off-by: Glenn Washburn --- tests/util/grub-fs-tester.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in index ab58a96c8..f4d9f161b 100644 --- a/tests/util/grub-fs-tester.in +++ b/tests/util/grub-fs-tester.in @@ -59,7 +59,7 @@ case x"$fs" in MINLOGSECSIZE=8 MAXLOGSECSIZE=12;; xbtrfs*) - MINLOGSECSIZE=8 + MINLOGSECSIZE=12 # OS LIMITATION: It could go up to 32768 but Linux rejects sector sizes > 4096 MAXLOGSECSIZE=12;; xxfs) -- 2.32.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 09/10] tests: Output list of devices when partmap fails
Signed-off-by: Glenn Washburn
---
tests/partmap_test.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index faace51ec..3d9871a2f 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -43,7 +43,7 @@ check_output () {
for dsk in $@; do
if ! grep "($dsk)" "${outfile}" >/dev/null
then
- echo "($dsk): disk/partiton not found"
+ echo "($dsk): disk/partiton not found in: $(cat "${outfile}")"
exit 1
fi
done
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 05/10] tests: Only test minix3 volumes of 1k block size
Apparently there used to be a -B option for mkfs.minix to create a volume
with a specified block size. This version is hard to come by and does not
appear to be available in debian distributions. So remove support for
testing a variety of blocks sizes for minix3. This allows the minix tests
to run because they were being skipped due to not finding a mkfs.minix with
the -B option.
Signed-off-by: Glenn Washburn
---
tests/minixfs_test.in| 5 -
tests/util/grub-fs-tester.in | 4 ++--
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/tests/minixfs_test.in b/tests/minixfs_test.in
index 3b16a4de0..437d92df6 100644
--- a/tests/minixfs_test.in
+++ b/tests/minixfs_test.in
@@ -25,11 +25,6 @@ if ! mkfs.minix -h | grep -- -3 > /dev/null; then
exit 77
fi
-if ! mkfs.minix -h | grep -- -B > /dev/null; then
-echo "mkfs.minix doesn't support variable block size; cannot test
minix*fs."
- exit 77
-fi
-
"@builddir@/grub-fs-tester" minix
"@builddir@/grub-fs-tester" minix2
"@builddir@/grub-fs-tester" minix3
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index f4d9f161b..96b204590 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -201,7 +201,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
# OS LIMITATION: Linux rejects non-power-of-two blocks.
# OS LIMITATION: Linux rejects > 4096.
MINBLKSIZE=1024
- MAXBLKSIZE=4096;;
+ MAXBLKSIZE=1024;;
esac
if test "$BLKSTEP" -eq 0; then
blksizes="$(powrange "$MINBLKSIZE" "$MAXBLKSIZE")"
@@ -697,7 +697,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE"
1); do
"mkfs.minix" -v "${MOUNTDEVICE}"
MOUNTFS="minix";;
x"minix3")
- "mkfs.minix" -B $BLKSIZE -3 "${MOUNTDEVICE}"
+ "mkfs.minix" -3 "${MOUNTDEVICE}"
MOUNTFS="minix";;
x"ntfs"*)
"mkfs.ntfs" -s "$SECSIZE" -c "$BLKSIZE" -L "$FSLABEL" -Q -q
"${MOUNTDEVICE}"
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 08/10] grub-shell: Boot PowerPC using PMU instead of CUDA for power management
At some point it looks like the defualt machine for qemu-system-ppc started
using CUDA as a backend for power management. This causes the machine to
throw an exception and not actually power down the VM[1]. Switching to PMU
allows power downs and reboots to work as expceted.
[1] https://gitlab.com/qemu-project/qemu/-/issues/624
Signed-off-by: Glenn Washburn
---
tests/util/grub-shell.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/util/grub-shell.in b/tests/util/grub-shell.in
index 93e9f5148..5354d8678 100644
--- a/tests/util/grub-shell.in
+++ b/tests/util/grub-shell.in
@@ -84,6 +84,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
serial_null="-serial null"
netbootext=elf
trim=1
+ qemuopts="-M mac99,via=pmu $qemuopts"
;;
sparc64-ieee1275)
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 7/8] tests: Exit with skipped exit code when test not performed
On Wed, 25 Aug 2021 02:04:01 -0500 Glenn Washburn wrote: > These tests were not performed and therefore did not pass, nor fail. > This fixes misleading test exit code where, for instance, the > pseries_test will pass on i386-pc, which is not a pseries > architecture. > > Signed-off-by: Glenn Washburn > --- > tests/ahci_test.in | 8 > tests/cdboot_test.in| 8 > tests/core_compress_test.in | 2 +- > tests/ehci_test.in | 8 > tests/fddboot_test.in | 16 > tests/grub_cmd_date.in | 2 +- > tests/grub_cmd_set_date.in | 6 +++--- > tests/hddboot_test.in | 6 +++--- > tests/netboot_test.in | 12 ++-- > tests/ohci_test.in | 8 > tests/partmap_test.in | 8 > tests/pata_test.in | 6 +++--- > tests/pseries_test.in | 2 +- > tests/uhci_test.in | 8 > 14 files changed, 50 insertions(+), 50 deletions(-) I missed that the test for grub_cmd_sleep.in on sparc64-ieee1275 should be skipped as well. Its a trivial change and will add it on the next revision of this series after review. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 03/10] tests: Disable reiserfs tests for old format because newer kernels do not support them
Signed-off-by: Glenn Washburn --- tests/reiserfs_test.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/reiserfs_test.in b/tests/reiserfs_test.in index b5fed7635..ff703b203 100644 --- a/tests/reiserfs_test.in +++ b/tests/reiserfs_test.in @@ -16,5 +16,7 @@ if ! which mkfs.reiserfs >/dev/null 2>&1; then fi "@builddir@/grub-fs-tester" reiserfs -"@builddir@/grub-fs-tester" reiserfs_old + +# Kernels since at least 4.15 can not mount reiser filesystems of the old format +#"@builddir@/grub-fs-tester" reiserfs_old -- 2.32.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 01/10] tests: Rename variable filtime -> filetime as its meant to be
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 7c22cf882..9f6f26f26 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -1170,8 +1170,8 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
fi
if [ x$NOFILETIME != xy ]; then
- filtime=$(TZ=UTC ls --time-style=+%Y%m%d%H%M%S -l -d
"$MNTPOINTRO/$OSDIR/$BASEFILE"|awk '{print $6; }')
- if echo "$LSROUT" | grep -F "$filtime $BASEFILE" > /dev/null;
then
+ filetime=$(TZ=UTC ls --time-style=+%Y%m%d%H%M%S -l -d
"$MNTPOINTRO/$OSDIR/$BASEFILE"|awk '{print $6; }')
+ if echo "$LSROUT" | grep -F "$filetime $BASEFILE" > /dev/null;
then
:
else
echo TIME FAIL
@@ -1180,8 +1180,8 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
exit 1
fi
- filtime=$(TZ=UTC ls --time-style=+%Y%m%d%H%M%S -l -d
"$MNTPOINTRO/$OSDIR/$LONGNAME"|awk '{print $6; }')
- if echo "$LSROUT" | grep -F "$filtime $LONGNAME" > /dev/null;
then
+ filetime=$(TZ=UTC ls --time-style=+%Y%m%d%H%M%S -l -d
"$MNTPOINTRO/$OSDIR/$LONGNAME"|awk '{print $6; }')
+ if echo "$LSROUT" | grep -F "$filetime $LONGNAME" > /dev/null;
then
:
else
echo LONG TIME FAIL
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 06/10] tests: Change FAT volume label to be with in the valid character range
The ';', semi-colon, character is not a valid character for a FAT filesystem
label. This test used to succeed because prior to v4.2 of dosfstools
mkfs.vfat did not enforce the character restrictions for volume labels. So
change the volume label string to be valid but contain symbol characters to
test odd volume labels.
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 96b204590..537cdd48b 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -343,9 +343,10 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
# FS LIMITATION: XFS label is at most 12 UTF-8 characters
x"xfs"|x"xfs_crc")
FSLABEL="géт к";;
-# FS LIMITATION: FAT labels limited to 11 characters, no
international characters or lowercase
+# FS LIMITATION: FAT labels limited to 11 characters, no
international characters or lowercase
+ # and excluding the restricted characters in "*?.,;:/\|+=<>[]"
x"vfat"* | xmsdos*)
- FSLABEL="GRUBTEST ;_";;
+ FSLABEL="G~!@#\$%^&(_";;
# FS LIMITATION: AFFS is latin1. At most 29 characters
x"affs" | xaffs_intl)
FSLABEL="grub_tt? #*w;/e£@¡¤½¾{[]}<>.,";;
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] docs: Improve search documentation, by adding short options and section on hints
Signed-off-by: Glenn Washburn
---
docs/grub.texi | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/docs/grub.texi b/docs/grub.texi
index 0be500387..8baf99ad1 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4999,16 +4999,25 @@ Search devices by file (@option{-f}, @option{--file}),
filesystem label
(@option{-l}, @option{--label}), or filesystem UUID (@option{-u},
@option{--fs-uuid}).
-If the @option{--set} option is used, the first device found is set as the
-value of environment variable @var{var}. The default variable is
+If the (@option{-s}, @option{--set}) option is used, the first device found is
+set as the value of environment variable @var{var}. The default variable is
@samp{root}.
-The @option{--no-floppy} option prevents searching floppy devices, which can
-be slow.
+The (@option{-n}, @option{--no-floppy}) option prevents searching floppy
+devices, which can be slow.
The @samp{search.file}, @samp{search.fs_label}, and @samp{search.fs_uuid}
commands are aliases for @samp{search --file}, @samp{search --label}, and
@samp{search --fs-uuid} respectively.
+
+Also hints as to which device may be the most likely to contain the item
+searched for may be given via the (@option{-h}, @option{--hint}) option with a
+device name as an argument. If the argument ends with a comma, then partitions
+on the device are also searched. Furthermore, platform specific hints may be
+given via the options @option{--hint-ieee1275}, @option{--hint-bios},
+@option{--hint-baremetal}, @option{--hint-efi}, and @option{--hint-arc}. When
+specified, these options take an argument and operate like @option{--hint}, but
+only on the specified platform.
@end deffn
--
2.32.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2] udf: Fix regression which is preventing symlink access
This code was broken by commit 3f05d693 ("malloc: Use overflow checking
primitives where we do complex allocations"), which added overflow checking
in many areas. The problem here is that the changes update the local
variable sz, which was already in use and which was not updated before the
change. So the code using sz was getting a different value of than it would
have previously for the same UDF image. This causes the logic getting the
destination of the symlink to not realize that its gotten the full
destination, but keeps trying to read past the end of the destination. The
bytes after the end are generally NULL padding bytes, but that's not a
valid component type (ECMA-167 14.16.1.1). So grub_udf_read_symlink branches
to error logic, returning NULL, instead of the symlink destination path.
The result of this bug is that the UDF filesystem tests were failing in the
symlink test with the grub-fstest error message:
grub-fstest: error: cannot open `(loop0)/sym': invalid symlink.
This change stores the result of doubling sz in another local variable s,
so as not to modify sz. Also remove unnecessary grub_add, which increased
the output by 1, persumably to account for a NULL byte. This isn't needed
because an output buffer of size twice sz is already guaranteed to be more
than enough to contain the path components converted to UTF-8. The value of
sz contains at least 4 bytes for the path component header (ECMA-167
14.16.1), which means that 2*4 bytes are allocated but will not be used for
UTF-8 characters, so the NULL byte is accounted for.
Signed-off-by: Glenn Washburn
---
grub-core/fs/udf.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
index 2ac5c1d00..4ff97021c 100644
--- a/grub-core/fs/udf.c
+++ b/grub-core/fs/udf.c
@@ -1022,7 +1022,7 @@ grub_udf_iterate_dir (grub_fshelp_node_t dir,
static char *
grub_udf_read_symlink (grub_fshelp_node_t node)
{
- grub_size_t sz = U64 (node->block.fe.file_size);
+ grub_size_t s, sz = U64 (node->block.fe.file_size);
grub_uint8_t *raw;
const grub_uint8_t *ptr;
char *out = NULL, *optr;
@@ -1035,11 +1035,19 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
if (grub_udf_read_file (node, NULL, NULL, 0, sz, (char *) raw) < 0)
goto fail_1;
- if (grub_mul (sz, 2, ) ||
- grub_add (sz, 1, ))
+ /*
+ * Local sz is the size of the symlink file data, which contains a sequence
+ * of path components (ECMA-167 14.16.1) representing the link destination.
+ * This size is an upper-bound on the number of bytes of a contained and
+ * potentially compressed UTF-16 character string. Allocate 2*sz for the
+ * output buffer containing the string converted to UTF-8 because the
+ * resulting string can not be more than double the size (2-byte unicode
+ * code points will be converted to a maximum of 3 bytes in UTF-8).
+ */
+ if (grub_mul (sz, 2, ))
goto fail_0;
- out = grub_malloc (sz);
+ out = grub_malloc (s);
if (!out)
{
fail_0:
@@ -1051,7 +1059,6 @@ grub_udf_read_symlink (grub_fshelp_node_t node)
for (ptr = raw; ptr < raw + sz; )
{
- grub_size_t s;
if ((grub_size_t) (ptr - raw + 4) > sz)
goto fail_1;
if (!(ptr[2] == 0 && ptr[3] == 0))
--
2.25.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: IS: Glenn's patches status WAS: Re: [PATCH 00/12] Grub-shell improvements
On Tue, 14 Sep 2021 13:04:31 +0200 Daniel Kiper wrote: > On Mon, Aug 30, 2021 at 05:23:44PM +0200, Daniel Kiper wrote: > > CC-ing Denis and Patrick... > > > > On Thu, Aug 26, 2021 at 05:08:21PM -0500, Glenn Washburn wrote: > > > Hi Daniel, > > > > > > On Thu, 26 Aug 2021 20:00:32 +0200 > > > Daniel Kiper wrote: > > > > > > > Hi Glenn, > > > > > > > > On Wed, Aug 25, 2021 at 06:06:30PM -0500, Glenn Washburn wrote: > > > > > Hi Daniel, > > [...] > > > > As far as I can tell, this is the full list. > > > > Great! > > It looks I missed at least this one: > - [PATCH 00/17] Fixes and improvements for cryptodisks+luks2 and a > few other things. > https://lists.gnu.org/archive/html/grub-devel/2020-07/msg00050.html > > Do you still care about it? No, these can be skipped. The changes in that patch series that aren't integrated, are in other patch series or I'll send to the list in the future. > > > My order preference is as follows: > > > > > > These two patches are only first because it should be a quick > > > review. > > > - [PATCH] command: Add silent mode to read command to suppress > > > input echo > > > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00291.html > > > - [PATCH] fs: Allow number of blocks in block list to be > > > optional, defaulting length to device length > > > https://lists.gnu.org/archive/html/grub-devel/2021-03/msg00286.html > > > > OK. > > I merged them yesterday. Thanks! > > > This is a high priority because I think it should be merged > > > before the keyfile and detached header support patch series. If > > > this is merged, I'll submit and updated keyfile and detached > > > header patch series that works with this patch series. > > > - [PATCH 0/3] Refactor/improve cryptomount data passing to > > > crypto modules > > > https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00129.html > > > > Denis, Patrick, are you both OK with this? > > I can see a discussion about this patch set. Should I wait for a new > version for review? Yes, there will be a new version of the patch series coming. I'm waiting to tie up the last concern of Patrick's. > > > Based on discussion with Thomas Schmitt, I think the first patch > > > of this series should be dropped. Should I make a v3, or would > > > you review it and if acceptable drop the first patch? > > > - [PATCH v2 0/8] Various fixes/improvements for tests > > > https://lists.gnu.org/archive/html/grub-devel/2021-08/msg00110.html > > Next on my list for review... > > Daniel Great! I also have some more testing fixes / changes coming in this vein. Glenn ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 3/3] cryptodisk: Move global variables into grub_cryptomount_args struct
On Sun, 12 Sep 2021 13:17:29 +0200
Patrick Steinhardt wrote:
> On Tue, Sep 07, 2021 at 02:34:30AM +0000, Glenn Washburn wrote:
> > On Mon, 30 Aug 2021 20:02:26 +0200
> > Patrick Steinhardt wrote:
> >
> > > On Thu, Aug 26, 2021 at 12:08:52AM -0500, Glenn Washburn wrote:
> > > > Signed-off-by: Glenn Washburn
> > > > ---
> > > > grub-core/disk/cryptodisk.c | 26 +-
> > > > include/grub/cryptodisk.h | 3 +++
> > > > 2 files changed, 12 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/grub-core/disk/cryptodisk.c
> > > > b/grub-core/disk/cryptodisk.c index b6cf1835d..00a671a59 100644
> > > > --- a/grub-core/disk/cryptodisk.c
> > > > +++ b/grub-core/disk/cryptodisk.c
> > > > @@ -984,9 +984,6 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
> > > >
> > > > #endif
> > > >
> > > > -static int check_boot, have_it;
> > > > -static char *search_uuid;
> > > > -
> > > > static void
> > > > cryptodisk_close (grub_cryptodisk_t dev)
> > > > {
> > > > @@ -1014,7 +1011,7 @@ grub_cryptodisk_scan_device_real (const char
> > > > *name,
> > > >FOR_CRYPTODISK_DEVS (cr)
> > > >{
> > > > -dev = cr->scan (source, search_uuid, check_boot);
> > > > +dev = cr->scan (source, cargs->search_uuid, cargs->check_boot);
> > > > if (grub_errno)
> > > >return grub_errno;
> > > > if (!dev)
> > > > @@ -1051,7 +1048,7 @@ grub_cryptodisk_scan_device_real (const char
> > > > *name,
> > > > grub_cryptodisk_insert (dev, name, source);
> > > >
> > > > -have_it = 1;
> > > > +cargs->found_uuid = 1;
> > > >
> > > > goto cleanup;
> > > >}
> > > > @@ -1093,7 +1090,7 @@ grub_cryptodisk_cheat_mount (const char
> > > > *sourcedev, const char *cheat)
> > > >FOR_CRYPTODISK_DEVS (cr)
> > > >{
> > > > -dev = cr->scan (source, search_uuid, check_boot);
> > > > +dev = cr->scan (source, NULL, 0);
> > > > if (grub_errno)
> > > >return grub_errno;
> > > > if (!dev)
> > > > @@ -1137,7 +1134,7 @@ grub_cryptodisk_scan_device (const char *name,
> > > >
> > > >if (err)
> > > > grub_print_error ();
> > > > - return have_it && search_uuid ? 1 : 0;
> > > > + return (cargs->found_uuid && cargs->search_uuid) ? 1 : 0;
> > > > }
> > > >
> > > > static grub_err_t
> > > > @@ -1155,7 +1152,6 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > > > ctxt, int argc, char **args) cargs.key_len =
> > > > grub_strlen(state[3].arg); }
> > > >
> > > > - have_it = 0;
> > > >if (state[0].set) /* uuid */
> > > > {
> > > >grub_cryptodisk_t dev;
> > > > @@ -1168,21 +1164,18 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > > > ctxt, int argc, char **args) return GRUB_ERR_NONE;
> > > > }
> > > >
> > > > - check_boot = state[2].set;
> > > > - search_uuid = args[0];
> > > > + cargs.check_boot = state[2].set;
> > > > + cargs.search_uuid = args[0];
> > > >grub_device_iterate (_cryptodisk_scan_device, );
> > > > - search_uuid = NULL;
> > > >
> > > > - if (!have_it)
> > > > + if (!cargs.found_uuid)
> > > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > > > cryptodisk found"); return GRUB_ERR_NONE;
> > > > }
> > > >else if (state[1].set || (argc == 0 && state[2].set)) /* -a|-b */
> > > > {
> > > > - search_uuid = NULL;
> > > > - check_boot = state[2].set;
> > > > + cargs.check_boot = state[2].set;
> > > >grub_device_iterate (_cryptodisk_scan_device, );
> > > > - search_uuid = NULL;
> > > >return GRUB_ERR_NONE;
> > > > }
> > > >else
> > > > @@ -1194,8 +1187,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > > > ctxt, int argc, char **args) char *disklast = NULL;
> > > >grub_size_t len;
>
[PATCH v4 2/9] tests: When checking squashfs fstime, use superblock last modified time
Currently, the filesystem timestamp check in grub-fs-tester uses the
squashfs image file's last modified timestamp and checks to see if that
time stamp is within 3 seconds of the superblock timestamp as determined by
grub. The image file's timestamp could be more than 3 seconds off if
mksquashfs takes more than 3 seconds to generate the image, as is the case
on a virtual machine. Instead use squashfs tools to get the filesystem
timestamp directly.
Signed-off-by: Glenn Washburn
---
tests/util/grub-fs-tester.in | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index a28e07295..cba94233d 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -1356,6 +1356,12 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE"
"$MAXLOGSECSIZE" 1); do
# With some abstractions like mdraid flushing to disk
# may be delayed for a long time.
FSTIME="$UMOUNT_TIME";;
+ xsquash*)
+ # Creating the squash image may take more than a few
+ # seconds. Use the more accurate timestamp from the
+ # superblock.
+ FSTIME="$(unsquashfs -s "${FSIMAGEP}0.img" | grep
^Creation | awk '{print $6 " " $7 " " $8 " " $9 " " $10; }')"
+ FSTIME="$(date -d "$FSTIME" -u '+%Y-%m-%d %H:%M:%S')";;
*)
FSTIME="$(TZ=UTC ls --time-style="+%Y-%m-%d_%H:%M:%S"
-l -d "${FSIMAGEP}0.img"|awk '{print $6; }'|sed 's,_, ,g')";;
esac
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 7/9] tests: A failure of mktemp should cause the test script to exit with code 99
A test exiting with code 99 means that there was an error in the test itself
and not a failure in the thing being tested (also known as a hard error).
Signed-off-by: Glenn Washburn
---
tests/ahci_test.in| 4 ++--
tests/ehci_test.in| 4 ++--
tests/gettext_strings_test.in | 2 +-
tests/grub_cmd_test.in| 6 +++---
tests/grub_script_blockarg.in | 2 +-
tests/ohci_test.in| 4 ++--
tests/partmap_test.in | 4 ++--
tests/pata_test.in| 4 ++--
tests/syslinux_test.in| 2 +-
tests/test_sha512sum.in | 6 +++---
tests/uhci_test.in| 4 ++--
11 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/tests/ahci_test.in b/tests/ahci_test.in
index 7853bb8b6..3f7645ad8 100644
--- a/tests/ahci_test.in
+++ b/tests/ahci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
echo "hello" > "$outfile"
diff --git a/tests/ehci_test.in b/tests/ehci_test.in
index 426bfa7db..df408d77a 100644
--- a/tests/ehci_test.in
+++ b/tests/ehci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
echo "hello" > "$outfile"
diff --git a/tests/gettext_strings_test.in b/tests/gettext_strings_test.in
index 813999ebe..1c37fe41b 100644
--- a/tests/gettext_strings_test.in
+++ b/tests/gettext_strings_test.in
@@ -2,7 +2,7 @@
cd '@srcdir@'
-tdir="$(mktemp -d "${TMPDIR:-/tmp}/tmp.XX")"
+tdir="$(mktemp -d "${TMPDIR:-/tmp}/tmp.XX")" || exit 99
xgettext -f po/POTFILES.in -L C -o "$tdir/"skip.pot -x po/grub.pot
--keyword=grub_util_info --keyword=grub_dprintf:1,2
--keyword=grub_register_command --keyword=grub_register_extcmd
--keyword=grub_efiemu_resolve_symbol --keyword=grub_efiemu_register_symbol
--keyword=grub_dl_load --keyword=grub_crypto_lookup_md_by_name
--keyword=grub_crypto_lookup_cipher_by_name
--keyword=grub_efiemu_resolve_symbol --keyword=alias
--keyword=grub_ieee1275_get_property:2 --keyword=grub_ieee1275_find_device
--keyword=grub_ieee1275_get_integer_property:2 --keyword=INIT_IEEE1275_COMMON:2
--keyword=grub_boot_time --keyword=grub_env_get --keyword=grub_env_set
--keyword=grub_register_variable_hook --keyword=grub_fatal --keyword=__asm__
--keyword=volatile --keyword=__volatile__ --keyword=grub_error:2
--from-code=iso-8859-1
xgettext -f po/POTFILES.in -L C -o "$tdir/"skip2.pot -x po/grub.pot
--keyword=volatile:2 --keyword=__volatile__:2 --keyword=grub_dprintf:2
--from-code=iso-8859-1
diff --git a/tests/grub_cmd_test.in b/tests/grub_cmd_test.in
index dac6f7d6a..043c3a634 100644
--- a/tests/grub_cmd_test.in
+++ b/tests/grub_cmd_test.in
@@ -2,8 +2,8 @@
set -e
# create a randome file
-empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-non_empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+non_empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
cat >$non_empty <$outfile <$tmp
error_if_not "`head -n1 $tmp|tail -n1`" '{ test_blockarg { true } }'
diff --git a/tests/ohci_test.in b/tests/ohci_test.in
index 2b96539b8..c72618656 100644
--- a/tests/ohci_test.in
+++ b/tests/ohci_test.in
@@ -34,8 +34,8 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
exit 0;;
esac
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
+imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
+outfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 99
echo "hello" > "$outfile"
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 5f18ab51c..7906db43d 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -101,8 +101,8 @@ if ! which parted >/dev/null 2>&1; then
exit 77
fi
-imgfile="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
-outfile="`mktemp &q
[PATCH v4 3/9] tests: Add set -e to missing tests
This helps to ensure that error codes do not get ignored.
Signed-off-by: Glenn Washburn
---
tests/grub_cmd_test.in| 1 +
tests/grub_script_blockarg.in | 1 +
tests/test_sha512sum.in | 1 +
3 files changed, 3 insertions(+)
diff --git a/tests/grub_cmd_test.in b/tests/grub_cmd_test.in
index 3399eb292..dac6f7d6a 100644
--- a/tests/grub_cmd_test.in
+++ b/tests/grub_cmd_test.in
@@ -1,4 +1,5 @@
#! @BUILD_SHEBANG@
+set -e
# create a randome file
empty="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
diff --git a/tests/grub_script_blockarg.in b/tests/grub_script_blockarg.in
index 6ea9b8c3d..134a43b05 100644
--- a/tests/grub_script_blockarg.in
+++ b/tests/grub_script_blockarg.in
@@ -1,4 +1,5 @@
#! @BUILD_SHEBANG@
+set -e
# Run GRUB script in a Qemu instance
# Copyright (C) 2010 Free Software Foundation, Inc.
diff --git a/tests/test_sha512sum.in b/tests/test_sha512sum.in
index 027092a8b..d97b7ae2c 100644
--- a/tests/test_sha512sum.in
+++ b/tests/test_sha512sum.in
@@ -1,4 +1,5 @@
#! @BUILD_SHEBANG@
+set -e
# create a randome file
file="`mktemp "${TMPDIR:-/tmp}/tmp.XX"`" || exit 1
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 2/4] tests: Do not occlude subshell error codes
On Tue, 12 Oct 2021 21:40:45 +0200
Daniel Kiper wrote:
> On Mon, Oct 11, 2021 at 01:06:17PM -0500, Glenn Washburn wrote:
> > On Mon, 11 Oct 2021 16:20:46 +0200
> > Daniel Kiper wrote:
> >
> > > On Thu, Oct 07, 2021 at 03:33:25PM -0500, Glenn Washburn wrote:
> > > > When a subshell's output is used as input to a "simple command", its
> > > > return
>
> I think you should move "(usually the test command)" from below here
> and/or add definition of "simple command".
I didn't state it, but "simple command" is defined in the man page for
dash and bash.
>
> > > > code is not checked. These subshells contain an execution of the
> > > > grub-shell
> > > > script which does the work of the actual test. If grub-shell returns an
> >
> > This "grub-shell" should be "subshell"
> >
> > > > error code, the test should fail. So refactor to not have the subshell
> > > > which
> > > > contains grub-shell be direct input into a simple command (usually the
> > > > test
> >
> > This "grub-shell" is correct, as far as I can tell.
> >
> > > > command). Mostly this is accomplished by having the output first go to a
> > > > shell variable, and then using the shell variable in the simple command.
> > > >
> > > > Signed-off-by: Glenn Washburn
> > > > ---
> > > > tests/ahci_test.in | 7 ++-
> > > > tests/cdboot_test.in| 3 ++-
> > > > tests/core_compress_test.in | 6 --
> > > > tests/ehci_test.in | 7 ++-
> > > > tests/fddboot_test.in | 3 ++-
> > > > tests/gzcompress_test.in| 3 ++-
> > > > tests/hddboot_test.in | 3 ++-
> > > > tests/lzocompress_test.in | 3 ++-
> > > > tests/netboot_test.in | 3 ++-
> > > > tests/ohci_test.in | 7 ++-
> > > > tests/pata_test.in | 3 ++-
> > > > tests/uhci_test.in | 7 ++-
> > > > tests/xzcompress_test.in| 3 ++-
> > > > 13 files changed, 44 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/tests/ahci_test.in b/tests/ahci_test.in
> > > > index 9b1d85df4..1e4e3e443 100644
> > > > --- a/tests/ahci_test.in
> > > > +++ b/tests/ahci_test.in
> > > > @@ -41,7 +41,12 @@ echo "hello" > "$outfile"
> > > >
> > > > tar cf "$imgfile" "$outfile"
> > > >
> > > > -if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}"
> > > > --qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci
> > > > -device ide-hd,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World"
> > > > ]; then
> > > > +v=$(echo "nativedisk; source '(ahci0)/$outfile';" |
> > > > +"${grubshell}" --qemu-opts="-drive id=disk,file=$imgfile,if=none
> > > > + -device ahci,id=ahci
> > > > + -device ide-hd,drive=disk,bus=ahci.0" |
> > > > +tail -n 1)
> > >
> > > The first change partially contradicts what you said in the commit
> > > message. I had to take a look at the patch #3 to understand why you did
> > > not drop "tail -n 1". I think the commit message should be clarified in
> > > a such way which does not require referring to another patch to
> > > understand this one.
> >
> > Yep, mix up. Please see correction above to see if it removes the
> > contradiction you're seeing. The intent was not to refer to a further
> > patch.
>
> I think it improves situation a bit. If you could think how to reword
> this commit message in general that (probably) would be better.
Yes, I agree. Now I'm seeing I was being more generic than I needed to
be and wasn't quite correct.
>
> > > > +if [ "$v" != "Hello World" ]; then
> > > > rm "$imgfile"
> > > > rm "$outfile"
> > > > exit 1
> > > > diff --git a/tests/cdboot_test.in b/tests/cdboot_test.in
> > > > index 75acdfedb..7229f79fb 100644
> > > > --- a/tests/cdboot_test.in
> > > > +++ b/tests/cdboot_test.in
> > > > @@ -34,6 +34,7 @@ case
> > > > "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
> > > > exit 0;;
> > > > esac
> > > >
> > > > -if [ "$(echo hello | "${grubshell}" --boot=cd)" != "Hello World" ];
> > > > then
> > > > +v=`echo hello | "${grubshell}" --boot=cd`
> > >
> > > I would prefer if you are more consistent and use "$(...)" instead of
> > > "`...`".
> > > The former looks more common in these scripts.
> >
> > Yep, I agree and prefer in general the former style. Not sure, why I
> > changed style there. I see its done in a few places. Want me to just
> > resend this patch or the whole series?
Now I see this change from $(...) to `...` was unintentional and due to
starting from an older commit.
>
> I think it would be easier for me if you could merge v3 with v2 patch
> series and repost as one batch.
Ok, I'll do that.
Glenn
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 1/9] tests: Fix partmap_test for arm*-efi, disk numbering has changed
Perhaps using a newer UEFI firmware is the reason for the created test disk
showing up as hd2 instead of hd3.
Signed-off-by: Glenn Washburn
---
tests/partmap_test.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/partmap_test.in b/tests/partmap_test.in
index 6ef518b0a..7353dc70e 100644
--- a/tests/partmap_test.in
+++ b/tests/partmap_test.in
@@ -89,7 +89,7 @@ case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
disk=arc/scsi0/disk0/rdisk0
;;
arm*-efi)
- disk=hd3
+ disk=hd2
;;
*)
disk=hd0
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v4 0/9] Various fixes/improvements for tests
Updates since v3:
* Dropped iso9660_test LANG patch
* Merged v3 in v2
* Rewrote commit message for #4 to hopefully be clearer
* Replace `...` with $(...)
These are mostly test improvement and fixes that stand on their own.
Glenn
Glenn Washburn (9):
tests: Fix partmap_test for arm*-efi, disk numbering has changed
tests: When checking squashfs fstime, use superblock last modified
time
tests: Add set -e to missing tests
tests: Do not occlude subshell error codes when used as input to the
test command
tests: Do not occlude grub-shell return code
tests: Make setup errors in grub-fs-tester hard errors
tests: A failure of mktemp should cause the test script to exit with
code 99
tests: Exit with skipped exit code when test not performed
tests: Use @BUILD_SHEBANG@ autoconf var instead of literal shell
tests/ahci_test.in | 19 ---
tests/cdboot_test.in | 11 ++-
tests/core_compress_test.in| 8 +---
tests/ehci_test.in | 19 ---
tests/f2fs_test.in | 2 +-
tests/fddboot_test.in | 19 ++-
tests/gettext_strings_test.in | 2 +-
tests/grub_cmd_date.in | 5 +++--
tests/grub_cmd_set_date.in | 6 +++---
tests/grub_cmd_sleep.in| 2 +-
tests/grub_cmd_test.in | 7 ---
tests/grub_script_blockarg.in | 3 ++-
tests/grub_script_expansion.in | 3 ++-
tests/gzcompress_test.in | 3 ++-
tests/hddboot_test.in | 9 +
tests/lzocompress_test.in | 3 ++-
tests/netboot_test.in | 15 ---
tests/ohci_test.in | 19 ---
tests/partmap_test.in | 18 +-
tests/pata_test.in | 13 +++--
tests/pseries_test.in | 2 +-
tests/syslinux_test.in | 2 +-
tests/test_sha512sum.in| 7 ---
tests/uhci_test.in | 19 ---
tests/util/grub-fs-tester.in | 17 -
tests/xzcompress_test.in | 3 ++-
26 files changed, 139 insertions(+), 97 deletions(-)
Range-diff against v3:
1: 3a2cc2738 = 1: 3a2cc2738 tests: Fix partmap_test for arm*-efi, disk
numbering has changed
2: 3da044f56 = 2: 3da044f56 tests: When checking squashfs fstime, use
superblock last modified time
3: a061a35f5 = 3: a061a35f5 tests: Add set -e to missing tests
4: ab85eb4c8 ! 4: 5828e4c79 tests: Do not occlude subshell error codes
@@ Metadata
Author: Glenn Washburn
## Commit message ##
-tests: Do not occlude subshell error codes
+tests: Do not occlude subshell error codes when used as input to the
test command
-When a subshell's output is used as input to a "simple command", its
return
-code is not checked. These subshells contain an execution of the
grub-shell
-script which does the work of the actual test. If grub-shell returns an
-error code, the test should fail. So refactor to not have the subshell
which
-contains grub-shell be direct input into a simple command (usually the
test
-command). Mostly this is accomplished by having the output first go to
a
-shell variable, and then using the shell variable in the simple
command.
+When using the output of a subshell as input, its error code is
ignored in
+the context of "set -e". Many test scripts use grub-shell in a
subshell with
+output used as an argument to the test command to test for expected
output.
+Refactor these tests so that the subshell output goes to a shell
variable,
+so that if the subshell errors the script will immediately exit with an
+error code.
## tests/ahci_test.in ##
@@ tests/ahci_test.in: echo "hello" > "$outfile"
@@ tests/cdboot_test.in: case
"${grub_modinfo_target_cpu}-${grub_modinfo_platform}"
esac
-if [ "$(echo hello | "${grubshell}" --boot=cd)" != "Hello World" ]; then
-+v=`echo hello | "${grubshell}" --boot=cd`
++v=$(echo hello | "${grubshell}" --boot=cd)
+if [ "$v" != "Hello World" ]; then
exit 1
fi
@@ tests/core_compress_test.in: case
"${grub_modinfo_target_cpu}-${grub_modinfo_pla
-if [ "$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz)"
!= "Hello World" ]; then
-+v=`echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz`
++v=$(echo hello | "${grubshell}" --grub-mkimage-extra=--compress=xz)
+if [ "$v" != "Hello World" ]; then
exit 1
fi
-if [ "$(echo hello | "${grubshell}"
--grub-mkimage-extra=--compress=none)" != "Hello World" ]; then
-+v=`echo hello | &
