[PATCH] read_file: Make it work on tftp servers which do not pass size

[PATCH] read_file: Make it work on tftp servers which do not pass size

[Sascha Hauer]

Some tftp servers (for example netkit-tftp) do not pass the filesize.
Add a workaround for read_file which reads the file into a temporary
file which then is copied to a buffer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c      | 18 ++++++++++++++++++
 fs/tftp.c    |  5 ++++-
 include/fs.h |  2 ++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index dc3a6e3..7046f2c 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
 	int fd;
 	struct stat s;
 	void *buf = NULL;
+	const char *tmpfile = "/.read_file_tmp";
+	int ret;
 
+again:
 	if (stat(filename, &s))
 		return NULL;
 
+	if (s.st_size == FILESIZE_MAX) {
+		ret = copy_file(filename, tmpfile, 0);
+		if (ret)
+			return NULL;
+		filename = tmpfile;
+		goto again;
+	}
+
 	buf = xzalloc(s.st_size + 1);
 
 	fd = open(filename, O_RDONLY);
@@ -56,12 +67,19 @@ void *read_file(const char *filename, size_t *size)
 	if (size)
 		*size = s.st_size;
 
+	if (filename == tmpfile)
+		unlink(tmpfile);
+
 	return buf;
 
 err_out1:
 	close(fd);
 err_out:
 	free(buf);
+
+	if (filename == tmpfile)
+		unlink(tmpfile);
+
 	return NULL;
 }
 
diff --git a/fs/tftp.c b/fs/tftp.c
index 98cbb37..1c37acf 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -597,7 +597,10 @@ static int tftp_stat(struct device_d *dev, const char *filename, struct stat *s)
 		return PTR_ERR(priv);
 
 	s->st_mode = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO;
-	s->st_size = priv->filesize;
+	if (priv->filesize)
+		s->st_size = priv->filesize;
+	else
+		s->st_size = FILESIZE_MAX;
 
 	tftp_do_close(priv);
 
diff --git a/include/fs.h b/include/fs.h
index 8ff7300..fa6a8da 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -147,6 +147,8 @@ int protect(int fd, size_t count, unsigned long offset, int prot);
 int protect_file(const char *file, int prot);
 void *memmap(int fd, int flags);
 
+#define FILESIZE_MAX	((size_t)-1)
+
 #define PROT_READ	1
 #define PROT_WRITE	2
 
-- 
1.8.3.1


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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Alexander Aring]

Hi Sascha,

only a very small hint.

On Wed, Jun 19, 2013 at 10:58:48PM +0200, Sascha Hauer wrote:
> Some tftp servers (for example netkit-tftp) do not pass the filesize.
> Add a workaround for read_file which reads the file into a temporary
> file which then is copied to a buffer.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  fs/fs.c      | 18 ++++++++++++++++++
>  fs/tftp.c    |  5 ++++-
>  include/fs.h |  2 ++
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index dc3a6e3..7046f2c 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
>  	int fd;

...

>  
>  	tftp_do_close(priv);
>  
> diff --git a/include/fs.h b/include/fs.h
> index 8ff7300..fa6a8da 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -147,6 +147,8 @@ int protect(int fd, size_t count, unsigned long offset, int prot);
>  int protect_file(const char *file, int prot);
>  void *memmap(int fd, int flags);
>  
> +#define FILESIZE_MAX	((size_t)-1)

The type of st_size in struct stat is loff_t. I check this and
((size_t)-1) is different than ((loff_t)-1), so I think we need to cast
to loff_t. Maybe it's better to use a define from limits.h, but we don't
have some header file like this.


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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Alexander Aring]

Hi,

