> Please also see this refactor which conflicts with this patch: > > http://patchwork.ozlabs.org/project/uboot/list/?series=264265 > > I think that series should be reviewed/applied first since it was sent in August.
yes ! i think need update your series because cant apply it for current uboot state On Thu, Oct 14, 2021 at 12:58 AM Simon Glass <[email protected]> wrote: > > Hi, > > On Tue, 12 Oct 2021 at 21:39, Artem Lapkin <[email protected]> wrote: > > > > Problem > > > > PXE cannot boot normally after Sysboot changed the bootfile env (called > > from boot_extlinux) from the default "boot.scr.uimg" to > > "/boot/extlinux/extlinux.conf". > > > > In addition, an unbootable extlinux configuration will also make the PXE > > boot unbootable, because it will use the incorrect path "/boot/extlinux/" > > from the bootfile env. > > > > Solution > > > > sysboot must care about bootfile and restore default value after usage. > > > > Come from: > > https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ > > > > Signed-off-by: Artem Lapkin <[email protected]> > > --- > > cmd/sysboot.c | 30 ++++++++++++++++++++---------- > > 1 file changed, 20 insertions(+), 10 deletions(-) > > Please also see this refactor which conflicts with this patch: > > http://patchwork.ozlabs.org/project/uboot/list/?series=264265 > > I think that series should be reviewed/applied first since it was sent > in August. > > > > > diff --git a/cmd/sysboot.c b/cmd/sysboot.c > > index af6a2f1b7f..99b11cc127 100644 > > --- a/cmd/sysboot.c > > +++ b/cmd/sysboot.c > > @@ -2,6 +2,7 @@ > > > > #include <common.h> > > #include <command.h> > > +#include <malloc.h> > > #include <env.h> > > #include <fs.h> > > #include "pxe_utils.h" > > @@ -61,8 +62,9 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, > > int argc, > > unsigned long pxefile_addr_r; > > struct pxe_menu *cfg; > > char *pxefile_addr_str; > > - char *filename; > > + char *filename, *filename_bak; > > Can we see filename_bak to NULL so we can simply the free() below? > > > int prompt = 0; > > + int ret = 0; > > > > is_pxe = false; > > > > @@ -83,9 +85,10 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, > > int argc, > > pxefile_addr_str = argv[4]; > > } > > > > - if (argc < 6) { > > - filename = env_get("bootfile"); > > - } else { > > + filename = env_get("bootfile"); > > + if (argc > 5) { > > + filename_bak = malloc(strlen(filename) + 1); > > + strcpy(filename_bak, filename); > > filename = argv[5]; > > env_set("bootfile", filename); > > } > > @@ -98,26 +101,26 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, > > int argc, > > do_getfile = do_get_any; > > } else { > > printf("Invalid filesystem: %s\n", argv[3]); > > - return 1; > > + goto err; > > } > > fs_argv[1] = argv[1]; > > fs_argv[2] = argv[2]; > > > > if (strict_strtoul(pxefile_addr_str, 16, &pxefile_addr_r) < 0) { > > printf("Invalid pxefile address: %s\n", pxefile_addr_str); > > - return 1; > > + goto err; > > } > > > > if (get_pxe_file(cmdtp, filename, pxefile_addr_r) < 0) { > > printf("Error reading config file\n"); > > - return 1; > > + goto err; > > } > > > > cfg = parse_pxefile(cmdtp, pxefile_addr_r); > > > > if (!cfg) { > > printf("Error parsing config file\n"); > > - return 1; > > + goto err; > > } > > > > if (prompt) > > @@ -126,8 +129,15 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, > > int argc, > > handle_pxe_menu(cmdtp, cfg); > > > > destroy_pxe_menu(cfg); > > - > > - return 0; > > + goto ret; > > This is a bit ugly. Could we set 'ret' to 1 in the error cases above, > or set it to 1 at the top ad 0 here, or pull the parsing code into a > function...? > > > + err: > > + ret = 1; > > + ret: > > + if (filename_bak) { > > + env_set("bootfile", filename_bak); > > + free(filename_bak); > > + } > > + return ret; > > } > > > > U_BOOT_CMD(sysboot, 7, 1, do_sysboot, > > -- > > 2.25.1 > > > > Regards, > Simon
