Hi Sascha,
On Wed, Jan 24, 2018 at 08:45:33AM +0100, Sascha Hauer wrote:
> We have helper functions now to ease file caching when a file
> is on TFTP. Use them.
>
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> ---
> fs/uimagefs.c | 33 ++++++++++-----------------------
> include/uimagefs.h | 2 +-
> 2 files changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/fs/uimagefs.c b/fs/uimagefs.c
> index c0c5750c2c..2607d285f1 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);
> }
>
> @@ -364,27 +364,18 @@ static int __uimage_open(struct uimagefs_handle *priv)
> size_t offset = 0;
> size_t data_offset = 0;
>
> -again:
> + if (is_tftp_fs(priv->filename)) {
> + ret = cache_file(priv->filename, &priv->copy);
> + if (ret)
> + return ret;
> + }
> +
> fd = open(priv->filename, O_RDONLY);
Same pattern as in the previous patch, but slightly different code :-)
The open() call must use the newly created file in 'priv->copy' and not
open the non-seekable file in 'priv-filename' again (if the file was not
seekable of course). Maybe copy the contents of priv->filename to
priv->copy in the seekable case.
+1 for removing the backwards jumping goto-statement that hides this
magic.
> if (fd < 0) {
> printf("could not open: %s\n", errno_str());
> return fd;
Just a note: Since the struct uimagefs_handle is caller allocated, the
caller takes care of freeing the priv->copy string with function
uimagefs_remove() in case of an error. So no leaking string here.
All in all. Nice cleanup work.
Kind regards,
Stefan
> }
>
> - /*
> - * 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 +505,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.11.0
>
>
> _______________________________________________
> barebox mailing list
> barebox@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/barebox
>
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
