| Message ID | 20180720025723.6736-7-takahiro.akashi@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | fs: fat: extend FAT write operations | expand |
On 07/20/2018 04:57 AM, AKASHI Takahiro wrote: > It would be good that FAT write function return error code instead of > just returning -1 as fat_read_file() does. > This patch attempts to address this issue although it is 'best effort > (or estimate)' for now. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Some of the error message are written with debug() others with printf(). Shouldn't we switch everything to debug() so that the EFI console is not messed up? Best regards Heinrich > --- > fs/fat/fat_write.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c > index 6c715a70f4..1e4f5af910 100644 > --- a/fs/fat/fat_write.c > +++ b/fs/fat/fat_write.c > @@ -956,7 +956,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) { > debug("error: reading boot sector\n"); > - return -1; > + return -EIO; > } > > total_sector = bs.total_sect; > @@ -997,7 +997,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > mydata->fatbuf = memalign(ARCH_DMA_MINALIGN, FATBUFSIZE); > if (mydata->fatbuf == NULL) { > debug("Error: allocating memory\n"); > - return -1; > + return -ENOMEM; > } > > if (disk_read(cursect, > @@ -1005,6 +1005,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > (mydata->clust_size) : > PREFETCH_BLOCKS, do_fat_read_at_block) < 0) { > debug("Error: reading rootdir block\n"); > + ret = -EIO; > goto exit; > } > dentptr = (dir_entry *) do_fat_read_at_block; > @@ -1029,6 +1030,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > size); > if (ret) { > printf("Error: %llu overflow\n", size); > + ret = -ENOSPC; > goto exit; > } > } > @@ -1036,6 +1038,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > ret = clear_fatent(mydata, start_cluster); > if (ret) { > printf("Error: clearing FAT entries\n"); > + ret = -EIO; > goto exit; > } > > @@ -1045,12 +1048,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > ret = start_cluster = find_empty_cluster(mydata); > if (ret < 0) { > printf("Error: finding empty cluster\n"); > + ret = -ENOSPC; > goto exit; > } > > ret = check_overflow(mydata, start_cluster, size); > if (ret) { > printf("Error: %llu overflow\n", size); > + ret = -ENOSPC; > goto exit; > } > > @@ -1065,12 +1070,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > ret = start_cluster = find_empty_cluster(mydata); > if (ret < 0) { > printf("Error: finding empty cluster\n"); > + ret = -ENOSPC; > goto exit; > } > > ret = check_overflow(mydata, start_cluster, size); > if (ret) { > printf("Error: %llu overflow\n", size); > + ret = -ENOSPC; > goto exit; > } > } else { > @@ -1087,6 +1094,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > ret = set_contents(mydata, retdent, buffer, size, actwrite); > if (ret < 0) { > printf("Error: writing contents\n"); > + ret = -EIO; > goto exit; > } > debug("attempt to write 0x%llx bytes\n", *actwrite); > @@ -1095,14 +1103,17 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > ret = flush_dirty_fat_buffer(mydata); > if (ret) { > printf("Error: flush fat buffer\n"); > + ret = -EIO; > goto exit; > } > > /* Write directory table to device */ > ret = set_cluster(mydata, dir_curclust, get_dentfromdir_block, > mydata->clust_size * mydata->sect_size); > - if (ret) > + if (ret) { > printf("Error: writing directory entry\n"); > + ret = -EIO; > + } > > exit: > free(mydata->fatbuf); > @@ -1114,7 +1125,7 @@ int file_fat_write(const char *filename, void *buffer, loff_t offset, > { > if (offset != 0) { > printf("Error: non zero offset is currently not supported.\n"); > - return -1; > + return -EINVAL; > } > > printf("writing %s\n", filename); >
On Fri, Jul 20, 2018 at 07:55:06PM +0200, Heinrich Schuchardt wrote: > On 07/20/2018 04:57 AM, AKASHI Takahiro wrote: > > It would be good that FAT write function return error code instead of > > just returning -1 as fat_read_file() does. > > This patch attempts to address this issue although it is 'best effort > > (or estimate)' for now. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > Some of the error message are written with debug() others with printf(). Right, but as you know, there is already a mixture of debug() and printf() in the existing code. Here I tried to do my best to use either one so as to make the usage consistent with existing code though I don't know the exact discipline. > Shouldn't we switch everything to debug() so that the EFI console is not > messed up? Is this a different issue, isn't it? (I suppose that one of Rob's patches tried to address this issue.) So I'd like to listen to any opinion from other folks, too. Thanks, -Takahiro AKASHI > Best regards > > Heinrich > > > --- > > fs/fat/fat_write.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c > > index 6c715a70f4..1e4f5af910 100644 > > --- a/fs/fat/fat_write.c > > +++ b/fs/fat/fat_write.c > > @@ -956,7 +956,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > > > if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) { > > debug("error: reading boot sector\n"); > > - return -1; > > + return -EIO; > > } > > > > total_sector = bs.total_sect; > > @@ -997,7 +997,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > mydata->fatbuf = memalign(ARCH_DMA_MINALIGN, FATBUFSIZE); > > if (mydata->fatbuf == NULL) { > > debug("Error: allocating memory\n"); > > - return -1; > > + return -ENOMEM; > > } > > > > if (disk_read(cursect, > > @@ -1005,6 +1005,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > (mydata->clust_size) : > > PREFETCH_BLOCKS, do_fat_read_at_block) < 0) { > > debug("Error: reading rootdir block\n"); > > + ret = -EIO; > > goto exit; > > } > > dentptr = (dir_entry *) do_fat_read_at_block; > > @@ -1029,6 +1030,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > size); > > if (ret) { > > printf("Error: %llu overflow\n", size); > > + ret = -ENOSPC; > > goto exit; > > } > > } > > @@ -1036,6 +1038,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > ret = clear_fatent(mydata, start_cluster); > > if (ret) { > > printf("Error: clearing FAT entries\n"); > > + ret = -EIO; > > goto exit; > > } > > > > @@ -1045,12 +1048,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > ret = start_cluster = find_empty_cluster(mydata); > > if (ret < 0) { > > printf("Error: finding empty cluster\n"); > > + ret = -ENOSPC; > > goto exit; > > } > > > > ret = check_overflow(mydata, start_cluster, size); > > if (ret) { > > printf("Error: %llu overflow\n", size); > > + ret = -ENOSPC; > > goto exit; > > } > > > > @@ -1065,12 +1070,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > ret = start_cluster = find_empty_cluster(mydata); > > if (ret < 0) { > > printf("Error: finding empty cluster\n"); > > + ret = -ENOSPC; > > goto exit; > > } > > > > ret = check_overflow(mydata, start_cluster, size); > > if (ret) { > > printf("Error: %llu overflow\n", size); > > + ret = -ENOSPC; > > goto exit; > > } > > } else { > > @@ -1087,6 +1094,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > ret = set_contents(mydata, retdent, buffer, size, actwrite); > > if (ret < 0) { > > printf("Error: writing contents\n"); > > + ret = -EIO; > > goto exit; > > } > > debug("attempt to write 0x%llx bytes\n", *actwrite); > > @@ -1095,14 +1103,17 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, > > ret = flush_dirty_fat_buffer(mydata); > > if (ret) { > > printf("Error: flush fat buffer\n"); > > + ret = -EIO; > > goto exit; > > } > > > > /* Write directory table to device */ > > ret = set_cluster(mydata, dir_curclust, get_dentfromdir_block, > > mydata->clust_size * mydata->sect_size); > > - if (ret) > > + if (ret) { > > printf("Error: writing directory entry\n"); > > + ret = -EIO; > > + } > > > > exit: > > free(mydata->fatbuf); > > @@ -1114,7 +1125,7 @@ int file_fat_write(const char *filename, void *buffer, loff_t offset, > > { > > if (offset != 0) { > > printf("Error: non zero offset is currently not supported.\n"); > > - return -1; > > + return -EINVAL; > > } > > > > printf("writing %s\n", filename); > > >
diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 6c715a70f4..1e4f5af910 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -956,7 +956,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) { debug("error: reading boot sector\n"); - return -1; + return -EIO; } total_sector = bs.total_sect; @@ -997,7 +997,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, mydata->fatbuf = memalign(ARCH_DMA_MINALIGN, FATBUFSIZE); if (mydata->fatbuf == NULL) { debug("Error: allocating memory\n"); - return -1; + return -ENOMEM; } if (disk_read(cursect, @@ -1005,6 +1005,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, (mydata->clust_size) : PREFETCH_BLOCKS, do_fat_read_at_block) < 0) { debug("Error: reading rootdir block\n"); + ret = -EIO; goto exit; } dentptr = (dir_entry *) do_fat_read_at_block; @@ -1029,6 +1030,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, size); if (ret) { printf("Error: %llu overflow\n", size); + ret = -ENOSPC; goto exit; } } @@ -1036,6 +1038,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, ret = clear_fatent(mydata, start_cluster); if (ret) { printf("Error: clearing FAT entries\n"); + ret = -EIO; goto exit; } @@ -1045,12 +1048,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, ret = start_cluster = find_empty_cluster(mydata); if (ret < 0) { printf("Error: finding empty cluster\n"); + ret = -ENOSPC; goto exit; } ret = check_overflow(mydata, start_cluster, size); if (ret) { printf("Error: %llu overflow\n", size); + ret = -ENOSPC; goto exit; } @@ -1065,12 +1070,14 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, ret = start_cluster = find_empty_cluster(mydata); if (ret < 0) { printf("Error: finding empty cluster\n"); + ret = -ENOSPC; goto exit; } ret = check_overflow(mydata, start_cluster, size); if (ret) { printf("Error: %llu overflow\n", size); + ret = -ENOSPC; goto exit; } } else { @@ -1087,6 +1094,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, ret = set_contents(mydata, retdent, buffer, size, actwrite); if (ret < 0) { printf("Error: writing contents\n"); + ret = -EIO; goto exit; } debug("attempt to write 0x%llx bytes\n", *actwrite); @@ -1095,14 +1103,17 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, ret = flush_dirty_fat_buffer(mydata); if (ret) { printf("Error: flush fat buffer\n"); + ret = -EIO; goto exit; } /* Write directory table to device */ ret = set_cluster(mydata, dir_curclust, get_dentfromdir_block, mydata->clust_size * mydata->sect_size); - if (ret) + if (ret) { printf("Error: writing directory entry\n"); + ret = -EIO; + } exit: free(mydata->fatbuf); @@ -1114,7 +1125,7 @@ int file_fat_write(const char *filename, void *buffer, loff_t offset, { if (offset != 0) { printf("Error: non zero offset is currently not supported.\n"); - return -1; + return -EINVAL; } printf("writing %s\n", filename);
It would be good that FAT write function return error code instead of just returning -1 as fat_read_file() does. This patch attempts to address this issue although it is 'best effort (or estimate)' for now. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- fs/fat/fat_write.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)