[PATCH v2 0/8] Make TFTP detection more convenient

[PATCH v2 0/8] Make TFTP detection more convenient

[Sascha Hauer]

We can't lseek on files provided by TFTP, so some code copies the file
to operate on from TFTP to RAM before continuing. Make this a bit easier
by providing some helper code. We now create /tmp/, provide make_temp()
to create temporary files and also a single function to create a copy
of a file in /tmp/.

Changes since v1:
- Integrated changes suggested by Stefan Lengfeld

Sascha Hauer (8):
  startup: create /tmp
  fs: implement is_tftp_fs()
  libfile: implement make_temp
  libfile: implement a function to cache a file
  uimage: fix memory leak in error path
  uimage: Use is_tftp_fs() and cache_file() to ease TFTP workaround
  fs/uimagefs: Use is_tftp_fs() and cache_file() to ease TFTP workaround
  fs: remove now unused function can_lseek_backward()

 common/startup.c   |  1 +
 common/uimage.c    | 49 ++++++++++++++++++++++++------------------------
 fs/fs.c            | 22 ++++++++++++++++++++++
 fs/uimagefs.c      | 37 +++++++++++++-----------------------
 include/fs.h       | 22 ++++++----------------
 include/image.h    |  1 +
 include/libfile.h  |  4 ++++
 include/uimagefs.h |  2 +-
 lib/libfile.c      | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 127 insertions(+), 66 deletions(-)

-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/8] startup: create /tmp

[Sascha Hauer]

Some commands create temporary files in /. Create /tmp to offer
these commands an appropriate place for storing temporary files.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/startup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/startup.c b/common/startup.c
index 432be67cd6..8553849cb3 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -54,6 +54,7 @@ static int mount_root(void)
 {
 	mount("none", "ramfs", "/", NULL);
 	mkdir("/dev", 0);
+	mkdir("/tmp", 0);
 	mount("none", "devfs", "/dev", NULL);
 
 	if (IS_ENABLED(CONFIG_FS_EFIVARFS)) {
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/8] fs: implement is_tftp_fs()

[Sascha Hauer]

Some commands need files in which they can lseek backwards which
is particularly not possible on TFTP. Instead of hiding this
behind can_lseek_backward() create a function for it which tests
if the file is on TFTP directly rather than using different
lseek operations.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c      | 22 ++++++++++++++++++++++
 include/fs.h | 10 ++++++++++
 2 files changed, 32 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 6f15e93ba9..e073e3577d 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -1906,3 +1906,25 @@ char *path_get_linux_rootarg(const char *path)
 
 	return xstrdup(str);
 }
+
+/**
+ * __is_tftp_fs() - return true when path is mounted on TFTP
+ * @path: The path
+ *
+ * Do not use directly, use is_tftp_fs instead.
+ *
+ * Return: true when @path is on TFTP, false otherwise
+ */
+bool __is_tftp_fs(const char *path)
+{
+	struct fs_device_d *fsdev;
+
+	fsdev = get_fsdevice_by_path(path);
+	if (!fsdev)
+		return false;
+
+	if (strcmp(fsdev->driver->drv.name, "tftp"))
+		return false;
+
+	return true;
+}
diff --git a/include/fs.h b/include/fs.h
index 3d88bfad4a..d75a1ef644 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -121,6 +121,16 @@ static inline int can_lseek_backward(int fd)
 	return 1;
 }
 
+bool __is_tftp_fs(const char *path);
+
+static inline bool is_tftp_fs(const char *path)
+{
+	if (!IS_ENABLED(CONFIG_FS_TFTP))
+		return false;
+
+	return __is_tftp_fs(path);
+}
+
 #define drv_to_fs_driver(d) container_of(d, struct fs_driver_d, drv)
 
 int flush(int fd);
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/8] libfile: implement make_temp

[Sascha Hauer]

Create a make_temp() function which creates a name for a temporary file.
Since we do not have any concurrency in barebox we do not need to create
the file right away and can leave that to the caller. Unlike unix
mktemp the resulting filename is dynamically allocated and must be
freed by the caller.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/libfile.h |  2 ++
 lib/libfile.c     | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/libfile.h b/include/libfile.h
