Skip to contentSkip to conversation

fs: add pread and pwrite functions

RFC PATCH In review Alexander Aring 2 participants 7 messages Feb 17, 2013 – Feb 19, 2013 VFS

lore
Share & tools
lore

Alexander Aring

author
#0 lore raw

v2 cover fs: add pread and pwrite functions

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(-)
Signature · 7 lines
1.8.1.3


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
author
#1 lore raw

PATCH v2 1/2 fs: fix return type of read

Signed-off-by Alexander Aring <[email protected]>
---
 fs/fs.c      | 2 +-
 include/fs.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
fs/fs.c +1 -1
index f840516..48d1c89 100644
@@ -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;
include/fs.h +1 -1
index 919daab..d6b22f7 100644
@@ -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);
Signature · 7 lines
1.8.1.3


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
author
#2 lore raw

PATCH v2 2/2 fs: add pread and pwrite functions

In reply to Alexander Aring

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 <[email protected]>
---
 fs/fs.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/fs.h |  2 ++
 2 files changed, 77 insertions(+), 14 deletions(-)
fs/fs.c +75 -14
index 48d1c89..497a2ea 100644
@@ -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)
include/fs.h +2 -0
index d6b22f7..7c4e461 100644
@@ -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
Signature · 7 lines
1.8.1.3


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
#3 lore raw
Quoted 58 lines · Alexander Aring

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 <[email protected]>
---
 fs/fs.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/fs.h |  2 ++
 2 files changed, 77 insertions(+), 14 deletions(-)
fs/fs.c +11 -10
index 48d1c89..497a2ea 100644
@@ -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;
+

fs/fs.c ssize_t read(int fd, void *buf, size_t count) · line 799

Quoted 10 lines
fs/fs.c +10 -0
+	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

Signature · 9 lines
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
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
author
#4 lore raw

Hi Sascha,

Quoted 63 lines · Sascha Hauer
Quoted 62 lines · Alexander Aring

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 <[email protected]>
---
 fs/fs.c      | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 include/fs.h |  2 ++
 2 files changed, 77 insertions(+), 14 deletions(-)
fs/fs.c +15 -10
index 48d1c89..497a2ea 100644
@@ -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;
Quoted 9 lines

fs/fs.c ssize_t read(int fd, void *buf, size_t count) · line 799

Quoted 6 lines
fs/fs.c +6 -0
+	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
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

#5 lore raw
In reply to Alexander Aring

Hi Alexander,

Quoted 7 lines · Alexander Aring

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 <[email protected]>

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

Signature · 9 lines
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
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
author
#6 lore raw

Hi Sascha,

Quoted 9 lines · Sascha Hauer

Hi Alexander,

Quoted 6 lines · Alexander Aring

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.

Quoted 8 lines
Quoted 1 line
Signed-off-by Alexander Aring <[email protected]>

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
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox