commit d3078a3bf96242b6ba15afe318018382e8e2d75e Author: Quentin Schulz Date: Wed Nov 8 11:14:51 2023 +0100 env: make env_get_from_linear return value pointer instead of memcpy Since the introducing commit, env_get_from_linear has been memcpy'ing the value (if found) into a buffer passed by the user. This is not efficient for multiple reasons: - memcpy'ing if there's no need to do string manipulations that modify the variable in place is a waste of time, - memcpy'ing requires the user pass a pointer to an allocated memory, meaning the user absolutely needs to know the maximum size of the value one wants to get from the environment. This is not always possible, so existing users have just started to use arbitrary max size from empirical data. Therefore, let's make env_get_from_linear return a pointer to the value in the environment directly. If one wants to do some destructive manipulation on it, one needs to do a memcpy (or strlcpy for example). Because env_get_from_linear now returns a pointer, env_get_f needs to do it as well. Obviously, all users of env_get_f (and env_get because it simply returns the return value of env_get) need to be updated to now use the return value directly. The old env_get_f (env_get) used to return -1 if no variable is found in the environment or the length of the value of that variable (which can be 0). The new env_get_f (env_get) returns NULL if not found, otherwise the pointer to value, which may very well be '\0' (so length 0). This allows to have virtually no limit on the size of values to fetch from an environment, and this regardless of the readiness of the environment (GD_FLG_ENV_READY). This typically fixes env_get_default for boot_targets on Rockchip (at least) platforms which is currently 36 characters long and would therefore truncate the value to 32 characters max (the size of gd->env_buf), making it impossible to know if it was modified or not, which is something puma-rk3399 and ringneck-px30 do. Note: this introduces a compiler warning because env_get_from_linear now drops the const part of the char pointer when returning it. This will be addressed in the next commit. Cc: Quentin Schulz Signed-off-by: Quentin Schulz diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c index 12d31184ad9..ab2199f27b2 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c @@ -1075,15 +1075,13 @@ int check_psci(void) static void config_core_prefetch(void) { char *buf = NULL; - char buffer[HWCONFIG_BUFFER_SIZE]; const char *prefetch_arg = NULL; struct arm_smccc_res res; size_t arglen; unsigned int mask; - if (env_get_f("hwconfig", buffer, sizeof(buffer)) > 0) - buf = buffer; - else + buf = env_get_f("hwconfig"); + if (!buf || !strlen(buf)) return; prefetch_arg = hwconfig_subarg_f("core_prefetch", "disable", diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c index 96183ac2c84..e3f196d285e 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c @@ -188,7 +188,7 @@ void disable_cpc_sram(void) static void enable_tdm_law(void) { int ret; - char buffer[HWCONFIG_BUFFER_SIZE] = {0}; + char *buffer; int tdm_hwconfig_enabled = 0; /* @@ -196,8 +196,8 @@ static void enable_tdm_law(void) * is not setup properly yet. Search for tdm entry in * hwconfig. */ - ret = env_get_f("hwconfig", buffer, sizeof(buffer)); - if (ret > 0) { + buffer = env_get_f("hwconfig"); + if (buffer && strlen(buffer)) { tdm_hwconfig_enabled = hwconfig_f("tdm", buffer); /* If tdm is defined in hwconfig, set law for tdm workaround */ if (tdm_hwconfig_enabled) @@ -213,15 +213,15 @@ void enable_cpc(void) int ret; u32 size = 0; u32 cpccfg0; - char buffer[HWCONFIG_BUFFER_SIZE]; + char *buffer; char cpc_subarg[16]; bool have_hwconfig = false; int cpc_args = 0; cpc_corenet_t *cpc = (cpc_corenet_t *)CFG_SYS_FSL_CPC_ADDR; /* Extract hwconfig from environment */ - ret = env_get_f("hwconfig", buffer, sizeof(buffer)); - if (ret > 0) { + buffer = env_get_f("hwconfig"); + if (buffer && strlen(buffer)) { /* * If "en_cpc" is not defined in hwconfig then by default all * cpcs are enable. If this config is defined then individual @@ -682,22 +682,16 @@ int cpu_init_r(void) #ifdef CONFIG_SYS_P4080_ERRATUM_CPU22 enable_cpu_a011_workaround = (SVR_MAJ(svr) < 3); #else - char buffer[HWCONFIG_BUFFER_SIZE]; - char *buf = NULL; - int n, res; - - n = env_get_f("hwconfig", buffer, sizeof(buffer)); - if (n > 0) - buf = buffer; + char *buf; + int res; + buf = env_get_f("hwconfig"); + if (buf && !strlen(buf)) + buf = NULL; res = hwconfig_arg_cmp_f("fsl_cpu_a011", "disable", buf); if (res > 0) { enable_cpu_a011_workaround = 0; } else { - if (n >= HWCONFIG_BUFFER_SIZE) { - printf("fsl_cpu_a011 was not found. hwconfig variable " - "may be too long\n"); - } enable_cpu_a011_workaround = (SVR_SOC_VER(svr) == SVR_P4080 && SVR_MAJ(svr) < 3) || (SVR_SOC_VER(svr) != SVR_P4080 && SVR_MAJ(svr) < 2); diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c index e26436bf570..e0119b8ce79 100644 --- a/arch/powerpc/cpu/mpc85xx/fdt.c +++ b/arch/powerpc/cpu/mpc85xx/fdt.c @@ -45,9 +45,8 @@ void ft_fixup_cpu(void *blob, u64 memory_limit) u32 id = get_my_id(); const char *enable_method; #if defined(T1040_TDM_QUIRK_CCSR_BASE) - int ret; int tdm_hwconfig_enabled = 0; - char buffer[HWCONFIG_BUFFER_SIZE] = {0}; + char *buffer; #endif off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4); @@ -96,8 +95,8 @@ void ft_fixup_cpu(void *blob, u64 memory_limit) * Extract hwconfig from environment. * Search for tdm entry in hwconfig. */ - ret = env_get_f("hwconfig", buffer, sizeof(buffer)); - if (ret > 0) + buffer = env_get_f("hwconfig"); + if (buffer && strlen(buffer)) tdm_hwconfig_enabled = hwconfig_f("tdm", buffer); /* Reserve the memory hole created by TDM LAW, so OSes dont use it */ diff --git a/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c b/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c index 7c2de02c4c5..ec89c96b92a 100644 --- a/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c +++ b/arch/powerpc/cpu/mpc85xx/fsl_corenet_serdes.c @@ -510,15 +510,15 @@ void fsl_serdes_init(void) int need_serdes_a001; /* true == need work-around for SERDES A001 */ #endif #ifdef CONFIG_SYS_P4080_ERRATUM_SERDES8 - char buffer[HWCONFIG_BUFFER_SIZE]; - char *buf = NULL; + char *buf; /* * Extract hwconfig from environment since we have not properly setup * the environment but need it for ddr config params */ - if (env_get_f("hwconfig", buffer, sizeof(buffer)) > 0) - buf = buffer; + buf = env_get_f("hwconfig"); + if (buf && !strlen(buf)) + buf = NULL; #endif if (serdes_prtcl_map & (1 << NONE)) return; diff --git a/board/aristainetos/aristainetos.c b/board/aristainetos/aristainetos.c index 17f37badd74..900a118c430 100644 --- a/board/aristainetos/aristainetos.c +++ b/board/aristainetos/aristainetos.c @@ -501,16 +501,15 @@ int board_fit_config_name_match(const char *name) static void do_board_detect(void) { - int ret; - char s[30]; + char *s; /* default use board type 7 */ gd->board_type = BOARD_TYPE_7; if (env_init()) return; - ret = env_get_f("panel", s, sizeof(s)); - if (ret < 0) + s = env_get_f("panel"); + if (!s) return; if (!strncmp("lg4573", s, 6)) diff --git a/board/esd/meesc/meesc.c b/board/esd/meesc/meesc.c index 9e362104224..d35db0e894a 100644 --- a/board/esd/meesc/meesc.c +++ b/board/esd/meesc/meesc.c @@ -159,7 +159,7 @@ int board_eth_init(struct bd_info *bis) #ifdef CONFIG_DISPLAY_BOARDINFO int checkboard(void) { - char str[32]; + char *str; u_char hw_type; /* hardware type */ /* read the "Type" register of the ET1100 controller */ @@ -186,7 +186,8 @@ int checkboard(void) puts("Board: EtherCAN/2 Gateway"); break; } - if (env_get_f("serial#", str, sizeof(str)) > 0) { + str = env_get_f("serial#"); + if (str && strlen(str)) { puts(", serial# "); puts(str); } diff --git a/board/freescale/mpc837xerdb/mpc837xerdb.c b/board/freescale/mpc837xerdb/mpc837xerdb.c index 97884a39796..bc6b84bb4ca 100644 --- a/board/freescale/mpc837xerdb/mpc837xerdb.c +++ b/board/freescale/mpc837xerdb/mpc837xerdb.c @@ -180,10 +180,11 @@ int board_early_init_f(void) int board_mmc_init(struct bd_info *bd) { struct immap __iomem *im = (struct immap __iomem *)CONFIG_SYS_IMMR; - char buffer[HWCONFIG_BUFFER_SIZE] = {0}; + char buffer; int esdhc_hwconfig_enabled = 0; - if (env_get_f("hwconfig", buffer, sizeof(buffer)) > 0) + buffer = env_get_f("hwconfig"); + if (buffer && strlen(buffer)) esdhc_hwconfig_enabled = hwconfig_f("esdhc", buffer); if (esdhc_hwconfig_enabled == 0) diff --git a/board/liebherr/mccmon6/spl.c b/board/liebherr/mccmon6/spl.c index b1f6881275d..cac2e351f5d 100644 --- a/board/liebherr/mccmon6/spl.c +++ b/board/liebherr/mccmon6/spl.c @@ -346,8 +346,7 @@ int board_fit_config_name_match(const char *name) #ifdef CONFIG_SPL_OS_BOOT int spl_start_uboot(void) { - char s[16]; - int ret; + char *s; /* * We use BOOT_DEVICE_MMC1, but SD card is connected * to MMC2 @@ -367,22 +366,21 @@ int spl_start_uboot(void) return 1; env_init(); - ret = env_get_f("boot_os", s, sizeof(s)); - if ((ret != -1) && (strcmp(s, "no") == 0)) + s = env_get_f("boot_os"); + if (s && (strcmp(s, "no") == 0)) return 1; /* * Check if SWUpdate recovery needs to be started * - * recovery_status = NULL (not set - ret == -1) -> normal operation + * recovery_status = NULL (not found) -> normal operation * * recovery_status = progress or * recovery_status = failed or * recovery_status = -> start SWUpdate * */ - ret = env_get_f("recovery_status", s, sizeof(s)); - if (ret != -1) + if (env_get_f("recovery_status")) return 1; return 0; diff --git a/board/socrates/socrates.c b/board/socrates/socrates.c index 1d63c81a9c8..6aefcb031c2 100644 --- a/board/socrates/socrates.c +++ b/board/socrates/socrates.c @@ -36,15 +36,15 @@ ulong flash_get_size (ulong base, int banknum); int checkboard (void) { volatile ccsr_gur_t *gur = (void *)(CFG_SYS_MPC85xx_GUTS_ADDR); - char buf[64]; + char *buf; int f; - int i = env_get_f("serial#", buf, sizeof(buf)); #ifdef CONFIG_PCI char *src; #endif puts("Board: Socrates"); - if (i > 0) { + buf = env_get_f("serial#"); + if (buf && strlen(buf)) { puts(", serial# "); puts(buf); } diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c index b31db73ebfc..3137b087db7 100644 --- a/cmd/mtdparts.c +++ b/cmd/mtdparts.c @@ -1537,9 +1537,7 @@ static const char *env_get_mtdparts(char *buf) { if (gd->flags & GD_FLG_ENV_READY) return env_get("mtdparts"); - if (env_get_f("mtdparts", buf, MTDPARTS_MAXLEN) != -1) - return buf; - return NULL; + return env_get_f("mtdparts"); } /** diff --git a/drivers/ddr/fsl/fsl_ddr_gen4.c b/drivers/ddr/fsl/fsl_ddr_gen4.c index f8d1468a26f..20d210936b0 100644 --- a/drivers/ddr/fsl/fsl_ddr_gen4.c +++ b/drivers/ddr/fsl/fsl_ddr_gen4.c @@ -73,7 +73,7 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs, u32 cs0_bnds, cs1_bnds, cs2_bnds, cs3_bnds, cs0_config; #endif #ifdef CONFIG_FSL_DDR_BIST - char buffer[CONFIG_SYS_CBSIZE]; + char *buffer; #endif #if defined(CONFIG_SYS_FSL_ERRATUM_A009942) || \ (defined(CONFIG_SYS_FSL_ERRATUM_A008378) && \ @@ -571,7 +571,8 @@ step2: #define BIST_CR_STAT 0x00000001 /* Perform build-in test on memory. Three-way interleaving is not yet * supported by this code. */ - if (env_get_f("ddr_bist", buffer, CONFIG_SYS_CBSIZE) >= 0) { + buffer = env_get_f("ddr_bist"); + if (buffer && strlen(buffer)) { puts("Running BIST test. This will take a while..."); cs0_config = ddr_in32(&ddr->cs0_config); cs0_bnds = ddr_in32(&ddr->cs0_bnds); diff --git a/drivers/ddr/fsl/interactive.c b/drivers/ddr/fsl/interactive.c index eb2f06e8300..6937074fa24 100644 --- a/drivers/ddr/fsl/interactive.c +++ b/drivers/ddr/fsl/interactive.c @@ -1877,9 +1877,7 @@ static unsigned int fsl_ddr_parse_interactive_cmd( int fsl_ddr_interactive_env_var_exists(void) { - char buffer[CONFIG_SYS_CBSIZE]; - - if (env_get_f("ddr_interactive", buffer, CONFIG_SYS_CBSIZE) >= 0) + if (env_get_f("ddr_interactive")) return 1; return 0; @@ -1890,7 +1888,7 @@ unsigned long long fsl_ddr_interactive(fsl_ddr_info_t *pinfo, int var_is_set) unsigned long long ddrsize; const char *prompt = "FSL DDR>"; char buffer[CONFIG_SYS_CBSIZE]; - char buffer2[CONFIG_SYS_CBSIZE]; + char *buffer2; char *p = NULL; char *argv[CONFIG_SYS_MAXARGS + 1]; /* NULL terminated */ int argc; @@ -1909,8 +1907,8 @@ unsigned long long fsl_ddr_interactive(fsl_ddr_info_t *pinfo, int var_is_set) }; if (var_is_set) { - if (env_get_f("ddr_interactive", buffer2, - CONFIG_SYS_CBSIZE) > 0) + buffer2 = env_get_f("ddr_interactive"); + if (buffer2 && strlen(buffer2)) p = buffer2; else var_is_set = 0; diff --git a/drivers/ddr/fsl/options.c b/drivers/ddr/fsl/options.c index 7cff8234584..83c67faf904 100644 --- a/drivers/ddr/fsl/options.c +++ b/drivers/ddr/fsl/options.c @@ -747,6 +747,9 @@ unsigned int populate_memctl_options(const common_timing_params_t *common_dimm, unsigned int ctrl_num) { unsigned int i; +#if CONFIG_IS_ENABLED(ENV_SUPPORT) + char *buffer; +#endif char buf[HWCONFIG_BUFFER_SIZE]; #if defined(CONFIG_SYS_FSL_DDR3) || \ defined(CONFIG_SYS_FSL_DDR2) || \ @@ -762,7 +765,10 @@ unsigned int populate_memctl_options(const common_timing_params_t *common_dimm, * the environment but need it for ddr config params */ #if CONFIG_IS_ENABLED(ENV_SUPPORT) - if (env_get_f("hwconfig", buf, sizeof(buf)) < 0) + buffer = env_get_f("hwconfig"); + if (buffer) + strlcpy(buf, buffer, sizeof(buf)); + else #endif buf[0] = '\0'; @@ -1411,7 +1417,11 @@ int fsl_use_spd(void) * the environment but need it for ddr config params */ #if CONFIG_IS_ENABLED(ENV_SUPPORT) - if (env_get_f("hwconfig", buf, sizeof(buf)) < 0) + char *buffer = env_get_f("hwconfig"); + + if (buffer) + strlcpy(buf, buffer, sizeof(buf)); + else #endif buf[0] = '\0'; diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 8ade7949a68..cdbb1862804 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -2408,9 +2408,9 @@ unsigned long flash_init(void) #ifdef CONFIG_SYS_FLASH_PROTECTION /* read environment from EEPROM */ - char s[64]; + char *s; - env_get_f("unlock", s, sizeof(s)); + s = env_get_f("unlock"); #endif #ifdef CONFIG_CFI_FLASH /* for driver model */ @@ -2437,7 +2437,7 @@ unsigned long flash_init(void) #endif /* CONFIG_SYS_FLASH_QUIET_TEST */ } #ifdef CONFIG_SYS_FLASH_PROTECTION - else if (strcmp(s, "yes") == 0) { + else if (s && strcmp(s, "yes") == 0) { /* * Only the U-Boot image and it's environment * is protected, all other sectors are diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c index 14ce726b10d..83613a35c22 100644 --- a/drivers/mtd/mtd_uboot.c +++ b/drivers/mtd/mtd_uboot.c @@ -132,14 +132,9 @@ static void mtd_probe_uclass_spi_nor_devs(void) { } static const char *get_mtdparts(void) { __maybe_unused const char *mtdids = NULL; - static char tmp_parts[MTDPARTS_MAXLEN]; const char *mtdparts = NULL; - if (gd->flags & GD_FLG_ENV_READY) - mtdparts = env_get("mtdparts"); - else if (env_get_f("mtdparts", tmp_parts, sizeof(tmp_parts)) != -1) - mtdparts = tmp_parts; - + mtdparts = env_get("mtdparts"); if (mtdparts) return mtdparts; diff --git a/drivers/net/fm/b4860.c b/drivers/net/fm/b4860.c index 1c5543e3c87..22e300a2bd1 100644 --- a/drivers/net/fm/b4860.c +++ b/drivers/net/fm/b4860.c @@ -49,7 +49,6 @@ phy_interface_t fman_port_enet_if(enum fm_port port) { #if defined(CONFIG_TARGET_B4860QDS) || defined(CONFIG_TARGET_B4420QDS) u32 serdes2_prtcl; - char buffer[HWCONFIG_BUFFER_SIZE]; char *buf = NULL; ccsr_gur_t *gur = (void *)(CFG_SYS_MPC85xx_GUTS_ADDR); #endif @@ -97,8 +96,7 @@ phy_interface_t fman_port_enet_if(enum fm_port port) * Extract hwconfig from environment since environment * is not setup yet */ - env_get_f("hwconfig", buffer, sizeof(buffer)); - buf = buffer; + buf = env_get_f("hwconfig"); /* check if 10GBase-R interface enable in hwconfig for 10g */ if (hwconfig_subarg_cmp_f("fsl_b4860_serdes2", diff --git a/env/common.c b/env/common.c index eb1a9137953..8cd23ecae8e 100644 --- a/env/common.c +++ b/env/common.c @@ -126,10 +126,7 @@ char *env_get(const char *name) } /* restricted capabilities before import */ - if (env_get_f(name, (char *)(gd->env_buf), sizeof(gd->env_buf)) >= 0) - return (char *)(gd->env_buf); - - return NULL; + return env_get_f(name); } /* @@ -149,48 +146,33 @@ char *from_env(const char *envvar) return ret; } -static int env_get_from_linear(const char *env, const char *name, char *buf, - unsigned len) +static char* env_get_from_linear(const char *env, const char *name) { const char *p, *end; size_t name_len; if (name == NULL || *name == '\0') - return -1; + return NULL; name_len = strlen(name); for (p = env; *p != '\0'; p = end + 1) { - const char *value; - unsigned res; - for (end = p; *end != '\0'; ++end) if (end - env >= CONFIG_ENV_SIZE) - return -1; + return NULL; if (strncmp(name, p, name_len) || p[name_len] != '=') continue; - value = &p[name_len + 1]; - - res = end - value; - memcpy(buf, value, min(len, res + 1)); - - if (len <= res) { - buf[len - 1] = '\0'; - printf("env_buf [%u bytes] too small for value of \"%s\"\n", - len, name); - } - - return res; + return &p[name_len + 1]; } - return -1; + return NULL; } /* * Look up variable from environment for restricted C runtime env. */ -int env_get_f(const char *name, char *buf, unsigned len) +char* env_get_f(const char *name) { const char *env; @@ -199,7 +181,7 @@ int env_get_f(const char *name, char *buf, unsigned len) else env = (const char *)gd->env_addr; - return env_get_from_linear(env, name, buf, len); + return env_get_from_linear(env, name); } /** @@ -246,12 +228,7 @@ bool env_get_autostart(void) */ char *env_get_default(const char *name) { - if (env_get_from_linear(default_environment, name, - (char *)(gd->env_buf), - sizeof(gd->env_buf)) >= 0) - return (char *)(gd->env_buf); - - return NULL; + return env_get_from_linear(default_environment, name); } void env_set_default(const char *s, int flags) diff --git a/include/env.h b/include/env.h index 430c4fa94a4..6915211b96c 100644 --- a/include/env.h +++ b/include/env.h @@ -123,7 +123,7 @@ char *from_env(const char *envvar); * Return: actual length of the variable value excluding the terminating * NULL-byte, or -1 if the variable is not found */ -int env_get_f(const char *name, char *buf, unsigned int len); +char* env_get_f(const char *name); /** * env_get_yesno() - Read an environment variable as a boolean diff --git a/post/post.c b/post/post.c index 946d9094d45..a3626effb78 100644 --- a/post/post.c +++ b/post/post.c @@ -176,14 +176,15 @@ static void post_get_env_flags(int *test_flags) char *var[] = { "post_poweron", "post_normal", "post_slowtest", "post_critical" }; int varnum = ARRAY_SIZE(var); - char list[128]; /* long enough for POST list */ + char *list; char *name; char *s; int last; int i, j; for (i = 0; i < varnum; i++) { - if (env_get_f(var[i], list, sizeof(list)) <= 0) + list = env_get_f(var[i]); + if (!list) continue; for (j = 0; j < post_list_size; j++)