index dd0b00f988..6dbb81a241 100644
--- a/include/libfile.h
+++ b/include/libfile.h
@@ -26,4 +26,6 @@ int make_directory(const char *pathname);
 
 int unlink_recursive(const char *path, char **failedpath);
 
+char *make_temp(const char *template);
+
 #endif /* __LIBFILE_H */
diff --git a/lib/libfile.c b/lib/libfile.c
index 6b70306dbd..79054eb5ac 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -20,6 +20,7 @@
 #include <malloc.h>
 #include <libfile.h>
 #include <progress.h>
+#include <stdlib.h>
 #include <linux/stat.h>
 
 /*
@@ -485,3 +486,29 @@ int open_and_lseek(const char *filename, int mode, loff_t pos)
 
 	return fd;
 }
+
+/**
+ * make_temp - create a name for a temporary file
+ * @template:	The filename prefix
+ *
+ * This function creates a name for a temporary file. @template is used as a
+ * template for the name which gets appended a 8-digit hexadecimal number to
+ * create a unique filename.
+ *
+ * Return: This function returns a filename which can be used as a temporary
+ *         file lateron. The returned filename must be freed by the caller.
+ */
+char *make_temp(const char *template)
+{
+	char *name = NULL;
+	struct stat s;
+	int ret;
+
+	do {
+		free(name);
+		name = basprintf("/tmp/%s-%08x", template, random32());
+		ret = stat(name, &s);
+	} while (!ret);
+
+	return name;
+}
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/8] libfile: implement a function to cache a file

[Sascha Hauer]

Due to the nature of TFTP which can't lseek and due to the silliness
of our filesystem implementation which can't cache accesses we have to
manually cache files on TFTP filesystems sometimes. Make it easier
for them by providing a cache_file() function which copies the file
from TFTP to RAM.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/libfile.h |  2 ++
 lib/libfile.c     | 30 +++++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/include/libfile.h b/include/libfile.h
index 6dbb81a241..beec7cff79 100644
--- a/include/libfile.h
+++ b/include/libfile.h
@@ -28,4 +28,6 @@ int unlink_recursive(const char *path, char **failedpath);
 
 char *make_temp(const char *template);
 
+int cache_file(const char *path, char **newpath);
+
 #endif /* __LIBFILE_H */
diff --git a/lib/libfile.c b/lib/libfile.c
index 79054eb5ac..6dce5cbfee 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -496,7 +496,7 @@ int open_and_lseek(const char *filename, int mode, loff_t pos)
  * create a unique filename.
  *
  * Return: This function returns a filename which can be used as a temporary
- *         file lateron. The returned filename must be freed by the caller.
+ *         file later on. The returned filename must be freed by the caller.
  */
 char *make_temp(const char *template)
 {
@@ -512,3 +512,31 @@ char *make_temp(const char *template)
 
 	return name;
 }
+
+/**
+ * cache_file - Cache a file in /tmp
+ * @path:	The file to cache
+ * @newpath:	The return path where the file is copied to
+ *
+ * This function copies a given file to /tmp and returns its name in @newpath.
+ * @newpath is dynamically allocated and must be freed by the caller.
+ *
+ * Return: 0 for success, negative error code otherwise.
+ */
+int cache_file(const char *path, char **newpath)
+{
+	char *npath;
+	int ret;
+
+	npath = make_temp("filecache");
+
+	ret = copy_file(path, npath, 0);
+	if (ret) {
+		free(npath);
+		return ret;
+	}
+
+	*newpath = npath;
+
+	return 0;
+}
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/8] uimage: fix memory leak in error path

[Sascha Hauer]

handle->name is dynamically allocated, so free it in the error path.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Reported-by: Stefan Lengfeld <contact@stefanchrist.eu>
---
 common/uimage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/uimage.c b/common/uimage.c
index b6f0f109ca..e857c11d37 100644
--- a/common/uimage.c
+++ b/common/uimage.c
@@ -202,6 +202,8 @@ again:
 	return handle;
 err_out:
 	close(fd);
