Le jeu. 8 mai 2025, 15:49, Yair Yarom <[email protected]> a écrit :
> Initial testpci module and command used to query if PCI devices are
> present.
>
> ---
> docs/grub.texi | 8 ++
> grub-core/Makefile.core.def | 7 ++
> grub-core/commands/testpci.c | 167 +++++++++++++++++++++++++++++++++++
> 3 files changed, 182 insertions(+)
> create mode 100644 grub-core/commands/testpci.c
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 2b3d536d3..296c60b72 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4139,6 +4139,7 @@ Modules can be loaded via the @command{insmod}
> (@pxref{insmod}) command.
> * test_module::
> * test_blockarg_module::
> * testload_module::
> +* testpci_module::
> * testspeed_module::
> * tftp_module::
> * tga_module::
> @@ -5728,6 +5729,12 @@ argument function in GRUB internal functions via a
> test command
> This module is intended for performing a functional test of some file
> reading /
> seeking functions in GRUB internals via a test command @command{testload}.
>
> +@node testpci_module
> +@section testpci
> +This module provides support for the @command{testpci} command. This
> +command can be used to test if specific PCI / PCIe devices are found on
> +the system.
> +
>
This needs more details. How to use it, format of the arguments and format
of the file. Is possible to know it from reading the code but this
shouldn't be a requirement.
>
> +#include <grub/dl.h>
> +#include <grub/extcmd.h>
> +#include <grub/mm.h>
> +#include <grub/file.h>
> +#include <grub/normal.h>
>
What requires normal.h? Just curious
> +static const struct grub_arg_option options[] = {
> + {"file", 0, 0, "read device list from file", "FILE", ARG_TYPE_STRING},
>
You can also add a short option.
> +struct grub_testpci_devlist {
> + char** devices;
> + int n_devices;
> + int s_devices;
>
What is s_devices? Maybe a more descriptive name? From reading code I see
that it's the number of allocated devices. Maybe name it allocated_devices?
> + bool found;
> +};
> +
> +static int
> +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)),
> + grub_pci_id_t pciid,
> + void *data) {
> +
> + struct grub_testpci_devlist* devlist = (struct
> grub_testpci_devlist*)data;
> +
> + char* device = grub_xasprintf ("%04x:%04x", pciid & 0xFFFF, pciid >>
> 16);
>
You forgot to check that allocation succeeded. The easiest way around it is
to allocate it on stack and use snprintf. 10 bytes should be enough
> +
> +static void
> +testpci_add_device_to_list(struct grub_testpci_devlist* devlist,
> + char* device)
> +{
> + if (devlist->n_devices == devlist->s_devices) {
> + devlist->s_devices *= 2;
> + devlist->devices = grub_realloc(devlist->devices,
> + devlist->s_devices * sizeof(char*));
> + if (!(devlist->devices)) {
>
Memory leak if I remember realloc semantics correctly.
> + return;
> + }
> + }
> + devlist->devices[devlist->n_devices++] = grub_strdup(device);
>
You forgot the check for the error.
> +}
> +
> +static grub_err_t
> +grub_cmd_testpci (grub_extcmd_context_t ctxt,
> + int argc, char **args)
> +{
> +
> + struct grub_testpci_devlist devlist;
> +
> + devlist.found = false;
> + devlist.n_devices = 0;
> + devlist.s_devices = argc + (ctxt->state[0].set ? 5 : 0);
> + devlist.devices = grub_malloc(devlist.s_devices * sizeof(char*));
> + if (!(devlist.devices)) {
> + return GRUB_ERR_OUT_OF_MEMORY;
>
return grub_errno;
> + }
> +
> + for (int i = 0; i < argc; i++) {
> + testpci_add_device_to_list(&devlist, args[i]);
> + if (!(devlist.devices)) {
> + return GRUB_ERR_OUT_OF_MEMORY;
>
Ditto
> + }
> + }
> +
> + /* device list from file */
> + if (ctxt->state[0].set) {
> +
> + grub_file_t listfile = grub_file_open(ctxt->state[0].arg,
> GRUB_FILE_TYPE_NONE);
>
File type is wrong
> + if (listfile) {
>
You need to pass on the error if the file fails to open. Not just do
without it
> +
> + char *buf = NULL;
> + while (grub_free (buf), (buf = grub_file_getline (listfile))) {
> +
> + /* remove comments */
> + char *p = grub_strchr(buf, '#');
> + if (p) {
> + *p = '\0';
> + }
> +
> + /* remove suffix spaces */
> + p = buf + grub_strlen(buf) - 1;
> + while (p >= buf && *p && grub_isspace(*p)) {
> + *p-- = '\0';
> + }
> +
> + /* remove prefix spaces */
> + p = buf;
> + while (*p && grub_isspace(*p)) {
> + p++;
> + }
> +
> + /* ignore empty */
> + if (*p == '\0')
> + continue;
> +
> + testpci_add_device_to_list(&devlist, p);
> + if (!(devlist.devices)) {
> + return GRUB_ERR_OUT_OF_MEMORY;
>
Ditto
> + }
> +
> + }
> +
> + grub_file_close (listfile);
> + }
> + }
> +
> + for (int d = 0 ; d < devlist.n_devices; d++) {
> + if (grub_strlen(devlist.devices[d]) != 9 || devlist.devices[d][4] !=
> ':') {
> + grub_printf("bad input device (%d) \"%s\", expected xxxx:xxxx\n",
> d, devlist.devices[d]);
>
Return an error.
> + }
> + }
> +
> + grub_pci_iterate (grub_testpci_iter, (void*)&devlist);
> +
> + for (int i = 0; i < devlist.n_devices; i++) {
> + grub_free(devlist.devices[i]);
> + }
> + grub_free(devlist.devices);
> + return devlist.found ? GRUB_ERR_NONE : GRUB_ERR_TEST_FAILURE;
>
You need to call grub_error here
> +}
> +
> +static grub_extcmd_t cmd;
> +
> +GRUB_MOD_INIT(testpci)
> +{
> + cmd = grub_register_extcmd ("testpci", grub_cmd_testpci, 0,
> + "[<devid> [...]] [--file <filename>]",
> + N_("Check if any of the PCI devices
> exist."), options);
> +}
> +
> +GRUB_MOD_FINI(testpci)
> +{
> + grub_unregister_extcmd (cmd);
> +}
> --
> 2.39.5
>
>
> _______________________________________________
> Grub-devel mailing list
> [email protected]
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
Regards
Vladimir 'phcoder' Serbinenko
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
