Re: [PATCH grub] hurdhelper: Add helper to load GNU/Hurd multiboot modules

[Samuel Thibault]

Hello,

Damien Zammit, le ven. 30 sept. 2022 12:53:27 +0000, a ecrit:
> This adds a new grub module called hurdhelper.

That's nice, but be should be submitted to the grub mailing list :)

> +#define ACPI_TASK            "acpi"
> +#define PCI_TASK             "pci-arbiter"
> +#define DISK_TASK            "rumpdisk"
> +#define FS_TASK                      "ext2fs"
> +#define EXEC_TASK            "exec"

I don't think you want to hardcode this. Just take the modules in the
order they are given in the grub command and link them. The only things
you want to hardcode is passing the command line and the root fs details
to the FS task, and using ld to run exec.

(we'll probably want to add support for --next-task to ext2fs)

> +static char *hurd_argv[MAX_MODULES][MAX_PARAMS];

I don't think you nee to make it global? Making it dynamic will allow to
avoid hardcoding MAX_*, which will most likely hit us in the face at the
most unprobable time.

> +  /* first module has different args */

Better use an if?

Samuel