+
+	free(handle->name);
 	free(handle);
 	if (IS_BUILTIN(CONFIG_FS_TFTP) && !stat(uimage_tmp, &s))
 		unlink(uimage_tmp);
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/8] uimage: Use is_tftp_fs() and cache_file() to ease TFTP workaround

[Sascha Hauer]

We have helper functions now to ease file caching when a file
is on TFTP. Use them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/uimage.c | 47 ++++++++++++++++++++++-------------------------
 include/image.h |  1 +
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/common/uimage.c b/common/uimage.c
index e857c11d37..3273bc1871 100644
--- a/common/uimage.c
+++ b/common/uimage.c
@@ -85,8 +85,6 @@ ssize_t uimage_get_size(struct uimage_handle *handle, unsigned int image_no)
 }
 EXPORT_SYMBOL(uimage_get_size);
 
-static const char uimage_tmp[] = "/.uImage_tmp";
-
 /*
  * open a uimage. This will check the header contents and
  * return a handle to the uImage
@@ -99,32 +97,27 @@ struct uimage_handle *uimage_open(const char *filename)
 	struct image_header *header;
 	int i;
 	int ret;
-	struct stat s;
+	char *copy = NULL;
+
+	if (is_tftp_fs(filename)) {
+		ret = cache_file(filename, &copy);
+		if (ret)
+			return NULL;
+		filename = copy;
+	}
 
-again:
 	fd = open(filename, O_RDONLY);
 	if (fd < 0) {
 		printf("could not open: %s\n", errno_str());
+		free(copy);
 		return NULL;
 	}
 
-	/*
-	 * Hack around tftp fs. We need lseek for uImage support, but
-	 * this cannot be implemented in tftp fs, so we detect this
-	 * and copy the file to ram if it fails
-	 */
-	if (IS_BUILTIN(CONFIG_FS_TFTP) && !can_lseek_backward(fd)) {
-		close(fd);
-		ret = copy_file(filename, uimage_tmp, 0);
-		if (ret)
-			return NULL;
-		filename = uimage_tmp;
-		goto again;
-	}
-
 	handle = xzalloc(sizeof(struct uimage_handle));
 	header = &handle->header;
 
+	handle->copy = copy;
+
 	if (read(fd, header, sizeof(*header)) < 0) {
 		printf("could not read: %s\n", errno_str());
 		goto err_out;
@@ -204,9 +197,12 @@ err_out:
 	close(fd);
 
 	free(handle->name);
+	if (handle->copy) {
+		unlink(handle->copy);
+		free(handle->copy);
+	}
 	free(handle);
-	if (IS_BUILTIN(CONFIG_FS_TFTP) && !stat(uimage_tmp, &s))
-		unlink(uimage_tmp);
+
 	return NULL;
 }
 EXPORT_SYMBOL(uimage_open);
@@ -216,14 +212,15 @@ EXPORT_SYMBOL(uimage_open);
  */
 void uimage_close(struct uimage_handle *handle)
 {
-	struct stat s;
-
 	close(handle->fd);
+
+	if (handle->copy) {
+		unlink(handle->copy);
+		free(handle->copy);
+	}
+
 	free(handle->name);
 	free(handle);
-
-	if (IS_BUILTIN(CONFIG_FS_TFTP) && !stat(uimage_tmp, &s))
-		unlink(uimage_tmp);
 }
 EXPORT_SYMBOL(uimage_close);
 
diff --git a/include/image.h b/include/image.h
index 3e75d49b88..add9c85874 100644
--- a/include/image.h
+++ b/include/image.h
@@ -246,6 +246,7 @@ struct resource *file_to_sdram(const char *filename, unsigned long adr);
 struct uimage_handle {
 	struct image_header header;
 	char *name;
+	char *copy;
 	struct uimage_handle_data ihd[MAX_MULTI_IMAGE_COUNT];
 	int nb_data_entries;
 	size_t data_offset;
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 7/8] fs/uimagefs: Use is_tftp_fs() and cache_file() to ease TFTP workaround

[Sascha Hauer]

We have helper functions now to ease file caching when a file
is on TFTP. Use them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/uimagefs.c      | 37 +++++++++++++------------------------
 include/uimagefs.h |  2 +-
 2 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/fs/uimagefs.c b/fs/uimagefs.c
