[PATCH v4 07/10] cmd: ubi: reorganize command messages

Weijie Gao weijie.gao at mediatek.com
Wed May 13 10:02:58 CEST 2026


This patch moves normal subcommand messages into the main command function.
This will allow current and potential api functions being called with clean
output on success.

A new function ubi_require_volume() is added for finding and printing error
message if volume not found. The original ubi_find_volume() will be silent
for being an api function.

To avoid ubi_require_volume() being called twice for volume read/remove,
some changes are required:
- The parameter of ubi_remove_vol() is changed to accept
'struct ubi_volume *' directly.
- The original ubi_volume_read() is renamed to __ubi_volume_read, with its
  first parameter changed to accept also 'struct ubi_volume *' directly.
- A new ubi_volume_read() is added to wraps __ubi_volume_read() to accept
  volume name as its first parameter.

Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
---
v4: Make error message always be printed in both command and api calls.
    Avoid duplicated call to ubi_require_volume().
    Add parameter check for "ubi rename" subcommand.
v3: Fix use-after-free and "ubi read" command message displaying error
v2: new
---
 cmd/ubi.c | 118 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 80 insertions(+), 38 deletions(-)

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 23b846f22ce..b0b513cfe53 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -241,8 +241,7 @@ static int ubi_create_vol(const char *volume, int64_t size, bool dynamic,
 		printf("verify_mkvol_req failed %d\n", err);
 		return err;
 	}
-	printf("Creating %s volume %s of size %lld\n",
-		dynamic ? "dynamic" : "static", volume, size);
+
 	/* Call real ubi create volume */
 	return ubi_create_volume(ubi, &req);
 }
@@ -258,20 +257,22 @@ static struct ubi_volume *ubi_find_volume(const char *volume)
 			return vol;
 	}
 
-	printf("Volume %s not found!\n", volume);
 	return NULL;
 }
 
-static int ubi_remove_vol(const char *volume)
+static struct ubi_volume *ubi_require_volume(const char *volume)
 {
-	int err, reserved_pebs, i;
-	struct ubi_volume *vol;
+	struct ubi_volume *vol = ubi_find_volume(volume);
 
-	vol = ubi_find_volume(volume);
-	if (vol == NULL)
-		return -ENODEV;
+	if (!vol)
+		printf("Volume %s not found!\n", volume);
+
+	return vol;
+}
 
