diff mbox series

[06/17] fs: fat: write returns error code instead of -1

Message ID 20180720025723.6736-7-takahiro.akashi@linaro.org
State Superseded
Headers show
Series fs: fat: extend FAT write operations | expand

Commit Message

AKASHI Takahiro July 20, 2018, 2:57 a.m. UTC
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(-)

Comments

Heinrich Schuchardt July 20, 2018, 5:55 p.m. UTC | #1
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);
>
AKASHI Takahiro July 23, 2018, 8:32 a.m. UTC | #2
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 mbox series

Patch

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);