diff mbox series

[v2,03/12] bootm: Update fixup_silent_linux() to return an error

Message ID 20201105173349.903603-4-sjg@chromium.org
State Accepted
Commit 4ae42643d0d71dbb5af45d19fa05b7a6807150c0
Delegated to: Tom Rini
Headers show
Series bootm: Support substitions in bootargs and add tests | expand

Commit Message

Simon Glass Nov. 5, 2020, 5:33 p.m. UTC
At present this function fails silently on error. Update it to produce
an error code. Report this error to the user and abort the boot, since it
likely will prevent a successful start.

No tests are added at this stage, since additional refactoring is taking
place in subsequent patches.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/bootm.c  | 22 +++++++++++++++-------
 include/bootm.h | 11 +++++++++--
 test/bootm.c    | 10 +++++-----
 3 files changed, 29 insertions(+), 14 deletions(-)

Comments

Tom Rini Dec. 7, 2020, 10:18 p.m. UTC | #1
On Thu, Nov 05, 2020 at 10:33:39AM -0700, Simon Glass wrote:

> At present this function fails silently on error. Update it to produce
> an error code. Report this error to the user and abort the boot, since it
> likely will prevent a successful start.
> 
> No tests are added at this stage, since additional refactoring is taking
> place in subsequent patches.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/common/bootm.c b/common/bootm.c
index 0d36c572101..950ff7cff62 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -468,7 +468,7 @@  ulong bootm_disable_interrupts(void)
 #define CONSOLE_ARG     "console="
 #define CONSOLE_ARG_LEN (sizeof(CONSOLE_ARG) - 1)
 
-void fixup_silent_linux(void)
+int fixup_silent_linux(void)
 {
 	char *buf;
 	const char *env_val;
@@ -477,7 +477,7 @@  void fixup_silent_linux(void)
 
 	if (!IS_ENABLED(CONFIG_SILENT_CONSOLE) &&
 	    !IS_ENABLED(CONFIG_SILENT_U_BOOT_ONLY))
-		return;
+		return 0;
 	cmdline = env_get("bootargs");
 
 	/*
@@ -489,9 +489,9 @@  void fixup_silent_linux(void)
 	 */
 	want_silent = env_get_yesno("silent_linux");
 	if (want_silent == 0)
-		return;
+		return 0;
 	else if (want_silent == -1 && !(gd->flags & GD_FLG_SILENT))
-		return;
+		return 0;
 
 	debug("before silent fix-up: %s\n", cmdline);
 	if (cmdline && (cmdline[0] != '\0')) {
@@ -501,7 +501,7 @@  void fixup_silent_linux(void)
 		buf = malloc(strlen(cmdline) + 1 + CONSOLE_ARG_LEN + 1);
 		if (!buf) {
 			debug("%s: out of memory\n", __func__);
-			return;
+			return -ENOSPC;
 		}
 
 		if (start) {
@@ -525,6 +525,8 @@  void fixup_silent_linux(void)
 	env_set("bootargs", env_val);
 	debug("after silent fix-up: %s\n", env_val);
 	free(buf);
+
+	return 0;
 }
 
 /**
@@ -629,8 +631,14 @@  int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (!ret && (states & BOOTM_STATE_OS_BD_T))
 		ret = boot_fn(BOOTM_STATE_OS_BD_T, argc, argv, images);
 	if (!ret && (states & BOOTM_STATE_OS_PREP)) {
-		if (images->os.os == IH_OS_LINUX)
-			fixup_silent_linux();
+		if (images->os.os == IH_OS_LINUX) {
+			ret = fixup_silent_linux();
+			if (ret) {
+				printf("Cmdline setup failed (err=%d)\n", ret);
+				ret = CMD_RET_FAILURE;
+				goto err;
+			}
+		}
 		ret = boot_fn(BOOTM_STATE_OS_PREP, argc, argv, images);
 	}
 
diff --git a/include/bootm.h b/include/bootm.h
index 6d675e64559..438829af0fe 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -85,7 +85,14 @@  void arch_preboot_os(void);
  */
 void board_preboot_os(void);
 
-/* Adjust the 'bootargs' to ensure that Linux boots silently, if required */
-void fixup_silent_linux(void);
+/*
+ * fixup_silent_linux() - Process fix-ups for the command line
+ *
+ * Updates the 'bootargs' envvar as required. This handles making Linux boot
+ * silently if requested ('silent_linux' envvar)
+ *
+ * @return 0 if OK, -ENOMEM if out of memory
+ */
+int fixup_silent_linux(void);
 
 #endif
diff --git a/test/bootm.c b/test/bootm.c
index 59d16cb3df6..ab1711609ba 100644
--- a/test/bootm.c
+++ b/test/bootm.c
@@ -23,26 +23,26 @@  static int bootm_test_silent_var(struct unit_test_state *uts)
 	/* 'silent_linux' not set should do nothing */
 	env_set("silent_linux", NULL);
 	env_set("bootargs", CONSOLE_STR);
-	fixup_silent_linux();
+	ut_assertok(fixup_silent_linux());
 	ut_asserteq_str(CONSOLE_STR, env_get("bootargs"));
 
 	env_set("bootargs", NULL);
-	fixup_silent_linux();
+	ut_assertok(fixup_silent_linux());
 	ut_assertnull(env_get("bootargs"));
 
 	ut_assertok(env_set("silent_linux", "no"));
 	env_set("bootargs", CONSOLE_STR);
-	fixup_silent_linux();
+	ut_assertok(fixup_silent_linux());
 	ut_asserteq_str(CONSOLE_STR, env_get("bootargs"));
 
 	ut_assertok(env_set("silent_linux", "yes"));
 	env_set("bootargs", CONSOLE_STR);
-	fixup_silent_linux();
+	ut_assertok(fixup_silent_linux());
 	ut_asserteq_str("console=", env_get("bootargs"));
 
 	/* Empty buffer should still add the string */
 	env_set("bootargs", NULL);
-	fixup_silent_linux();
+	ut_assertok(fixup_silent_linux());
 	ut_asserteq_str("console=", env_get("bootargs"));
 
 	return 0;