From 39ee4a14a940bc5ffa2168dd729eb7018dbe19cd Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Thu, 14 Dec 2023 21:19:09 -0700 Subject: pxe: Refactor to reduce the size of label_boot() This function is far too long and complicated. Split out the part which actually calls the boot commands into a separate function. Change a strncpy() to strlcpy() to keep checkpatch happy. No functional change is intended. Signed-off-by: Simon Glass --- boot/pxe_utils.c | 301 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 163 insertions(+), 138 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 2c87aba..6fbccad 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -470,159 +470,47 @@ skip_overlay: #endif /** - * label_boot() - Boot according to the contents of a pxe_label + * label_run_boot() - Run the correct boot procedure * - * If we can't boot for any reason, we return. A successful boot never - * returns. + * fdt usage is optional: + * It handles the following scenarios. * - * The kernel will be stored in the location given by the 'kernel_addr_r' - * environment variable. + * Scenario 1: If fdt_addr_r specified and "fdt" or "fdtdir" label is + * defined in pxe file, retrieve fdt blob from server. Pass fdt_addr_r to + * bootm, and adjust argc appropriately. * - * If the label specifies an initrd file, it will be stored in the location - * given by the 'ramdisk_addr_r' environment variable. + * If retrieve fails and no exact fdt blob is specified in pxe file with + * "fdt" label, try Scenario 2. * - * If the label specifies an 'append' line, its contents will overwrite that - * of the 'bootargs' environment variable. + * Scenario 2: If there is an fdt_addr specified, pass it along to + * bootm, and adjust argc appropriately. + * + * Scenario 3: If there is an fdtcontroladdr specified, pass it along to + * bootm, and adjust argc appropriately, unless the image type is fitImage. + * + * Scenario 4: fdt blob is not available. * * @ctx: PXE context * @label: Label to process + * @kernel_addr: string containing the kernel address / config + * @initrd_str: string containing the initrd address / size + * @initrd_addr_str: initrd address, or NULL if none + * @initrd_filesize: initrd size in bytes; only valid if initrd_addr_str is not + * NULL * Returns does not return on success, otherwise returns 0 if a localboot * label was processed, or 1 on error */ -static int label_boot(struct pxe_context *ctx, struct pxe_label *label) +static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label, + char *kernel_addr, char *initrd_str, + char *initrd_addr_str, char *initrd_filesize) { char *bootm_argv[] = { "bootm", NULL, NULL, NULL, NULL }; char *zboot_argv[] = { "zboot", NULL, "0", NULL, NULL }; - char *kernel_addr = NULL; - char *initrd_addr_str = NULL; - char initrd_filesize[10]; - char initrd_str[28]; - char mac_str[29] = ""; - char ip_str[68] = ""; - char *fit_addr = NULL; + ulong kernel_addr_r; int bootm_argc = 2; int zboot_argc = 3; - int len = 0; - ulong kernel_addr_r; void *buf; - label_print(label); - - label->attempted = 1; - - if (label->localboot) { - if (label->localboot_val >= 0) - label_localboot(label); - return 0; - } - - if (!label->kernel) { - printf("No kernel given, skipping %s\n", - label->name); - return 1; - } - - if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", - NULL) < 0) { - printf("Skipping %s for failure retrieving kernel\n", - label->name); - return 1; - } - - kernel_addr = env_get("kernel_addr_r"); - /* for FIT, append the configuration identifier */ - if (label->config) { - int len = strlen(kernel_addr) + strlen(label->config) + 1; - - fit_addr = malloc(len); - if (!fit_addr) { - printf("malloc fail (FIT address)\n"); - return 1; - } - snprintf(fit_addr, len, "%s%s", kernel_addr, label->config); - kernel_addr = fit_addr; - } - - /* For FIT, the label can be identical to kernel one */ - if (label->initrd && !strcmp(label->kernel_label, label->initrd)) { - initrd_addr_str = kernel_addr; - } else if (label->initrd) { - ulong size; - if (get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", - &size) < 0) { - printf("Skipping %s for failure retrieving initrd\n", - label->name); - goto cleanup; - } - strcpy(initrd_filesize, simple_xtoa(size)); - initrd_addr_str = env_get("ramdisk_addr_r"); - size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx", - initrd_addr_str, size); - if (size >= sizeof(initrd_str)) - goto cleanup; - } - - if (label->ipappend & 0x1) { - sprintf(ip_str, " ip=%s:%s:%s:%s", - env_get("ipaddr"), env_get("serverip"), - env_get("gatewayip"), env_get("netmask")); - } - - if (IS_ENABLED(CONFIG_CMD_NET)) { - if (label->ipappend & 0x2) { - int err; - - strcpy(mac_str, " BOOTIF="); - err = format_mac_pxe(mac_str + 8, sizeof(mac_str) - 8); - if (err < 0) - mac_str[0] = '\0'; - } - } - - if ((label->ipappend & 0x3) || label->append) { - char bootargs[CONFIG_SYS_CBSIZE] = ""; - char finalbootargs[CONFIG_SYS_CBSIZE]; - - if (strlen(label->append ?: "") + - strlen(ip_str) + strlen(mac_str) + 1 > sizeof(bootargs)) { - printf("bootarg overflow %zd+%zd+%zd+1 > %zd\n", - strlen(label->append ?: ""), - strlen(ip_str), strlen(mac_str), - sizeof(bootargs)); - goto cleanup; - } - - if (label->append) - strlcpy(bootargs, label->append, sizeof(bootargs)); - - strcat(bootargs, ip_str); - strcat(bootargs, mac_str); - - cli_simple_process_macros(bootargs, finalbootargs, - sizeof(finalbootargs)); - env_set("bootargs", finalbootargs); - printf("append: %s\n", finalbootargs); - } - - /* - * fdt usage is optional: - * It handles the following scenarios. - * - * Scenario 1: If fdt_addr_r specified and "fdt" or "fdtdir" label is - * defined in pxe file, retrieve fdt blob from server. Pass fdt_addr_r to - * bootm, and adjust argc appropriately. - * - * If retrieve fails and no exact fdt blob is specified in pxe file with - * "fdt" label, try Scenario 2. - * - * Scenario 2: If there is an fdt_addr specified, pass it along to - * bootm, and adjust argc appropriately. - * - * Scenario 3: If there is an fdtcontroladdr specified, pass it along to - * bootm, and adjust argc appropriately, unless the image type is fitImage. - * - * Scenario 4: fdt blob is not available. - */ bootm_argv[3] = env_get("fdt_addr_r"); /* For FIT, the label can be identical to kernel one */ @@ -637,6 +525,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) fdtfile = label->fdt; } else if (label->fdtdir) { char *f1, *f2, *f3, *f4, *slash; + int len; f1 = env_get("fdtfile"); if (f1) { @@ -679,7 +568,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) fdtfilefree = malloc(len); if (!fdtfilefree) { printf("malloc fail (FDT filename)\n"); - goto cleanup; + return -ENOMEM; } snprintf(fdtfilefree, len, "%s%s%s%s%s%s", @@ -698,7 +587,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (label->fdt) { printf("Skipping %s for failure retrieving FDT\n", label->name); - goto cleanup; + return -ENOENT; } } @@ -757,6 +646,142 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) unmap_sysmem(buf); + return 0; +} + +/** + * label_boot() - Boot according to the contents of a pxe_label + * + * If we can't boot for any reason, we return. A successful boot never + * returns. + * + * The kernel will be stored in the location given by the 'kernel_addr_r' + * environment variable. + * + * If the label specifies an initrd file, it will be stored in the location + * given by the 'ramdisk_addr_r' environment variable. + * + * If the label specifies an 'append' line, its contents will overwrite that + * of the 'bootargs' environment variable. + * + * @ctx: PXE context + * @label: Label to process + * Returns does not return on success, otherwise returns 0 if a localboot + * label was processed, or 1 on error + */ +static int label_boot(struct pxe_context *ctx, struct pxe_label *label) +{ + char *kernel_addr = NULL; + char *initrd_addr_str = NULL; + char initrd_filesize[10]; + char initrd_str[28]; + char mac_str[29] = ""; + char ip_str[68] = ""; + char *fit_addr = NULL; + int ret; + + label_print(label); + + label->attempted = 1; + + if (label->localboot) { + if (label->localboot_val >= 0) + label_localboot(label); + return 0; + } + + if (!label->kernel) { + printf("No kernel given, skipping %s\n", + label->name); + return 1; + } + + if (get_relfile_envaddr(ctx, label->kernel, "kernel_addr_r", + NULL) < 0) { + printf("Skipping %s for failure retrieving kernel\n", + label->name); + return 1; + } + + kernel_addr = env_get("kernel_addr_r"); + /* for FIT, append the configuration identifier */ + if (label->config) { + int len = strlen(kernel_addr) + strlen(label->config) + 1; + + fit_addr = malloc(len); + if (!fit_addr) { + printf("malloc fail (FIT address)\n"); + return 1; + } + snprintf(fit_addr, len, "%s%s", kernel_addr, label->config); + kernel_addr = fit_addr; + } + + /* For FIT, the label can be identical to kernel one */ + if (label->initrd && !strcmp(label->kernel_label, label->initrd)) { + initrd_addr_str = kernel_addr; + } else if (label->initrd) { + ulong size; + + if (get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r", + &size) < 0) { + printf("Skipping %s for failure retrieving initrd\n", + label->name); + goto cleanup; + } + strcpy(initrd_filesize, simple_xtoa(size)); + initrd_addr_str = env_get("ramdisk_addr_r"); + size = snprintf(initrd_str, sizeof(initrd_str), "%s:%lx", + initrd_addr_str, size); + if (size >= sizeof(initrd_str)) + goto cleanup; + } + + if (label->ipappend & 0x1) { + sprintf(ip_str, " ip=%s:%s:%s:%s", + env_get("ipaddr"), env_get("serverip"), + env_get("gatewayip"), env_get("netmask")); + } + + if (IS_ENABLED(CONFIG_CMD_NET)) { + if (label->ipappend & 0x2) { + int err; + + strcpy(mac_str, " BOOTIF="); + err = format_mac_pxe(mac_str + 8, sizeof(mac_str) - 8); + if (err < 0) + mac_str[0] = '\0'; + } + } + + if ((label->ipappend & 0x3) || label->append) { + char bootargs[CONFIG_SYS_CBSIZE] = ""; + char finalbootargs[CONFIG_SYS_CBSIZE]; + + if (strlen(label->append ?: "") + + strlen(ip_str) + strlen(mac_str) + 1 > sizeof(bootargs)) { + printf("bootarg overflow %zd+%zd+%zd+1 > %zd\n", + strlen(label->append ?: ""), + strlen(ip_str), strlen(mac_str), + sizeof(bootargs)); + goto cleanup; + } + + if (label->append) + strlcpy(bootargs, label->append, sizeof(bootargs)); + + strcat(bootargs, ip_str); + strcat(bootargs, mac_str); + + cli_simple_process_macros(bootargs, finalbootargs, + sizeof(finalbootargs)); + env_set("bootargs", finalbootargs); + printf("append: %s\n", finalbootargs); + } + + ret = label_run_boot(ctx, label, kernel_addr, initrd_str, + initrd_addr_str, initrd_filesize); + cleanup: free(fit_addr); -- cgit v1.1