-	printf("Remove UBI volume %s (id %d)\n", vol->name, vol->vol_id);
+static int ubi_remove_vol(struct ubi_volume *vol)
+{
+	int err, reserved_pebs, i;
 
 	if (ubi->ro_mode) {
 		printf("It's read-only mode\n");
@@ -310,7 +311,7 @@ static int ubi_remove_vol(const char *volume)
 
 	return 0;
 out_err:
-	ubi_err(ubi, "cannot remove volume %s, error %d", volume, err);
+	ubi_err(ubi, "cannot remove volume %s, error %d", vol->name, err);
 	return err;
 }
 
@@ -321,19 +322,15 @@ static int ubi_rename_vol(const char *oldname, const char *newname)
 	struct ubi_volume_desc desc;
 	struct list_head list;
 
-	vol = ubi_find_volume(oldname);
-	if (!vol) {
-		printf("%s: volume %s doesn't exist\n", __func__, oldname);
+	vol = ubi_require_volume(oldname);
+	if (!vol)
 		return -ENODEV;
-	}
 
 	if (!ubi_check(newname)) {
 		printf("%s: volume %s already exist\n", __func__, newname);
 		return -EINVAL;
 	}
 
-	printf("Rename UBI volume %s to %s\n", oldname, newname);
-
 	if (ubi->ro_mode) {
 		printf("%s: ubi device is in read-only mode\n", __func__);
 		return -EROFS;
@@ -358,7 +355,7 @@ static int ubi_volume_continue_write(const char *volume, const void *buf,
 	int err;
 	struct ubi_volume *vol;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (vol == NULL)
 		return -ENODEV;
 
@@ -400,7 +397,7 @@ int ubi_volume_begin_write(const char *volume, const void *buf, size_t size,
 	int rsvd_bytes;
 	struct ubi_volume *vol;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (vol == NULL)
 		return -ENODEV;
 
@@ -432,7 +429,7 @@ static int ubi_volume_offset_write(const char *volume, const void *buf,
 	loff_t off = offset;
 	void *tbuf;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (!vol)
 		return -ENODEV;
 
@@ -503,19 +500,15 @@ int ubi_volume_write(const char *volume, const void *buf, loff_t offset,
 	return ret;
 }
 
-int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
+static int __ubi_volume_read(struct ubi_volume *vol, void *buf, loff_t offset,
+			     size_t size)
 {
 	int err, lnum, off, len, tbuf_size;
 	void *tbuf;
 	unsigned long long tmp;
-	struct ubi_volume *vol;
 	loff_t offp = offset;
 	size_t len_read;
 
-	vol = ubi_find_volume(volume);
-	if (vol == NULL)
-		return -ENODEV;
-
 	if (vol->updating) {
 		printf("updating");
 		return -EBUSY;
@@ -527,12 +520,8 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 	if (offp == vol->used_bytes)
 		return 0;
 
-	if (size == 0) {
-		printf("No size specified -> Using max size (%lld)\n", vol->used_bytes);
+	if (size == 0)
 		size = vol->used_bytes;
-	}
-
-	printf("Read %zu bytes from volume %s to %p\n", size, volume, buf);
 
 	if (vol->corrupted)
 		printf("read from corrupted volume %d", vol->vol_id);
@@ -586,6 +575,17 @@ int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
 	return err;
 }
 
+int ubi_volume_read(const char *volume, void *buf, loff_t offset, size_t size)
+{
+	struct ubi_volume *vol;
+
+	vol = ubi_require_volume(volume);
+	if (!vol)
+		return -ENODEV;
+
+	return __ubi_volume_read(vol, buf, offset, size);
+}
+
 static int ubi_dev_scan(const struct mtd_info *info,
 			const char *vid_header_offset)
 {
@@ -616,13 +616,10 @@ static int ubi_set_skip_check(const char *volume, bool skip_check)
 	struct ubi_vtbl_record vtbl_rec;
 	struct ubi_volume *vol;
 
-	vol = ubi_find_volume(volume);
+	vol = ubi_require_volume(volume);
 	if (!vol)
 		return -ENODEV;
 
-	printf("%sing skip_check on volume %s\n",
-	       skip_check ? "Sett" : "Clear", volume);
-
 	vtbl_rec = ubi->vtbl[vol->vol_id];
 	if (skip_check) {
 		vtbl_rec.flags |= UBI_VTBL_SKIP_CRC_CHECK_FLG;
@@ -695,6 +692,7 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	int64_t size;
 	ulong addr = 0;
 	bool skipcheck = false;
+	struct ubi_volume *vol;
 	int ret;
 
 	if (argc < 2)
@@ -803,7 +801,9 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		/* Use maximum available size */
 		if (!size) {
 			size = (int64_t)ubi->avail_pebs * ubi->leb_size;
-			printf("No size specified -> Using max size (%lld)\n", size);
+			if (size)
+				printf("No size specified -> Using max size (%lld)\n",
+				       size);
 		}
 		/* E.g., create volume */
 		if (argc == 3) {
@@ -811,6 +811,10 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 					     skipcheck);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("Created %s volume %s of size %lld\n",
+			       dynamic ? "dynamic" : "static", argv[2], size);
+
 			return 0;
 		}
 	}
@@ -818,17 +822,37 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	if (strncmp(argv[1], "remove", 6) == 0) {
 		/* E.g., remove volume */
 		if (argc == 3) {
-			ret = ubi_remove_vol(argv[2]);
+			int vol_id;
+
+			vol = ubi_require_volume(argv[2]);
+			if (!vol)
+				return CMD_RET_FAILURE;
+
+			vol_id = vol->vol_id;
+
+			ret = ubi_remove_vol(vol);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("Removed UBI volume %s (id %d)\n", argv[2],
+			       vol_id);
+
 			return 0;
 		}
 	}
 
 	if (IS_ENABLED(CONFIG_CMD_UBI_RENAME) && !strncmp(argv[1], "rename", 6)) {
+		if (argc < 4) {
+			printf("Please see usage\n");
+			return CMD_RET_USAGE;
+		}
+
 		ret = ubi_rename_vol(argv[2], argv[3]);
 		if (ret)
 			return CMD_RET_FAILURE;
+
+		printf("UBI volume %s renamed to %s\n", argv[2], argv[3]);
+
 		return 0;
 	}
 
@@ -839,6 +863,10 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 			ret = ubi_set_skip_check(argv[2], skipcheck);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("%s skip_check on volume %s\n",
+			       skipcheck ? "Set" : "Cleared", argv[2]);
+
 			return 0;
 		}
 	}
@@ -892,9 +920,23 @@ static int do_ubi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		}
 
 		if (argc == 3) {
-			ret = ubi_volume_read(argv[3], (void *)addr, 0, size);
+			vol = ubi_require_volume(argv[3]);
+			if (!vol)
+				return CMD_RET_FAILURE;
+
+			if (!size) {
+				printf("No size specified -> Using max size (%lld)\n",
+				       vol->used_bytes);
+				size = vol->used_bytes;
+			}
+
+			ret = __ubi_volume_read(vol, (void *)addr, 0, size);
 			if (ret)
 				return CMD_RET_FAILURE;
+
+			printf("%lld bytes read from volume %s to 0x%lx\n",
+			       size, argv[3], addr);
+
 			return 0;
 		}
 	}
-- 
2.45.2



More information about the U-Boot mailing list