[PATCH] uimage: fix: broken uimage_open on tftp since forward lseek works

[PATCH] uimage: fix: broken uimage_open on tftp since forward lseek works

[Michael Grzeschik]

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))) {
 		close(fd);
 		ret = copy_file(filename, uimage_tmp, 0);
 		if (ret)
-- 
2.11.0


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

Re: [PATCH] uimage: fix: broken uimage_open on tftp since forward lseek works

[Sascha Hauer]

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