[PATCH 1/3] filetype: return error and pass filetype as pointer argument

[PATCH 1/3] filetype: return error and pass filetype as pointer argument

[Sascha Hauer]

file_name_detect_type(), file_name_detect_type_offset() and
cdev_detect_type() return the filetype. With this all errors from these
functions remain undetected and are just returned as filetype_unknown.

Explicitly return an error code and pass the filetype as pointer
argument so that callers can detect and handle errors.

This fixes a bug in the bootm code where the returned filetype was
erroneously tested for being smaller than 0. This was never true
and so the corresponding error message was never printed. Now with
this patch a non existing initrd or device tree file is responded
with a meaningful error message.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/filetype.c |  6 +++++-
 common/binfmt.c     |  6 +++++-
 common/boot.c       |  6 +++++-
 common/bootm.c      | 21 +++++++++------------
 common/fastboot.c   |  6 +++++-
 common/filetype.c   | 27 ++++++++++++++-------------
 common/firmware.c   |  4 +++-
 fs/fs.c             |  7 ++++---
 include/filetype.h  |  6 +++---
 9 files changed, 53 insertions(+), 36 deletions(-)

diff --git a/commands/filetype.c b/commands/filetype.c
index 818c14fe79..7a556a980d 100644
--- a/commands/filetype.c
+++ b/commands/filetype.c
@@ -66,7 +66,11 @@ static int do_filetype(int argc, char *argv[])
 	if (S_ISDIR(s.st_mode))
 		return -EISDIR;
 
-	type = file_name_detect_type(filename);
+	ret = file_name_detect_type(filename, &type);
+	if (ret) {
+		printf("failed to detect type of %s: %s\n", filename, strerror(-ret));
+		return COMMAND_ERROR;
+	}
 
 	if (verbose)
 		printf("%s: %s (%s)\n", filename,
diff --git a/common/binfmt.c b/common/binfmt.c
index 1846477206..6a1e9fc83e 100644
--- a/common/binfmt.c
+++ b/common/binfmt.c
@@ -15,9 +15,13 @@ static LIST_HEAD(binfmt_hooks);
 static int binfmt_run(char *file, int argc, char **argv)
 {
 	struct binfmt_hook *b;
-	enum filetype type = file_name_detect_type(file);
+	enum filetype type;
 	int ret;
 
+	ret = file_name_detect_type(file, &type);
+	if (ret)
+		return ret;
+
 	list_for_each_entry(b, &binfmt_hooks, list) {
 		if (b->type != type)
 			continue;
diff --git a/common/boot.c b/common/boot.c
index 76bf52b529..dd9e26afc7 100644
--- a/common/boot.c
+++ b/common/boot.c
@@ -183,8 +183,12 @@ static int bootscript_create_entry(struct bootentries *bootentries, const char *
 {
 	struct bootentry_script *bs;
 	enum filetype type;
+	int ret;
+
+	ret = file_name_detect_type(name, &type);
+	if (ret)
+		return ret;
 
-	type = file_name_detect_type(name);
 	if (type != filetype_sh)
 		return -EINVAL;
 
diff --git a/common/bootm.c b/common/bootm.c
index 0e0eee1f59..9ec4b127f8 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -250,12 +250,10 @@ int bootm_load_initrd(struct image_data *data, unsigned long load_address)
 		goto done1;
 	}
 
-	type = file_name_detect_type(data->initrd_file);
-
-	if ((int)type < 0) {
-		pr_err("could not open %s: %s\n", data->initrd_file,
-				strerror(-type));
-		return (int)type;
+	ret = file_name_detect_type(data->initrd_file, &type);
+	if (ret) {
+		pr_err("could not open initrd \"%s\": %s\n", data->initrd_file, strerror(-ret));
+		return ret;
 	}
 
 	if (type == filetype_uimage) {
@@ -372,12 +370,11 @@ void *bootm_get_devicetree(struct image_data *data)
 	} else if (data->oftree_file) {
 		size_t size;
 
-		type = file_name_detect_type(data->oftree_file);
-
-		if ((int)type < 0) {
-			pr_err("could not open %s: %s\n", data->oftree_file,
-			       strerror(-type));
-			return ERR_PTR((int)type);
+		ret = file_name_detect_type(data->oftree_file, &type);
+		if (ret) {
+			pr_err("could not open device tree \"%s\": %s\n", data->oftree_file,
+			       strerror(-ret));
+			return ERR_PTR(ret);
 		}
 
 		switch (type) {
diff --git a/common/fastboot.c b/common/fastboot.c
index ae7f132444..f91f398d7a 100644
--- a/common/fastboot.c
+++ b/common/fastboot.c
@@ -624,7 +624,11 @@ static void cb_flash(struct fastboot *fb, const char *cmd)
 	const char *filename = NULL;
 	enum filetype filetype;
 
-	filetype = file_name_detect_type(fb->tempname);
+	ret = file_name_detect_type(fb->tempname, &filetype);
+	if (ret) {
+		fastboot_tx_print(fb, FASTBOOT_MSG_FAIL, "internal error");
+		goto out;
+	}
 
 	fastboot_tx_print(fb, FASTBOOT_MSG_INFO, "Copying file to %s...",
 			  cmd);
diff --git a/common/filetype.c b/common/filetype.c
index 820bc89ea6..ffc67f5698 100644
--- a/common/filetype.c
+++ b/common/filetype.c
@@ -420,15 +420,14 @@ enum filetype file_detect_type(const void *_buf, size_t bufsize)
 	return filetype_unknown;
 }
 
-enum filetype file_name_detect_type_offset(const char *filename, loff_t pos)
+int file_name_detect_type_offset(const char *filename, loff_t pos, enum filetype *type)
 {
 	int fd, ret;
 	void *buf;
-	enum filetype type = filetype_unknown;
 
 	fd = open_and_lseek(filename, O_RDONLY, pos);
 	if (fd < 0)
-		goto out;
+		return fd;
 
 	buf = xzalloc(FILE_TYPE_SAFE_BUFSIZE);
 
@@ -436,33 +435,34 @@ enum filetype file_name_detect_type_offset(const char *filename, loff_t pos)
 	if (ret < 0)
 		goto err_out;
 
-	type = file_detect_type(buf, ret);
+	*type = file_detect_type(buf, ret);
 
+	ret = 0;
 err_out:
 	close(fd);
 	free(buf);
 out:
-	return type;
+	return ret;
 }
 
-enum filetype file_name_detect_type(const char *filename)
+int file_name_detect_type(const char *filename, enum filetype *type)
 {
-	return file_name_detect_type_offset(filename, 0);
+	return file_name_detect_type_offset(filename, 0, type);
 }
 
-enum filetype cdev_detect_type(const char *name)
+int cdev_detect_type(const char *name, enum filetype *type)
 {
-	enum filetype type = filetype_unknown;
 	int ret;
 	struct cdev *cdev;
 	void *buf;
 
 	cdev = cdev_open_by_name(name, O_RDONLY);
 	if (!cdev)
-		return type;
+		return -ENOENT;
 
 	if (cdev->filetype != filetype_unknown) {
-		type = cdev->filetype;
+		*type = cdev->filetype;
+		ret = 0;
 		goto cdev_close;
 	}
 
@@ -471,13 +471,14 @@ enum filetype cdev_detect_type(const char *name)
 	if (ret < 0)
 		goto err_out;
 
-	type = file_detect_type(buf, ret);
+	*type = file_detect_type(buf, ret);
+	ret = 0;
 
 err_out:
 	free(buf);
 cdev_close:
 	cdev_close(cdev);
-	return type;
+	return ret;
 }
 
 bool filetype_is_barebox_image(enum filetype ft)
diff --git a/common/firmware.c b/common/firmware.c
index 6dc621d308..3c7960581f 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -277,7 +277,9 @@ int firmwaremgr_load_file(struct firmware_mgr *mgr, const char *firmware)
 		goto out;
 	}
 
-	type = file_name_detect_type(firmware);
+	ret = file_name_detect_type(firmware, &type);
+	if (ret)
+		goto out;
 
 	devicefd = open(dst, O_WRONLY);
 	if (devicefd < 0) {
diff --git a/fs/fs.c b/fs/fs.c
index e0ab826bca..1800d6826d 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -903,15 +903,16 @@ const char *fs_detect(const char *filename, const char *fsoptions)
 	struct fs_driver *fdrv;
 	bool loop = false;
 	unsigned long long offset = 0;
+	int ret;
 
 	parseopt_b(fsoptions, "loop", &loop);
 	parseopt_llu_suffix(fsoptions, "offset", &offset);
 	if (loop)
-		type = file_name_detect_type_offset(filename, offset);
+		ret = file_name_detect_type_offset(filename, offset, &type);
 	else
-		type = cdev_detect_type(filename);
+		ret = cdev_detect_type(filename, &type);
 
-	if (type == filetype_unknown)
+	if (ret || type == filetype_unknown)
 		return NULL;
 
 	bus_for_each_driver(&fs_bus, drv) {
diff --git a/include/filetype.h b/include/filetype.h
index 783418c652..e5aa050ebc 100644
--- a/include/filetype.h
+++ b/include/filetype.h
@@ -69,9 +69,9 @@ const char *file_type_to_string(enum filetype f);
 const char *file_type_to_short_string(enum filetype f);
 enum filetype file_detect_partition_table(const void *_buf, size_t bufsize);
 enum filetype file_detect_type(const void *_buf, size_t bufsize);
-enum filetype file_name_detect_type(const char *filename);
-enum filetype file_name_detect_type_offset(const char *filename, loff_t pos);
-enum filetype cdev_detect_type(const char *name);
+int file_name_detect_type(const char *filename, enum filetype *type);
+int file_name_detect_type_offset(const char *filename, loff_t pos, enum filetype *type);
+int cdev_detect_type(const char *name, enum filetype *type);
 enum filetype is_fat_or_mbr(const unsigned char *sector, unsigned long *bootsec);
 int is_fat_boot_sector(const void *_buf);
 bool filetype_is_barebox_image(enum filetype ft);
-- 
2.39.2

[PATCH 2/3] lib: open_and_lseek(): return error code on error

[Sascha Hauer]

Some callers of open_and_lseek() expect an error code instead of -1
as return value, so consistently return an error code.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 lib/libfile.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/lib/libfile.c b/lib/libfile.c
index ebd1de3d8e..c7ea4e497f 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -589,7 +589,7 @@ int compare_file(const char *f1, const char *f2)
  * @pos:	The position to lseek to
  *
  * Return: If successful this function returns a positive
- *         filedescriptor number, otherwise -1 is returned
+ *         filedescriptor number, otherwise a negative error code is returned
  */
 int open_and_lseek(const char *filename, int mode, loff_t pos)
 {
@@ -607,26 +607,30 @@ int open_and_lseek(const char *filename, int mode, loff_t pos)
 	if (mode & (O_WRONLY | O_RDWR)) {
 		struct stat s;
 
-		if (fstat(fd, &s)) {
+		ret = fstat(fd, &s);
+		if (ret < 0) {
 			perror("fstat");
 			goto out;
 		}
 
-		if (s.st_size < pos && ftruncate(fd, pos)) {
-			perror("ftruncate");
-			goto out;
-		}
+		if (s.st_size < pos) {
+			ret = ftruncate(fd, pos));
+			if (ret) {
+				perror("ftruncate");
+				goto out;
+			}
 	}
 
 	if (lseek(fd, pos, SEEK_SET) != pos) {
 		perror("lseek");
+		ret = -errno;
 		goto out;
 	}
 
 	return fd;
 out:
 	close(fd);
-	return -1;
+	return ret;
 }
 
 /**
-- 
2.39.2

[PATCH 3/3] lib: open_and_lseek(): move error messages to callers

[Sascha Hauer]

For some cases like in common/filetype.c the caller already prints an
error message, so to avoid duplicated error messages leave it up to
the caller to print an error. This also adds error messages to all
callers where necessary.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/mips/mach-ath79/art.c |  3 +--
 commands/loadb.c           |  2 +-
 commands/loadxy.c          |  2 +-
 commands/md.c              |  4 +++-
 commands/memset.c          |  4 +++-
 commands/mm.c              |  4 +++-
 commands/mw.c              |  4 +++-
 common/ratp/md.c           |  4 +++-
 common/ratp/mw.c           |  1 +
 lib/libfile.c              | 13 +++----------
 lib/misc.c                 |  5 ++++-
 11 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/arch/mips/mach-ath79/art.c b/arch/mips/mach-ath79/art.c
index 585cccc5af..bdc14bee51 100644
--- a/arch/mips/mach-ath79/art.c
+++ b/arch/mips/mach-ath79/art.c
@@ -43,8 +43,7 @@ static int art_read_mac(struct device *dev, const char *file)
 
 	fd = open_and_lseek(file, O_RDONLY, AR93000_EPPROM_OFFSET);
 	if (fd < 0) {
-		dev_err(dev, "Failed to open eeprom path %s %d\n",
-		       file, -errno);
+		dev_err(dev, "Failed to open eeprom path \"%s\": %m\n", file);
 		return -errno;
 	}
 
diff --git a/commands/loadb.c b/commands/loadb.c
index 7ab989f459..140d3743f6 100644
--- a/commands/loadb.c
+++ b/commands/loadb.c
@@ -646,7 +646,7 @@ static int do_load_serial_bin(int argc, char *argv[])
 	/* File should exist */
 	ofd = open_and_lseek(output_file, O_WRONLY | O_CREAT, offset);
 	if (ofd < 0) {
-		perror(argv[0]);
+		printf("Could not open \"%s\": %m\n", output_file);
 		return 3;
 	}
 
diff --git a/commands/loadxy.c b/commands/loadxy.c
index 66daa117d9..e2d1a11a2c 100644
--- a/commands/loadxy.c
+++ b/commands/loadxy.c
@@ -165,7 +165,7 @@ static int do_loadx(int argc, char *argv[])
 	/* File should exist */
 	ofd = open_and_lseek(output_file, O_WRONLY | O_CREAT, offset);
 	if (ofd < 0) {
-		perror(argv[0]);
+		printf("Could not open \"%s\": %m\n", output_file);
 		return 3;
 	}
 
diff --git a/commands/md.c b/commands/md.c
index 7a96634e27..f3758f571f 100644
--- a/commands/md.c
+++ b/commands/md.c
@@ -49,8 +49,10 @@ static int do_mem_md(int argc, char *argv[])
 	}
 
 	fd = open_and_lseek(filename, mode | O_RDONLY, start);
-	if (fd < 0)
+	if (fd < 0) {
+		printf("Could not open \"%s\": %m\n", filename);
 		return 1;
+	}
 
 	map = memmap(fd, PROT_READ);
 	if (map != MAP_FAILED) {
diff --git a/commands/memset.c b/commands/memset.c
index e4412533f1..1139691f2f 100644
--- a/commands/memset.c
+++ b/commands/memset.c
@@ -41,8 +41,10 @@ static int do_memset(int argc, char *argv[])
 	n = strtoull_suffix(argv[optind + 2], NULL, 0);
 
 	fd = open_and_lseek(file, mode | O_WRONLY | O_CREAT, s);
-	if (fd < 0)
+	if (fd < 0) {
+		printf("Could not open \"%s\": %m\n", file);
 		return 1;
+	}
 
 	buf = xmalloc(RW_BUF_SIZE);
 	memset(buf, c, RW_BUF_SIZE);
diff --git a/commands/mm.c b/commands/mm.c
index 8fe87a80a1..8755a0f2c9 100644
--- a/commands/mm.c
+++ b/commands/mm.c
@@ -40,8 +40,10 @@ static int do_mem_mm(int argc, char *argv[])
 	mask = simple_strtoull(argv[optind++], NULL, 0);
 
 	fd = open_and_lseek(filename, mode | O_RDWR | O_CREAT, adr);
-	if (fd < 0)
+	if (fd < 0) {
+		printf("Could not open \"%s\": %m\n", filename);
 		return 1;
+	}
 
 	switch (mode) {
 	case O_RWSIZE_1:
diff --git a/commands/mw.c b/commands/mw.c
index 5dcef7e2fc..915f549216 100644
--- a/commands/mw.c
+++ b/commands/mw.c
@@ -39,8 +39,10 @@ static int do_mem_mw(int argc, char *argv[])
 	adr = strtoull_suffix(argv[optind++], NULL, 0);
 
 	fd = open_and_lseek(filename, mode | O_WRONLY | O_CREAT, adr);
-	if (fd < 0)
+	if (fd < 0) {
+		printf("Could not open \"%s\": %m\n", filename);
 		return 1;
+	}
 
 	while (optind < argc) {
 		u8 val8;
diff --git a/common/ratp/md.c b/common/ratp/md.c
index 3e258c59a0..8221afaebc 100644
--- a/common/ratp/md.c
+++ b/common/ratp/md.c
@@ -68,8 +68,10 @@ static int do_ratp_mem_md(const char *filename,
 	char *buf = NULL;
 
 	fd = open_and_lseek(filename, O_RWSIZE_1 | O_RDONLY, start);
-	if (fd < 0)
+	if (fd < 0) {
+		printf("Could not open \"%s\": %m\n", filename);
 		return -errno;
+	}
 
 	map = memmap(fd, PROT_READ);
 	if (map != MAP_FAILED) {
diff --git a/common/ratp/mw.c b/common/ratp/mw.c
index 8945799f1d..87dc8cb95c 100644
--- a/common/ratp/mw.c
+++ b/common/ratp/mw.c
@@ -133,6 +133,7 @@ static int ratp_cmd_mw(const struct ratp_bb *req, int req_len,
 
 	fd = open_and_lseek(path, O_RWSIZE_1 | O_WRONLY, addr);
 	if (fd < 0) {
+		printf("Could not open \"%s\": %m\n", path);
 		ret = -errno;
 		goto out;
 	}
diff --git a/lib/libfile.c b/lib/libfile.c
index c7ea4e497f..a8654e6deb 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -596,10 +596,8 @@ int open_and_lseek(const char *filename, int mode, loff_t pos)
 	int fd;
 
 	fd = open(filename, mode);
-	if (fd < 0) {
-		perror("open");
+	if (fd < 0)
 		return fd;
-	}
 
 	if (!pos)
 		return fd;
@@ -608,21 +606,16 @@ int open_and_lseek(const char *filename, int mode, loff_t pos)
 		struct stat s;
 
 		ret = fstat(fd, &s);
-		if (ret < 0) {
-			perror("fstat");
+		if (ret < 0)
 			goto out;
-		}
 
 		if (s.st_size < pos) {
 			ret = ftruncate(fd, pos));
-			if (ret) {
-				perror("ftruncate");
+			if (ret)
 				goto out;
-			}
 	}
 
 	if (lseek(fd, pos, SEEK_SET) != pos) {
-		perror("lseek");
 		ret = -errno;
 		goto out;
 	}
diff --git a/lib/misc.c b/lib/misc.c
index 2d5f7c1985..1cb2a6b9b5 100644
--- a/lib/misc.c
+++ b/lib/misc.c
@@ -209,11 +209,14 @@ int memcpy_parse_options(int argc, char *argv[], int *sourcefd,
 	destfile = destfile ?: "/dev/mem";
 
 	*sourcefd = open_and_lseek(sourcefile, mode | O_RDONLY, src);
-	if (*sourcefd < 0)
+	if (*sourcefd < 0) {
+		printf("Could not open source file \"%s\": %m\n", sourcefile);
 		return -1;
+	}
 
 	*destfd = open_and_lseek(destfile, mode | destmode, dest);
 	if (*destfd < 0) {
+		printf("Could not open destination file \"%s\": %m\n", destfile);
 		close(*sourcefd);
 		return -1;
 	}
-- 
2.39.2