On Fri, May 13, 2022 at 12:54:11PM -0500, Glenn Washburn wrote: > This command is meant to behave similarly to the 'mode' command of the EFI > Shell application. One difference is that to set the mode the mode number > is given, not the rows and columns of the desired mode. Also supported are > the arguments "min" and "max", which set the mode to the minimum and > maximum mode respectively as calculated by the columns * rows of that mode. > > Signed-off-by: Glenn Washburn <developm...@efficientek.com> > Reviewed-by: Gerd Hoffmann <kra...@redhat.com> > Tested-by: Gerd Hoffmann <kra...@redhat.com> > --- > grub-core/Makefile.core.def | 6 ++ > grub-core/commands/efi/efitextmode.c | 118 +++++++++++++++++++++++++++ > 2 files changed, 124 insertions(+) > create mode 100644 grub-core/commands/efi/efitextmode.c > > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def > index 726f51be7..b22e48f0f 100644 > --- a/grub-core/Makefile.core.def > +++ b/grub-core/Makefile.core.def > @@ -813,6 +813,12 @@ module = { > enable = efi; > }; > > +module = { > + name = efitextmode; > + efi = commands/efi/efitextmode.c; > + enable = efi; > +}; > + > module = { > name = blocklist; > common = commands/blocklist.c; > diff --git a/grub-core/commands/efi/efitextmode.c > b/grub-core/commands/efi/efitextmode.c > new file mode 100644 > index 000000000..fb72aa6f3 > --- /dev/null > +++ b/grub-core/commands/efi/efitextmode.c > @@ -0,0 +1,118 @@ > +/* efitextmode.c - command to get/set text mode resolution */ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2003,2005,2006,2007,2009 Free Software Foundation, Inc.
s/2003,2005,2006,2007,2009/2022/ > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. Please add one line here describing what this module does. Good example is in grub-core/kern/efi/sb.c. > + */ > + > +#include <grub/dl.h> > +#include <grub/misc.h> > +#include <grub/mm.h> > +#include <grub/command.h> > +#include <grub/i18n.h> > +#include <grub/efi/efi.h> > +#include <grub/efi/api.h> > + > +GRUB_MOD_LICENSE ("GPLv3+"); > + > +static grub_err_t > +grub_cmd_efitextmode (grub_command_t cmd __attribute__ ((unused)), > + int argc, char **args) > +{ > + grub_efi_simple_text_output_interface_t *o; > + unsigned long mode; > + const char *p = NULL; > + grub_efi_status_t status; > + grub_efi_uintn_t columns, rows; > + grub_efi_int32_t i; > + > + if (argc > 1) > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("at most one argument > expected")); > + > + o = grub_efi_system_table->con_out; I think you can do it in definition above. > + > + if (argc == 1) > + { > + if (grub_strcmp (args[0], "min") == 0) > + mode = 0; > + else if (grub_strcmp (args[0], "max") == 0) > + mode = o->mode->max_mode - 1; Are your sure "o" and "o->mode" are always not NULL? > + else > + { > + mode = grub_strtoul (args[0], &p, 0); > + > + if (grub_errno != GRUB_ERR_NONE) > + return grub_errno; Please drop this check. It is redundant as we chatted earlier. > + if (*args[0] == '\0' || *p != '\0') > + return grub_error (GRUB_ERR_BAD_ARGUMENT, > + N_("non-numeric or invalid mode `%s'"), > + args[0]); > + } > + > + if (mode < (unsigned long) o->mode->max_mode) > + { > + if (mode != (unsigned long) o->mode->mode) > + { > + status = efi_call_2 (o->set_mode, o, (grub_efi_int32_t) mode); > + if (status == GRUB_EFI_SUCCESS) > + ; > + else if (status == GRUB_EFI_DEVICE_ERROR) > + return grub_error (GRUB_ERR_BAD_DEVICE, > + N_("device error: could not set requested" > + " mode")); Please do not wrap error msg here. I am OK with lines a bit longer than 80 chars. And I think you can put args[0] in one line with error msg above too. > + else if (status == GRUB_EFI_UNSUPPORTED) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, > + N_("invalid mode: number not valid")); > + else > + return grub_error (GRUB_ERR_BAD_OS, > + N_("unexpected EFI error number: `%u'"), > + (unsigned) status); > + } > + } > + else > + return grub_error (GRUB_ERR_BAD_ARGUMENT, > + N_("invalid mode: `%lu' is greater than maximum" > + " mode `%lu'"), Ditto. > + mode, (unsigned long) o->mode->max_mode); > + } > + > + if (argc == 0) > + { > + grub_printf_ (N_("Available modes for console output device.\n")); > + > + for (i=0; i < o->mode->max_mode; i++) i = 0; Missing spaces... > + if (GRUB_EFI_SUCCESS == efi_call_4 (o->query_mode, o, i, > + &columns, &rows)) > + grub_printf_ (N_(" [%lu] Col %5u Row %5u %c\n"), > + (unsigned long) i, (unsigned) columns, (unsigned) rows, Could we avoid this casts? Same above. I think we can introduce PRIxGRUB_EFI_UINT32_T, etc. They should land in include/grub/efi/api.h somewhere around grub_efi_uint*_t typedefs. > + (i == o->mode->mode) ? '*' : ' '); > + } > + > + return GRUB_ERR_NONE; > +} > + > +static grub_command_t cmd; > + Please drop this ^L. > +GRUB_MOD_INIT(cmp) GRUB_MOD_INIT (cmp) > +{ > + cmd = grub_register_command ("efitextmode", grub_cmd_efitextmode, > + N_("[min|max|mode_num]"), N_("Get or set EFI > text mode.")); > +} > + > +GRUB_MOD_FINI(cmp) GRUB_MOD_FINI (cmp) > +{ > + grub_unregister_command (cmd); > +} Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel