From: Stefan Lengfeld <contact@stefanchrist.eu>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Barebox List <barebox@lists.infradead.org>
Subject: Re: [PATCH 6/7] fs/uimagefs: Use is_tftp_fs() and cache_file() to ease TFTP workaround
Date: Wed, 24 Jan 2018 20:56:17 +0100 [thread overview]
Message-ID: <20180124195617.GD521@porty> (raw)
In-Reply-To: <20180124074534.7966-7-s.hauer@pengutronix.de>
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@pengutronix.de>
> ---
> 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
>
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
next prev parent reply other threads:[~2018-01-24 19:56 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-24 7:45 [PATCH 0/7] Make TFTP detection more convenient Sascha Hauer
2018-01-24 7:45 ` [PATCH 1/7] startup: create /tmp Sascha Hauer
2018-01-24 7:45 ` [PATCH 2/7] fs: implement is_tftp_fs() Sascha Hauer
2018-01-24 19:12 ` Stefan Lengfeld
2018-01-25 7:46 ` Sascha Hauer
2018-01-24 7:45 ` [PATCH 3/7] libfile: implement make_temp Sascha Hauer
2018-01-24 7:45 ` [PATCH 4/7] libfile: implement a function to cache a file Sascha Hauer
2018-01-24 19:23 ` Stefan Lengfeld
2018-01-24 7:45 ` [PATCH 5/7] uimage: Use is_tftp_fs() and cache_file() to ease TFTP workaround Sascha Hauer
2018-01-24 19:39 ` Stefan Lengfeld
2018-01-24 7:45 ` [PATCH 6/7] fs/uimagefs: " Sascha Hauer
2018-01-24 19:56 ` Stefan Lengfeld [this message]
2018-01-24 7:45 ` [PATCH 7/7] fs: remove now unused function can_lseek_backward() Sascha Hauer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180124195617.GD521@porty \
--to=contact@stefanchrist.eu \
--cc=barebox@lists.infradead.org \
--cc=s.hauer@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox