Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

CVS commit: src/sys



> Module Name:    src
> Committed By:   pgoyette
> Date:           Tue May  6 18:16:12 UTC 2025
> 
> Modified Files:
>         src/sys/arch/i386/stand/boot: boot2.c
>         src/sys/arch/i386/stand/lib: bootmenu.c libi386.h
>         src/sys/arch/i386/stand/pxeboot: main.c
>         src/sys/lib/libsa: bootcfg.c bootcfg.h
> 
> Log Message:
> Allow the dev= command when processing /boot.cfg file.  This
> addresses kern/59207

The goal of this change looks good, but I worry that:

(a) this is a high-risk change because if it breaks someone's
bootloader then it renders the machine nonbootable (and we have very
limited automatic testing of bootloaders -- which is a problem in
itself, not your fault!), and

(b) I'm confused by how some parts of it are relevant to the goal,
particularly these hunks:

--- a/sys/lib/libsa/bootcfg.c   Tue May 06 17:12:33 2025 +0000
+++ b/sys/lib/libsa/bootcfg.c   Tue May 06 18:16:12 2025 +0000
...
@@ -227,8 +227,6 @@ perform_bootcfg(const char *conf, bootcf
                        bootcfg_info.consdev = value;
                } else if (!strncmp(key, "root", 4)) {
                        bootcfg_info.root = value;
-               } else if (!strncmp(key, BOOTCFG_CMD_LOAD, 4)) {
-                       command(BOOTCFG_CMD_LOAD, value);
                } else if (!strncmp(key, "format", 6)) {
                        printf("value:%c\n", *value);
                        switch (*value) {
@@ -251,8 +249,6 @@ perform_bootcfg(const char *conf, bootcf
                        }
                } else if (!strncmp(key, "clear", 5)) {
                        bootcfg_info.clear = !!atoi(value);
-               } else if (!strncmp(key, BOOTCFG_CMD_USERCONF, 8)) {
-                       command(BOOTCFG_CMD_USERCONF, value);
                } else {
                        command(key, value);
                }

Were these branches already dead code?  Are they _now_ dead code on
x86, but possibly would have been still live on non-x86 bootloaders?

Can you expand on how this change works and what you did to test it?

As a general rule, I think we should post bootloader changes for
review to tech-kern@ and relevant port-*@ before committing, because
they are so high-risk.  It would also be good to state in the commit
message what testing has been done.


Home | Main Index | Thread Index | Old Index