From: Sascha Hauer <s.hauer@pengutronix.de>
To: Michael Grzeschik <m.grzeschik@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH] uimage: fix: broken uimage_open on tftp since forward lseek works
Date: Fri, 3 Nov 2017 08:58:42 +0100 [thread overview]
Message-ID: <20171103075842.rfdcw55f23rnyv54@pengutronix.de> (raw)
In-Reply-To: <20171101182345.1495-1-m.grzeschik@pengutronix.de>
On Wed, Nov 01, 2017 at 07:23:45PM +0100, Michael Grzeschik wrote:
> Since commit ce0cc7fe we support forward seek on tftpfs. This feature
> breaks the condition to check rather we open an uimage over tftp. Since
> random seeking is the problem here, we check in both directions.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> common/uimage.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/common/uimage.c b/common/uimage.c
> index 28a25bba2d..b07c6ff279 100644
> --- a/common/uimage.c
> +++ b/common/uimage.c
> @@ -113,7 +113,8 @@ again:
> * this cannot be implemented in tftp fs, so we detect this
> * by doing a test lseek and copy the file to ram if it fails
> */
> - if (IS_BUILTIN(CONFIG_FS_TFTP) && lseek(fd, 0, SEEK_SET)) {
> + if (IS_BUILTIN(CONFIG_FS_TFTP) &&
> + (lseek(fd, 4, SEEK_SET) && lseek(fd, 0, SEEK_SET))) {
lseek returns a negative error code or the position in the file, so
seeking to position 4 always returns a true value. Ok, the way the patch
works you do not care about the return value of the first lseek and all
you care about is that you can lseek backwards. From reading the code it
is unnecessarily hard to understand. Better create a
can_lseek_backward() function and write a clear comment above it what
the function does and why it is needed.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
prev parent reply other threads:[~2017-11-03 7:59 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-01 18:23 Michael Grzeschik
2017-11-03 7:58 ` Sascha Hauer [this message]
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=20171103075842.rfdcw55f23rnyv54@pengutronix.de \
--to=s.hauer@pengutronix.de \
--cc=barebox@lists.infradead.org \
--cc=m.grzeschik@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