On Wed, Jun 19, 2013 at 11:32:39PM +0200, Alexander Aring wrote:
> Hi Sascha,
> 
> only a very small hint.
> 
> On Wed, Jun 19, 2013 at 10:58:48PM +0200, Sascha Hauer wrote:
> > Some tftp servers (for example netkit-tftp) do not pass the filesize.
> > Add a workaround for read_file which reads the file into a temporary
> > file which then is copied to a buffer.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  fs/fs.c      | 18 ++++++++++++++++++
> >  fs/tftp.c    |  5 ++++-
> >  include/fs.h |  2 ++
> >  3 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/fs.c b/fs/fs.c
> > index dc3a6e3..7046f2c 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
> >  	int fd;
> 
> ...
> 
> >  
> >  	tftp_do_close(priv);
> >  
> > diff --git a/include/fs.h b/include/fs.h
> > index 8ff7300..fa6a8da 100644
> > --- a/include/fs.h
> > +++ b/include/fs.h
> > @@ -147,6 +147,8 @@ int protect(int fd, size_t count, unsigned long offset, int prot);
> >  int protect_file(const char *file, int prot);
> >  void *memmap(int fd, int flags);
> >  
> > +#define FILESIZE_MAX	((size_t)-1)
> 
> The type of st_size in struct stat is loff_t. I check this and
> ((size_t)-1) is different than ((loff_t)-1), so I think we need to cast
> to loff_t. Maybe it's better to use a define from limits.h, but we don't
> have some header file like this.
> 

ah, I see it now. The fs layer use size_t instead of loff_t. Sorry for
the noise.

Regards
Alex

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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Sascha Hauer]

On Wed, Jun 19, 2013 at 11:57:01PM +0200, Alexander Aring wrote:
> Hi,
> 
> On Wed, Jun 19, 2013 at 11:32:39PM +0200, Alexander Aring wrote:
> > Hi Sascha,
> > 
> > only a very small hint.
> > 
> > On Wed, Jun 19, 2013 at 10:58:48PM +0200, Sascha Hauer wrote:
> > > Some tftp servers (for example netkit-tftp) do not pass the filesize.
> > > Add a workaround for read_file which reads the file into a temporary
> > > file which then is copied to a buffer.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  fs/fs.c      | 18 ++++++++++++++++++
> > >  fs/tftp.c    |  5 ++++-
> > >  include/fs.h |  2 ++
> > >  3 files changed, 24 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/fs.c b/fs/fs.c
> > > index dc3a6e3..7046f2c 100644
> > > --- a/fs/fs.c
> > > +++ b/fs/fs.c
> > > @@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
> > >  	int fd;
> > 
> > ...
> > 
> > >  
> > >  	tftp_do_close(priv);
> > >  
> > > diff --git a/include/fs.h b/include/fs.h
> > > index 8ff7300..fa6a8da 100644
> > > --- a/include/fs.h
> > > +++ b/include/fs.h
> > > @@ -147,6 +147,8 @@ int protect(int fd, size_t count, unsigned long offset, int prot);
> > >  int protect_file(const char *file, int prot);
> > >  void *memmap(int fd, int flags);
> > >  
> > > +#define FILESIZE_MAX	((size_t)-1)
> > 
> > The type of st_size in struct stat is loff_t. I check this and
> > ((size_t)-1) is different than ((loff_t)-1), so I think we need to cast
> > to loff_t. Maybe it's better to use a define from limits.h, but we don't
> > have some header file like this.
> > 
> 
> ah, I see it now. The fs layer use size_t instead of loff_t. Sorry for
> the noise.

No no, you were right. This indeed has to be a loff_t. Where did you
find that the fs layer uses size_t? It should do so only for the length
arguments of read/write and friends.

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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Alexander Aring]

Hi Sascha,

On Thu, Jun 20, 2013 at 08:42:17AM +0200, Sascha Hauer wrote:
> On Wed, Jun 19, 2013 at 11:57:01PM +0200, Alexander Aring wrote:
> > Hi,
> > 
> > On Wed, Jun 19, 2013 at 11:32:39PM +0200, Alexander Aring wrote:
> > > Hi Sascha,
> > > 
> > > only a very small hint.
> > > 
> > > On Wed, Jun 19, 2013 at 10:58:48PM +0200, Sascha Hauer wrote:
> > > > Some tftp servers (for example netkit-tftp) do not pass the filesize.
> > > > Add a workaround for read_file which reads the file into a temporary
> > > > file which then is copied to a buffer.
> > > > 
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > >  fs/fs.c      | 18 ++++++++++++++++++
> > > >  fs/tftp.c    |  5 ++++-
> > > >  include/fs.h |  2 ++
> > > >  3 files changed, 24 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/fs/fs.c b/fs/fs.c
> > > > index dc3a6e3..7046f2c 100644
> > > > --- a/fs/fs.c
> > > > +++ b/fs/fs.c
> > > > @@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
> > > >  	int fd;
> > > 
> > > ...
> > > 
> > > >  
> > > >  	tftp_do_close(priv);
> > > >  
> > > > diff --git a/include/fs.h b/include/fs.h
> > > > index 8ff7300..fa6a8da 100644
> > > > --- a/include/fs.h
> > > > +++ b/include/fs.h
> > > > @@ -147,6 +147,8 @@ int protect(int fd, size_t count, unsigned long offset, int prot);
> > > >  int protect_file(const char *file, int prot);
> > > >  void *memmap(int fd, int flags);
> > > >  
> > > > +#define FILESIZE_MAX	((size_t)-1)
> > > 
> > > The type of st_size in struct stat is loff_t. I check this and
> > > ((size_t)-1) is different than ((loff_t)-1), so I think we need to cast
> > > to loff_t. Maybe it's better to use a define from limits.h, but we don't
> > > have some header file like this.
> > > 
> > 
> > ah, I see it now. The fs layer use size_t instead of loff_t. Sorry for
> > the noise.
> 
> No no, you were right. This indeed has to be a loff_t. Where did you
> find that the fs layer uses size_t? It should do so only for the length
> arguments of read/write and friends.
> 

In function read_file :). The parameter size should be changed to
loff_t instead of size_t, because we make "*size = s.st_size;".



The point is, that a file can be greater than 4GB, but we _can't_
read/write a "block" from a file that is greather than 4GB.

But in this function we do "read(fd, buf, s.st_size) < s.st_size", this
need to be in small (max) 4GB pieces(if necessary). In other words, we
can't use s.st_size here if the file is greater than ((size_t)-1).

I don't think if we need something like this, because we never handle
files which are greater than 4GB.

That's my point of view. This example is only for a 32Bit architecture.

Regards
Alex

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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Sascha Hauer]

On Thu, Jun 20, 2013 at 09:28:44AM +0200, Alexander Aring wrote:
> Hi Sascha,
> 
> > 
> > No no, you were right. This indeed has to be a loff_t. Where did you
> > find that the fs layer uses size_t? It should do so only for the length
> > arguments of read/write and friends.
> > 
> 
> In function read_file :). The parameter size should be changed to
> loff_t instead of size_t, because we make "*size = s.st_size;".
> 
> 
> 
> The point is, that a file can be greater than 4GB, but we _can't_
> read/write a "block" from a file that is greather than 4GB.
> 
> But in this function we do "read(fd, buf, s.st_size) < s.st_size", this
> need to be in small (max) 4GB pieces(if necessary). In other words, we
> can't use s.st_size here if the file is greater than ((size_t)-1).
> 
> I don't think if we need something like this, because we never handle
> files which are greater than 4GB.
> 
> That's my point of view. This example is only for a 32Bit architecture.

Ok, so read_file is broken for files > 4GiB. I think we can live with
this for a while ;)

BTW the maximum memory size the tlsf allocator can handle is 1GiB, we
have to overcome this first.

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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Jan Weitzel]

Am Mittwoch, den 19.06.2013, 22:58 +0200 schrieb Sascha Hauer:
> Some tftp servers (for example netkit-tftp) do not pass the filesize.
> Add a workaround for read_file which reads the file into a temporary
> file which then is copied to a buffer.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  fs/fs.c      | 18 ++++++++++++++++++
>  fs/tftp.c    |  5 ++++-
>  include/fs.h |  2 ++
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index dc3a6e3..7046f2c 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
>  	int fd;
>  	struct stat s;
>  	void *buf = NULL;
> +	const char *tmpfile = "/.read_file_tmp";
> +	int ret;
>  
> +again:
>  	if (stat(filename, &s))
>  		return NULL;
>  
> +	if (s.st_size == FILESIZE_MAX) {
> +		ret = copy_file(filename, tmpfile, 0);
> +		if (ret)
> +			return NULL;
> +		filename = tmpfile;
> +		goto again;
> +	}
> +
>  	buf = xzalloc(s.st_size + 1);
>  
>  	fd = open(filename, O_RDONLY);
> @@ -56,12 +67,19 @@ void *read_file(const char *filename, size_t *size)
>  	if (size)
>  		*size = s.st_size;
>  
> +	if (filename == tmpfile)
> +		unlink(tmpfile);
> +
>  	return buf;
>  
>  err_out1:
>  	close(fd);
>  err_out:
>  	free(buf);
> +
> +	if (filename == tmpfile)
> +		unlink(tmpfile);
> +
>  	return NULL;
>  }
>  
> diff --git a/fs/tftp.c b/fs/tftp.c
> index 98cbb37..1c37acf 100644
> --- a/fs/tftp.c
> +++ b/fs/tftp.c
> @@ -597,7 +597,10 @@ static int tftp_stat(struct device_d *dev, const char *filename, struct stat *s)
>  		return PTR_ERR(priv);
>  
>  	s->st_mode = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO;
> -	s->st_size = priv->filesize;
> +	if (priv->filesize)
> +		s->st_size = priv->filesize;
> +	else
> +		s->st_size = FILESIZE_MAX;
Maybe we can determine the size here? Commands like "ls -l" and
"ubiformat" asks about the filesize via stat. I'm not sure how much
overheat it will be.

Jan
>  
>  	tftp_do_close(priv);
>  
> diff --git a/include/fs.h b/include/fs.h
> index 8ff7300..fa6a8da 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -147,6 +147,8 @@ int protect(int fd, size_t count, unsigned long offset, int prot);
>  int protect_file(const char *file, int prot);
>  void *memmap(int fd, int flags);
>  
> +#define FILESIZE_MAX	((size_t)-1)
> +
>  #define PROT_READ	1
>  #define PROT_WRITE	2
>  



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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Sascha Hauer]

On Thu, Jun 20, 2013 at 01:15:31PM +0200, Jan Weitzel wrote:
> Am Mittwoch, den 19.06.2013, 22:58 +0200 schrieb Sascha Hauer:
> > Some tftp servers (for example netkit-tftp) do not pass the filesize.
> > Add a workaround for read_file which reads the file into a temporary
> > file which then is copied to a buffer.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  fs/fs.c      | 18 ++++++++++++++++++
> >  fs/tftp.c    |  5 ++++-
> >  include/fs.h |  2 ++
> >  3 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/fs.c b/fs/fs.c
> > index dc3a6e3..7046f2c 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
> >  	int fd;
> >  	struct stat s;
> >  	void *buf = NULL;
> > +	const char *tmpfile = "/.read_file_tmp";
> > +	int ret;
> >  
> > +again:
> >  	if (stat(filename, &s))
> >  		return NULL;
> >  
> > +	if (s.st_size == FILESIZE_MAX) {
> > +		ret = copy_file(filename, tmpfile, 0);
> > +		if (ret)
> > +			return NULL;
> > +		filename = tmpfile;
> > +		goto again;
> > +	}
> > +
> >  	buf = xzalloc(s.st_size + 1);
> >  
> >  	fd = open(filename, O_RDONLY);
> > @@ -56,12 +67,19 @@ void *read_file(const char *filename, size_t *size)
> >  	if (size)
> >  		*size = s.st_size;
> >  
> > +	if (filename == tmpfile)
> > +		unlink(tmpfile);
> > +
> >  	return buf;
> >  
> >  err_out1:
> >  	close(fd);
> >  err_out:
> >  	free(buf);
> > +
> > +	if (filename == tmpfile)
> > +		unlink(tmpfile);
> > +
> >  	return NULL;
> >  }
> >  
> > diff --git a/fs/tftp.c b/fs/tftp.c
> > index 98cbb37..1c37acf 100644
> > --- a/fs/tftp.c
> > +++ b/fs/tftp.c
> > @@ -597,7 +597,10 @@ static int tftp_stat(struct device_d *dev, const char *filename, struct stat *s)
> >  		return PTR_ERR(priv);
> >  
> >  	s->st_mode = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO;
> > -	s->st_size = priv->filesize;
> > +	if (priv->filesize)
> > +		s->st_size = priv->filesize;
> > +	else
> > +		s->st_size = FILESIZE_MAX;
> Maybe we can determine the size here? Commands like "ls -l" and
> "ubiformat" asks about the filesize via stat. I'm not sure how much
> overheat it will be.

How do you want to do that? You would have to transfer the whole file
first and see how big it is. That works for small files we expect to fit
into memory like the ones read_file normally is called with. If you want
to transfer a rootfs image it might happen that it's bigger than the
available memory.

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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Jan Weitzel]

Am Donnerstag, den 20.06.2013, 17:24 +0200 schrieb Sascha Hauer:
> On Thu, Jun 20, 2013 at 01:15:31PM +0200, Jan Weitzel wrote:
> > Am Mittwoch, den 19.06.2013, 22:58 +0200 schrieb Sascha Hauer:
> > > Some tftp servers (for example netkit-tftp) do not pass the filesize.
> > > Add a workaround for read_file which reads the file into a temporary
> > > file which then is copied to a buffer.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > >  fs/fs.c      | 18 ++++++++++++++++++
> > >  fs/tftp.c    |  5 ++++-
> > >  include/fs.h |  2 ++
> > >  3 files changed, 24 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/fs.c b/fs/fs.c
> > > index dc3a6e3..7046f2c 100644
> > > --- a/fs/fs.c
> > > +++ b/fs/fs.c
> > > @@ -38,10 +38,21 @@ void *read_file(const char *filename, size_t *size)
> > >  	int fd;
> > >  	struct stat s;
> > >  	void *buf = NULL;
> > > +	const char *tmpfile = "/.read_file_tmp";
> > > +	int ret;
> > >  
> > > +again:
> > >  	if (stat(filename, &s))
> > >  		return NULL;
> > >  
> > > +	if (s.st_size == FILESIZE_MAX) {
> > > +		ret = copy_file(filename, tmpfile, 0);
> > > +		if (ret)
> > > +			return NULL;
> > > +		filename = tmpfile;
> > > +		goto again;
> > > +	}
> > > +
> > >  	buf = xzalloc(s.st_size + 1);
> > >  
> > >  	fd = open(filename, O_RDONLY);
> > > @@ -56,12 +67,19 @@ void *read_file(const char *filename, size_t *size)
> > >  	if (size)
> > >  		*size = s.st_size;
> > >  
> > > +	if (filename == tmpfile)
> > > +		unlink(tmpfile);
> > > +
> > >  	return buf;
> > >  
> > >  err_out1:
> > >  	close(fd);
> > >  err_out:
> > >  	free(buf);
> > > +
> > > +	if (filename == tmpfile)
> > > +		unlink(tmpfile);
> > > +
> > >  	return NULL;
> > >  }
> > >  
> > > diff --git a/fs/tftp.c b/fs/tftp.c
> > > index 98cbb37..1c37acf 100644
> > > --- a/fs/tftp.c
> > > +++ b/fs/tftp.c
> > > @@ -597,7 +597,10 @@ static int tftp_stat(struct device_d *dev, const char *filename, struct stat *s)
> > >  		return PTR_ERR(priv);
> > >  
> > >  	s->st_mode = S_IFREG | S_IRWXU | S_IRWXG | S_IRWXO;
> > > -	s->st_size = priv->filesize;
> > > +	if (priv->filesize)
> > > +		s->st_size = priv->filesize;
> > > +	else
> > > +		s->st_size = FILESIZE_MAX;
> > Maybe we can determine the size here? Commands like "ls -l" and
> > "ubiformat" asks about the filesize via stat. I'm not sure how much
> > overheat it will be.
> 
> How do you want to do that? You would have to transfer the whole file
> first and see how big it is. That works for small files we expect to fit
> into memory like the ones read_file normally is called with. If you want
> to transfer a rootfs image it might happen that it's bigger than the
> available memory.
That's a good point. I didn't see a way for big files. But setting the
st_size to FILESIZE_MAX can cause trouble in other commands. ubiformat
only blames that is doesn't fit to eraseblock boundaries. ll -l shows a
really big size. What do you think about handle it complete in read_file
if size == 0?

Jan
> 
> Sascha
> 



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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Sascha Hauer]

On Fri, Jun 21, 2013 at 09:03:31AM +0200, Jan Weitzel wrote:
> Am Donnerstag, den 20.06.2013, 17:24 +0200 schrieb Sascha Hauer:
> > 
> > How do you want to do that? You would have to transfer the whole file
> > first and see how big it is. That works for small files we expect to fit
> > into memory like the ones read_file normally is called with. If you want
> > to transfer a rootfs image it might happen that it's bigger than the
> > available memory.
> That's a good point. I didn't see a way for big files. But setting the
> st_size to FILESIZE_MAX can cause trouble in other commands. ubiformat
> only blames that is doesn't fit to eraseblock boundaries.

Have you tried it?

> ll -l shows a
> really big size.

You'll never see this. Listing directories is not implemented in the
tftp protocol.

> What do you think about handle it complete in read_file
> if size == 0?

Maybe. What happens if the file is really 0 bytes big?

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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Jan Weitzel]

Am Freitag, den 21.06.2013, 09:10 +0200 schrieb Sascha Hauer:
> On Fri, Jun 21, 2013 at 09:03:31AM +0200, Jan Weitzel wrote:
> > Am Donnerstag, den 20.06.2013, 17:24 +0200 schrieb Sascha Hauer:
> > > 
> > > How do you want to do that? You would have to transfer the whole file
> > > first and see how big it is. That works for small files we expect to fit
> > > into memory like the ones read_file normally is called with. If you want
> > > to transfer a rootfs image it might happen that it's bigger than the
> > > available memory.
> > That's a good point. I didn't see a way for big files. But setting the
> > st_size to FILESIZE_MAX can cause trouble in other commands. ubiformat
> > only blames that is doesn't fit to eraseblock boundaries.
> 
> Have you tried it?
Yes, with a v2013.03.0 based barebox. Without the patch ubiformat -f
erase the ubi volume and "writes" a image of size 0.
The patched version stumbles over:

 if (st_size % mtd->eb_size) {
                sys_errmsg("file \"%s\" (size %lld bytes) is not
multiple of "
                           "eraseblock size (%d bytes)",

> 
> > ll -l shows a
> > really big size.
> 
> You'll never see this. Listing directories is not implemented in the
> tftp protocol.
> 
I got this with automount:
barebox@Phytec phyCORE pcm049:/  ls -l /mnt/tftp/weitzel/root.ubi
100Mbps full duplex link detected
T -rwxrwxrwx 4294967295 /mnt/tftp/weitzel/root.ubi

Without the patch it was 
-rwxrwxrwx          0 /mnt/tftp/weitzel/root.ubi

> > What do you think about handle it complete in read_file
> > if size == 0?
> 
> Maybe. What happens if the file is really 0 bytes big?
copy_file and unlinking will be the cost for this special case. We
should avoid a goto again loop ;)

Jan
> 
> Sascha
> 



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

Re: [PATCH] read_file: Make it work on tftp servers which do not pass size

[Sascha Hauer]

On Fri, Jun 21, 2013 at 09:46:41AM +0200, Jan Weitzel wrote:
> Am Freitag, den 21.06.2013, 09:10 +0200 schrieb Sascha Hauer:
> > On Fri, Jun 21, 2013 at 09:03:31AM +0200, Jan Weitzel wrote:
> > > Am Donnerstag, den 20.06.2013, 17:24 +0200 schrieb Sascha Hauer:
> > > > 
> > > > How do you want to do that? You would have to transfer the whole file
> > > > first and see how big it is. That works for small files we expect to fit
> > > > into memory like the ones read_file normally is called with. If you want
> > > > to transfer a rootfs image it might happen that it's bigger than the
> > > > available memory.
> > > That's a good point. I didn't see a way for big files. But setting the
> > > st_size to FILESIZE_MAX can cause trouble in other commands. ubiformat
> > > only blames that is doesn't fit to eraseblock boundaries.
> > 
> > Have you tried it?
> Yes, with a v2013.03.0 based barebox. Without the patch ubiformat -f
> erase the ubi volume and "writes" a image of size 0.

I assume it does so even when the image is not of size 0, right?
That sounds broken.

Aynway, you are beginning to convince me that 0 might be a better
choice.

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