index c0c5750c2c..c120944a46 100644
--- a/fs/uimagefs.c
+++ b/fs/uimagefs.c
@@ -196,7 +196,6 @@ static void uimagefs_remove(struct device_d *dev)
 {
 	struct uimagefs_handle *priv = dev->priv;
 	struct uimagefs_handle_data *d, *tmp;
-	struct stat s;
 
 	list_for_each_entry_safe(d, tmp, &priv->list, list) {
 		free(d->name);
@@ -204,10 +203,11 @@ static void uimagefs_remove(struct device_d *dev)
 		free(d);
 	}
 
-	if (IS_BUILTIN(CONFIG_FS_TFTP) && !stat(priv->tmp, &s))
-		unlink(priv->tmp);
+	if (priv->copy) {
+		unlink(priv->copy);
+		free(priv->copy);
+	}
 
-	free(priv->tmp);
 	free(priv);
 }
 
@@ -363,28 +363,21 @@ static int __uimage_open(struct uimagefs_handle *priv)
 	int ret;
 	size_t offset = 0;
 	size_t data_offset = 0;
+	const char *filename = priv->filename;
+
+	if (is_tftp_fs(filename)) {
+		ret = cache_file(filename, &priv->copy);
+		if (ret)
+			return ret;
+		filename = priv->copy;
+	}
 
-again:
-	fd = open(priv->filename, O_RDONLY);
+	fd = open(filename, O_RDONLY);
 	if (fd < 0) {
 		printf("could not open: %s\n", errno_str());
 		return fd;
 	}
 
-	/*
-	 * Hack around tftp fs. We need lseek for uImage support, but
-	 * this cannot be implemented in tftp fs, so we detect this
-	 * and copy the file to ram if it fails
-	 */
-	if (IS_BUILTIN(CONFIG_FS_TFTP) && !can_lseek_backward(fd)) {
-		close(fd);
-		ret = copy_file(priv->filename, priv->tmp, 0);
-		if (ret)
-			return ret;
-		priv->filename = priv->tmp;
-		goto again;
-	}
-
 	header = &priv->header;
 
 	ret = read(fd, header, sizeof(*header));
@@ -514,10 +507,6 @@ static int uimagefs_probe(struct device_d *dev)
 	priv->filename = fsdev->backingstore;
 	dev_dbg(dev, "mount: %s\n", fsdev->backingstore);
 
-	if (IS_BUILTIN(CONFIG_FS_TFTP))
-		priv->tmp = basprintf("/.uImage_tmp_%08x",
-					crc32(0, fsdev->path, strlen(fsdev->path)));
-
 	ret = __uimage_open(priv);
 	if (ret)
 		goto err;
diff --git a/include/uimagefs.h b/include/uimagefs.h
index 81b32310ad..3f58589b73 100644
--- a/include/uimagefs.h
+++ b/include/uimagefs.h
@@ -45,7 +45,7 @@ struct uimagefs_handle {
 	struct image_header header;
 	int nb_data_entries;
 	char *filename;
-	char *tmp;
+	char *copy;
 
 	struct list_head list;
 };
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 8/8] fs: remove now unused function can_lseek_backward()

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/fs.h | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/include/fs.h b/include/fs.h
index d75a1ef644..e6fcd044dd 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -101,26 +101,6 @@ struct fs_device_d {
 	char *linux_rootarg;
 };
 
-/*
- * Some filesystems i.e. tftpfs only support lseek into one direction.
- * To detect this limited functionality we add this extra function.
- * Additionaly we also return 0 if we even can not seek forward.
- */
-static inline int can_lseek_backward(int fd)
-{
-	int ret;
-
-	ret = lseek(fd, 1, SEEK_SET);
-	if (ret < 0)
-		return 0;
-
-	ret = lseek(fd, 0, SEEK_SET);
-	if (ret < 0)
-		return 0;
-
-	return 1;
-}
-
 bool __is_tftp_fs(const char *path);
 
 static inline bool is_tftp_fs(const char *path)
-- 
2.15.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox