Re: [U-Boot] [PATCH 09/17] fs: fat: support write with non-zero offset
On Fri, Jul 20, 2018 at 07:46:49PM +0200, Heinrich Schuchardt wrote:
> On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
> > In this patch, all the necessary code for allowing for a file offset
> > at write is implemented. What plays a major roll here is get_set_cluster(),
> > which, in contrast to its counterpart, set_cluster(), only operates on
> > already-allocated clusters, overwriting with data.
> >
> > So, with a file offset specified, set_contents() seeks and writes data
> > with set_get_cluster() until the end of a file, and, once it reaches
> > there, continues writing with set_cluster() for the rest.
> >
> > Please note that a file will be trimmed as a result of write operation if
> > write ends before reaching file's end. This is an intended behavior
> > in order to maitain compatibility with the current interface.
>
> This does not match the EFI spec.
First, please understand my standpoint that I mentioned in a reply
to your comment on my cover letter.
> >
> > Signed-off-by: AKASHI Takahiro
>
> When testing I observed that I could not write at an offset larger than
> the file size. This does not match the EFI spec:
So this is an intended behavior.
> EFI_FILE_PROTOCOL.SetPosition():
> seeking past the end of the file is allowed (a subsequent write would
> grow the file).
Surely, this is a discussion point.
Any comment from other folks?
-Takahiro AKASHI
> Best regards
>
> Heinrich
>
>
> > ---
> > fs/fat/fat_write.c | 287 ++---
> > 1 file changed, 272 insertions(+), 15 deletions(-)
> >
> > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> > index 3a9c53e253..cc45a33876 100644
> > --- a/fs/fat/fat_write.c
> > +++ b/fs/fat/fat_write.c
> > @@ -450,6 +450,120 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8
> > *buffer,
> > return 0;
> > }
> >
> > +static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
> > +
> > +/*
> > + * Read and modify data on existing and consecutive cluster blocks
> > + */
> > +static int
> > +get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer,
> > + loff_t size, loff_t *gotsize)
> > +{
> > + unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
> > + __u32 startsect;
> > + loff_t wsize;
> > + int clustcount, i, ret;
> > +
> > + *gotsize = 0;
> > + if (!size)
> > + return 0;
> > +
> > + assert(pos < bytesperclust);
> > + startsect = clust_to_sect(mydata, clustnum);
> > +
> > + debug("clustnum: %d, startsect: %d, pos: %lld\n", clustnum, startsect,
> > + pos);
> > +
> > + /* partial write at beginning */
> > + if (pos) {
> > + wsize = min(bytesperclust - pos, size);
> > + ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
> > + if (ret != mydata->clust_size) {
> > + debug("Error reading data (got %d)\n", ret);
> > + return -1;
> > + }
> > +
> > + memcpy(tmpbuf_cluster + pos, buffer, wsize);
> > + ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
> > + if (ret != mydata->clust_size) {
> > + debug("Error writing data (got %d)\n", ret);
> > + return -1;
> > + }
> > +
> > + size -= wsize;
> > + buffer += wsize;
> > + *gotsize += wsize;
> > +
> > + startsect += mydata->clust_size;
> > +
> > + if (!size)
> > + return 0;
> > + }
> > +
> > + /* full-cluster write */
> > + if (size >= bytesperclust) {
> > + clustcount = lldiv(size, bytesperclust);
> > +
> > + if (!((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1))) {
> > + wsize = clustcount * bytesperclust;
> > + ret = disk_write(startsect,
> > + clustcount * mydata->clust_size,
> > + buffer);
> > + if (ret != clustcount * mydata->clust_size) {
> > + debug("Error writing data (got %d)\n", ret);
> > + return -1;
> > + }
> > +
> > + size -= wsize;
> > + buffer += wsize;
> > + *gotsize += wsize;
> > +
> > + startsect += clustcount * mydata->clust_size;
> > + } else {
> > + for (i = 0; i < clustcount; i++) {
> > + memcpy(tmpbuf_cluster, buffer, bytesperclust);
> > + ret = disk_write(startsect, mydata->clust_size,
> > + tmpbuf_cluster);
> > + if (ret != mydata->clust_size) {
> > + debug("Error writing data (got %d)\n",
> > +
Re: [U-Boot] [PATCH 09/17] fs: fat: support write with non-zero offset
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
> In this patch, all the necessary code for allowing for a file offset
> at write is implemented. What plays a major roll here is get_set_cluster(),
> which, in contrast to its counterpart, set_cluster(), only operates on
> already-allocated clusters, overwriting with data.
>
> So, with a file offset specified, set_contents() seeks and writes data
> with set_get_cluster() until the end of a file, and, once it reaches
> there, continues writing with set_cluster() for the rest.
>
> Please note that a file will be trimmed as a result of write operation if
> write ends before reaching file's end. This is an intended behavior
> in order to maitain compatibility with the current interface.
This does not match the EFI spec.
>
> Signed-off-by: AKASHI Takahiro
When testing I observed that I could not write at an offset larger than
the file size. This does not match the EFI spec:
EFI_FILE_PROTOCOL.SetPosition():
seeking past the end of the file is allowed (a subsequent write would
grow the file).
Best regards
Heinrich
> ---
> fs/fat/fat_write.c | 287 ++---
> 1 file changed, 272 insertions(+), 15 deletions(-)
>
> diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> index 3a9c53e253..cc45a33876 100644
> --- a/fs/fat/fat_write.c
> +++ b/fs/fat/fat_write.c
> @@ -450,6 +450,120 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8
> *buffer,
> return 0;
> }
>
> +static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
> +
> +/*
> + * Read and modify data on existing and consecutive cluster blocks
> + */
> +static int
> +get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer,
> + loff_t size, loff_t *gotsize)
> +{
> + unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
> + __u32 startsect;
> + loff_t wsize;
> + int clustcount, i, ret;
> +
> + *gotsize = 0;
> + if (!size)
> + return 0;
> +
> + assert(pos < bytesperclust);
> + startsect = clust_to_sect(mydata, clustnum);
> +
> + debug("clustnum: %d, startsect: %d, pos: %lld\n", clustnum, startsect,
> + pos);
> +
> + /* partial write at beginning */
> + if (pos) {
> + wsize = min(bytesperclust - pos, size);
> + ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
> + if (ret != mydata->clust_size) {
> + debug("Error reading data (got %d)\n", ret);
> + return -1;
> + }
> +
> + memcpy(tmpbuf_cluster + pos, buffer, wsize);
> + ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
> + if (ret != mydata->clust_size) {
> + debug("Error writing data (got %d)\n", ret);
> + return -1;
> + }
> +
> + size -= wsize;
> + buffer += wsize;
> + *gotsize += wsize;
> +
> + startsect += mydata->clust_size;
> +
> + if (!size)
> + return 0;
> + }
> +
> + /* full-cluster write */
> + if (size >= bytesperclust) {
> + clustcount = lldiv(size, bytesperclust);
> +
> + if (!((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1))) {
> + wsize = clustcount * bytesperclust;
> + ret = disk_write(startsect,
> + clustcount * mydata->clust_size,
> + buffer);
> + if (ret != clustcount * mydata->clust_size) {
> + debug("Error writing data (got %d)\n", ret);
> + return -1;
> + }
> +
> + size -= wsize;
> + buffer += wsize;
> + *gotsize += wsize;
> +
> + startsect += clustcount * mydata->clust_size;
> + } else {
> + for (i = 0; i < clustcount; i++) {
> + memcpy(tmpbuf_cluster, buffer, bytesperclust);
> + ret = disk_write(startsect, mydata->clust_size,
> + tmpbuf_cluster);
> + if (ret != mydata->clust_size) {
> + debug("Error writing data (got %d)\n",
> + ret);
> + return -1;
> + }
> +
> + size -= bytesperclust;
> + buffer += bytesperclust;
> + *gotsize += bytesperclust;
> +
> + startsect += mydata->clust_size;
> + }
> + }
> + }
> +
> + /* partial
[U-Boot] [PATCH 09/17] fs: fat: support write with non-zero offset
In this patch, all the necessary code for allowing for a file offset
at write is implemented. What plays a major roll here is get_set_cluster(),
which, in contrast to its counterpart, set_cluster(), only operates on
already-allocated clusters, overwriting with data.
So, with a file offset specified, set_contents() seeks and writes data
with set_get_cluster() until the end of a file, and, once it reaches
there, continues writing with set_cluster() for the rest.
Please note that a file will be trimmed as a result of write operation if
write ends before reaching file's end. This is an intended behavior
in order to maitain compatibility with the current interface.
Signed-off-by: AKASHI Takahiro
---
fs/fat/fat_write.c | 287 ++---
1 file changed, 272 insertions(+), 15 deletions(-)
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
index 3a9c53e253..cc45a33876 100644
--- a/fs/fat/fat_write.c
+++ b/fs/fat/fat_write.c
@@ -450,6 +450,120 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer,
return 0;
}
+static __u8 tmpbuf_cluster[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN);
+
+/*
+ * Read and modify data on existing and consecutive cluster blocks
+ */
+static int
+get_set_cluster(fsdata *mydata, __u32 clustnum, loff_t pos, __u8 *buffer,
+ loff_t size, loff_t *gotsize)
+{
+ unsigned int bytesperclust = mydata->clust_size * mydata->sect_size;
+ __u32 startsect;
+ loff_t wsize;
+ int clustcount, i, ret;
+
+ *gotsize = 0;
+ if (!size)
+ return 0;
+
+ assert(pos < bytesperclust);
+ startsect = clust_to_sect(mydata, clustnum);
+
+ debug("clustnum: %d, startsect: %d, pos: %lld\n", clustnum, startsect,
+ pos);
+
+ /* partial write at beginning */
+ if (pos) {
+ wsize = min(bytesperclust - pos, size);
+ ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
+ if (ret != mydata->clust_size) {
+ debug("Error reading data (got %d)\n", ret);
+ return -1;
+ }
+
+ memcpy(tmpbuf_cluster + pos, buffer, wsize);
+ ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
+ if (ret != mydata->clust_size) {
+ debug("Error writing data (got %d)\n", ret);
+ return -1;
+ }
+
+ size -= wsize;
+ buffer += wsize;
+ *gotsize += wsize;
+
+ startsect += mydata->clust_size;
+
+ if (!size)
+ return 0;
+ }
+
+ /* full-cluster write */
+ if (size >= bytesperclust) {
+ clustcount = lldiv(size, bytesperclust);
+
+ if (!((unsigned long)buffer & (ARCH_DMA_MINALIGN - 1))) {
+ wsize = clustcount * bytesperclust;
+ ret = disk_write(startsect,
+ clustcount * mydata->clust_size,
+ buffer);
+ if (ret != clustcount * mydata->clust_size) {
+ debug("Error writing data (got %d)\n", ret);
+ return -1;
+ }
+
+ size -= wsize;
+ buffer += wsize;
+ *gotsize += wsize;
+
+ startsect += clustcount * mydata->clust_size;
+ } else {
+ for (i = 0; i < clustcount; i++) {
+ memcpy(tmpbuf_cluster, buffer, bytesperclust);
+ ret = disk_write(startsect, mydata->clust_size,
+ tmpbuf_cluster);
+ if (ret != mydata->clust_size) {
+ debug("Error writing data (got %d)\n",
+ ret);
+ return -1;
+ }
+
+ size -= bytesperclust;
+ buffer += bytesperclust;
+ *gotsize += bytesperclust;
+
+ startsect += mydata->clust_size;
+ }
+ }
+ }
+
+ /* partial write at end */
+ if (size) {
+ wsize = size;
+ ret = disk_read(startsect, mydata->clust_size, tmpbuf_cluster);
+ if (ret != mydata->clust_size) {
+ debug("Error reading data (got %d)\n", ret);
+ return -1;
+ }
+ memcpy(tmpbuf_cluster, buffer, wsize);
+ ret = disk_write(startsect, mydata->clust_size, tmpbuf_cluster);
