mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/2] fs: add pread and pwrite functions
@ 2013-02-17 21:04 Alexander Aring
  2013-02-17 21:05 ` [PATCH 1/2] fs: fix return type of read Alexander Aring
  2013-02-17 21:05 ` [PATCH 2/2] fs: add pread and pwrite functions Alexander Aring
  0 siblings, 2 replies; 7+ messages in thread
From: Alexander Aring @ 2013-02-17 21:04 UTC (permalink / raw)
  To: barebox

These Patches fixes the return type of read function and add two new functions,
pread and pwrite in fs layer of barebox.

In my opinion this implementation of pread and pwrite could be better if fs
interface has as read callback:
int (*read)(struct device_d *dev, loff_t pos, ... flags, etc...);

not a "FILE *" as parameter.

instead of:
int (*read)(struct device_d *dev, FILE *f, void *buf, size_t size);

I don't want to screw up the interface, it's only a notice. This would be funny
if we change that. :-/

The current implementation of pread and pwrite saves the current file pos and
restore them after do some file manipulation. Could be optimize if a filesystem
driver doesn't read the pos from FILE pointer and I can give the offset over a
parameter. In case of read it would be "f->pos" and pread it would be "offset".

Another thing is:

We could optimize it (pwrite and pread) and (read and write) looks very similar.
The only different between these functions is a "const void *buf" and "void *buf"

I thinking a about a function with a functionpointer as parameter like

static ssize_t __do_read_or_write(int fd, void *buf, size_t count,
			ssize_t func(int fd, void *buf, size_t count))

Which func is for read = __read and for write = __write, but I got trouble because
the function prototype "void *" and "const void*". I can do more parameters and
decide it as runtime, but I don't like to do that, looks very ugly.

Regards
Alex

v2:
	rewritten pread and pwrite functions

Alexander Aring (2):
  fs: fix return type of read
  fs: add pread and pwrite functions

 fs/fs.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/fs.h |  4 ++-
 2 files changed, 78 insertions(+), 15 deletions(-)

-- 
1.8.1.3


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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 1/2] fs: fix return type of read
  2013-02-17 21:04 [RFC PATCH v2 0/2] fs: add pread and pwrite functions Alexander Aring
@ 2013-02-17 21:05 ` Alexander Aring
  2013-02-17 21:05 ` [PATCH 2/2] fs: add pread and pwrite functions Alexander Aring
  1 sibling, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2013-02-17 21:05 UTC (permalink / raw)
  To: barebox

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 fs/fs.c      | 2 +-
 include/fs.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index f840516..48d1c89 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -754,7 +754,7 @@ int ioctl(int fd, int request, void *buf)
 	return ret;
 }
 
-int read(int fd, void *buf, size_t count)
+ssize_t read(int fd, void *buf, size_t count)
 {
 	struct device_d *dev;
 	struct fs_driver_d *fsdrv;
diff --git a/include/fs.h b/include/fs.h
index 919daab..d6b22f7 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -113,7 +113,7 @@ int close(int fd);
 int flush(int fd);
 int lstat(const char *filename, struct stat *s);
 int stat(const char *filename, struct stat *s);
-int read(int fd, void *buf, size_t count);
+ssize_t read(int fd, void *buf, size_t count);
 int ioctl(int fd, int request, void *buf);
 ssize_t write(int fd, const void *buf, size_t count);
 
-- 
1.8.1.3


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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 2/2] fs: add pread and pwrite functions
  2013-02-17 21:04 [RFC PATCH v2 0/2] fs: add pread and pwrite functions Alexander Aring
  2013-02-17 21:05 ` [PATCH 1/2] fs: fix return type of read Alexander Aring
@ 2013-02-17 21:05 ` Alexander Aring
  2013-02-18 10:17   ` Sascha Hauer
  2013-02-19 19:33   ` Sascha Hauer
  1 sibling, 2 replies; 7+ messages in thread
From: Alexander Aring @ 2013-02-17 21:05 UTC (permalink / raw)
  To: barebox

Add pread and pwrite functions.

Split read and write functions to save some space.
The functions pread and pwrite saves and sets the file
position to a given offset and restore them afterwards.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 fs/fs.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/fs.h |  2 ++
 2 files changed, 77 insertions(+), 14 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index 48d1c89..497a2ea 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -754,17 +754,12 @@ int ioctl(int fd, int request, void *buf)
 	return ret;
 }
 
-ssize_t read(int fd, void *buf, size_t count)
+static ssize_t __read(FILE *f, void *buf, size_t count)
 {
 	struct device_d *dev;
 	struct fs_driver_d *fsdrv;
-	FILE *f;
 	int ret;
 
-	if (check_fd(fd))
-		return -errno;
-
-	f = &files[fd];
 	dev = f->dev;
 
 	fsdrv = dev_to_fs_driver(dev);
@@ -777,18 +772,34 @@ ssize_t read(int fd, void *buf, size_t count)
 
 	ret = fsdrv->read(dev, f, buf, count);
 
-	if (ret > 0)
-		f->pos += ret;
 	if (ret < 0)
 		errno = -ret;
 	return ret;
 }
-EXPORT_SYMBOL(read);
+EXPORT_SYMBOL(pread);
 
-ssize_t write(int fd, const void *buf, size_t count)
+ssize_t pread(int fd, void *buf, size_t count, loff_t offset)
+{
+	loff_t pos;
+	FILE *f;
+	int ret;
+
+	if (check_fd(fd))
+		return -errno;
+
+	f = &files[fd];
+
+	pos = f->pos;
+	f->pos = offset;
+	ret = __read(f, buf, count);
+	f->pos = pos;
+
+	return ret;
+}
+EXPORT_SYMBOL(pread);
+
+ssize_t read(int fd, void *buf, size_t count)
 {
-	struct device_d *dev;
-	struct fs_driver_d *fsdrv;
 	FILE *f;
 	int ret;
 
@@ -796,6 +807,21 @@ ssize_t write(int fd, const void *buf, size_t count)
 		return -errno;
 
 	f = &files[fd];
+
+	ret = __read(f, buf, count);
+
+	if (ret > 0)
+		f->pos += ret;
+	return ret;
+}
+EXPORT_SYMBOL(read);
+
+static ssize_t __write(FILE *f, const void *buf, size_t count)
+{
+	struct device_d *dev;
+	struct fs_driver_d *fsdrv;
+	int ret;
+
 	dev = f->dev;
 
 	fsdrv = dev_to_fs_driver(dev);
@@ -812,13 +838,48 @@ ssize_t write(int fd, const void *buf, size_t count)
 		}
 	}
 	ret = fsdrv->write(dev, f, buf, count);
-	if (ret > 0)
-		f->pos += ret;
 out:
 	if (ret < 0)
 		errno = -ret;
 	return ret;
 }
+
+ssize_t pwrite(int fd, const void *buf, size_t count, loff_t offset)
+{
+	loff_t pos;
+	FILE *f;
+	int ret;
+
+	if (check_fd(fd))
+		return -errno;
+
+	f = &files[fd];
+
+	pos = f->pos;
+	f->pos = offset;
+	ret = __write(f, buf, count);
+	f->pos = pos;
+
+	return ret;
+}
+EXPORT_SYMBOL(pwrite);
+
+ssize_t write(int fd, const void *buf, size_t count)
+{
+	FILE *f;
+	int ret;
+
+	if (check_fd(fd))
+		return -errno;
+
+	f = &files[fd];
+
+	ret = __write(f, buf, count);
+
+	if (ret > 0)
+		f->pos += ret;
+	return ret;
+}
 EXPORT_SYMBOL(write);
 
 int flush(int fd)
diff --git a/include/fs.h b/include/fs.h
index d6b22f7..7c4e461 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -114,8 +114,10 @@ int flush(int fd);
 int lstat(const char *filename, struct stat *s);
 int stat(const char *filename, struct stat *s);
 ssize_t read(int fd, void *buf, size_t count);
+ssize_t pread(int fd, void *buf, size_t count, loff_t offset);
 int ioctl(int fd, int request, void *buf);
 ssize_t write(int fd, const void *buf, size_t count);
+ssize_t pwrite(int fd, const void *buf, size_t count, loff_t offset);
 
 #define SEEK_SET	1
 #define SEEK_CUR	2
-- 
1.8.1.3


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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fs: add pread and pwrite functions
  2013-02-17 21:05 ` [PATCH 2/2] fs: add pread and pwrite functions Alexander Aring
@ 2013-02-18 10:17   ` Sascha Hauer
  2013-02-18 15:13     ` Alexander Aring
  2013-02-19 19:33   ` Sascha Hauer
  1 sibling, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2013-02-18 10:17 UTC (permalink / raw)
  To: Alexander Aring; +Cc: barebox

On Sun, Feb 17, 2013 at 10:05:01PM +0100, Alexander Aring wrote:
> Add pread and pwrite functions.
> 
> Split read and write functions to save some space.
> The functions pread and pwrite saves and sets the file
> position to a given offset and restore them afterwards.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  fs/fs.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
>  include/fs.h |  2 ++
>  2 files changed, 77 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index 48d1c89..497a2ea 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -754,17 +754,12 @@ int ioctl(int fd, int request, void *buf)
>  	return ret;
>  }
>  
> -ssize_t read(int fd, void *buf, size_t count)
> +static ssize_t __read(FILE *f, void *buf, size_t count)
>  {
>  	struct device_d *dev;
>  	struct fs_driver_d *fsdrv;
> -	FILE *f;
>  	int ret;
>  
> -	if (check_fd(fd))
> -		return -errno;
> -
> -	f = &files[fd];
>  	dev = f->dev;
>  
>  	fsdrv = dev_to_fs_driver(dev);
> @@ -777,18 +772,34 @@ ssize_t read(int fd, void *buf, size_t count)
>  
>  	ret = fsdrv->read(dev, f, buf, count);
>  
> -	if (ret > 0)
> -		f->pos += ret;
>  	if (ret < 0)
>  		errno = -ret;
>  	return ret;
>  }
> -EXPORT_SYMBOL(read);
> +EXPORT_SYMBOL(pread);
>  
> -ssize_t write(int fd, const void *buf, size_t count)
> +ssize_t pread(int fd, void *buf, size_t count, loff_t offset)
> +{
> +	loff_t pos;
> +	FILE *f;
> +	int ret;
> +
> +	if (check_fd(fd))
> +		return -errno;
> +
> +	f = &files[fd];
> +
> +	pos = f->pos;
> +	f->pos = offset;
> +	ret = __read(f, buf, count);
> +	f->pos = pos;
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(pread);

pread is exported twice with this patch. I fixed this while applying.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fs: add pread and pwrite functions
  2013-02-18 10:17   ` Sascha Hauer
@ 2013-02-18 15:13     ` Alexander Aring
  0 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2013-02-18 15:13 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

On Mon, Feb 18, 2013 at 11:17:33AM +0100, Sascha Hauer wrote:
> On Sun, Feb 17, 2013 at 10:05:01PM +0100, Alexander Aring wrote:
> > Add pread and pwrite functions.
> > 
> > Split read and write functions to save some space.
> > The functions pread and pwrite saves and sets the file
> > position to a given offset and restore them afterwards.
> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> > ---
> >  fs/fs.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
> >  include/fs.h |  2 ++
> >  2 files changed, 77 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/fs.c b/fs/fs.c
> > index 48d1c89..497a2ea 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -754,17 +754,12 @@ int ioctl(int fd, int request, void *buf)
> >  	return ret;
> >  }
> >  
> > -ssize_t read(int fd, void *buf, size_t count)
> > +static ssize_t __read(FILE *f, void *buf, size_t count)
> >  {
> >  	struct device_d *dev;
> >  	struct fs_driver_d *fsdrv;
> > -	FILE *f;
> >  	int ret;
> >  
> > -	if (check_fd(fd))
> > -		return -errno;
> > -
> > -	f = &files[fd];
> >  	dev = f->dev;
> >  
> >  	fsdrv = dev_to_fs_driver(dev);
> > @@ -777,18 +772,34 @@ ssize_t read(int fd, void *buf, size_t count)
> >  
> >  	ret = fsdrv->read(dev, f, buf, count);
> >  
> > -	if (ret > 0)
> > -		f->pos += ret;
> >  	if (ret < 0)
> >  		errno = -ret;
> >  	return ret;
> >  }
> > -EXPORT_SYMBOL(read);
> > +EXPORT_SYMBOL(pread);
> >  
> > -ssize_t write(int fd, const void *buf, size_t count)
> > +ssize_t pread(int fd, void *buf, size_t count, loff_t offset)
> > +{
> > +	loff_t pos;
> > +	FILE *f;
> > +	int ret;
> > +
> > +	if (check_fd(fd))
> > +		return -errno;
> > +
> > +	f = &files[fd];
> > +
> > +	pos = f->pos;
> > +	f->pos = offset;
> > +	ret = __read(f, buf, count);
> > +	f->pos = pos;
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(pread);
> 
> pread is exported twice with this patch. I fixed this while applying.
>

Ooops, sry. Thanks for applying.

Regards
Alex

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fs: add pread and pwrite functions
  2013-02-17 21:05 ` [PATCH 2/2] fs: add pread and pwrite functions Alexander Aring
  2013-02-18 10:17   ` Sascha Hauer
@ 2013-02-19 19:33   ` Sascha Hauer
  2013-02-19 20:30     ` Alexander Aring
  1 sibling, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2013-02-19 19:33 UTC (permalink / raw)
  To: Alexander Aring; +Cc: barebox

Hi Alexander,

On Sun, Feb 17, 2013 at 10:05:01PM +0100, Alexander Aring wrote:
> Add pread and pwrite functions.
> 
> Split read and write functions to save some space.
> The functions pread and pwrite saves and sets the file
> position to a given offset and restore them afterwards.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>

This currently breaks compilation of the nandtest command. We have two
options: Either rename pread/pwrite functions in the nandtest command or
just drop the static versions in the nandtest command.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] fs: add pread and pwrite functions
  2013-02-19 19:33   ` Sascha Hauer
@ 2013-02-19 20:30     ` Alexander Aring
  0 siblings, 0 replies; 7+ messages in thread
From: Alexander Aring @ 2013-02-19 20:30 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

On Tue, Feb 19, 2013 at 08:33:39PM +0100, Sascha Hauer wrote:
> Hi Alexander,
> 
> On Sun, Feb 17, 2013 at 10:05:01PM +0100, Alexander Aring wrote:
> > Add pread and pwrite functions.
> > 
> > Split read and write functions to save some space.
> > The functions pread and pwrite saves and sets the file
> > position to a given offset and restore them afterwards.
> > 
> > Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> 
> This currently breaks compilation of the nandtest command. We have two
> options: Either rename pread/pwrite functions in the nandtest command or
> just drop the static versions in the nandtest command.
> 
> Sascha
> 

Yes, I know... I have already Patches for this. I punish myself, because
it were better to add this changes into this patch series.

First patch series v1 has them include.
I will resend them now.

Regards
Alex

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-02-19 20:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-17 21:04 [RFC PATCH v2 0/2] fs: add pread and pwrite functions Alexander Aring
2013-02-17 21:05 ` [PATCH 1/2] fs: fix return type of read Alexander Aring
2013-02-17 21:05 ` [PATCH 2/2] fs: add pread and pwrite functions Alexander Aring
2013-02-18 10:17   ` Sascha Hauer
2013-02-18 15:13     ` Alexander Aring
2013-02-19 19:33   ` Sascha Hauer
2013-02-19 20:30     ` Alexander Aring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox