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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox



[Index of Archives]     [Linux Embedded]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux