Re: [PATCH 01/15] staging: exfat: Clean up return codes - FFS_FULL

2019-10-24 Thread Valdis Klētnieks 
On Thu, 24 Oct 2019 10:59:04 -0700, Matthew Wilcox said:
> Wouldn't it be better to do this as:

> Patch 1: Change all these defines to -Exxx and remove the stupid 
> errno-changing
> blocks like this:

Well, except for the fact that the one for FFS_MEDIAERR required splitting the
uses into -ENODEV, -EIO, and -ENOENT.

Also, "and remover the stupid blocks" would be a second change, and I *thought*
the rule was "one thing, one patch".

> That way nobody actually needs to review patches 2-n; all of the changes
> are done in patch 1.

Reviewing a patch where you know that exactly one thing is supposed to happen
means scrolling through 14 occurrences of the pattern

if (num_alloced == 0)
-   ret = FFS_FULL;
+   ret = -ENOSPC;

goes *really* fast, and those comprise most of the bulk of the patchset.

And as I already mentioned, the "stupid looking blocks" will be removed in
a future patch.


pgpYfCslQ7sA9.pgp
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/15] staging: exfat: Clean up return codes - FFS_FULL

2019-10-24 Thread Matthew Wilcox 
On Thu, Oct 24, 2019 at 11:53:12AM -0400, Valdis Kletnieks wrote:
> Start cleaning up the odd scheme of return codes, starting with FFS_FULL

> +++ b/drivers/staging/exfat/exfat.h
> @@ -221,7 +221,6 @@ static inline u16 get_row_index(u16 i)
>  #define FFS_PERMISSIONERR   11
>  #define FFS_NOTOPENED   12
>  #define FFS_MAXOPENED   13
> -#define FFS_FULL14
>  #define FFS_EOF 15
>  #define FFS_DIRBUSY 16
>  #define FFS_MEMORYERR   17

Wouldn't it be better to do this as:

Patch 1: Change all these defines to -Exxx and remove the stupid errno-changing
blocks like this:

> @@ -2360,7 +2360,7 @@ static int exfat_create(struct inode *dir, struct 
> dentry *dentry, umode_t mode,
>   err = -EINVAL;
>   else if (err == FFS_FILEEXIST)
>   err = -EEXIST;
> - else if (err == FFS_FULL)
> + else if (err == -ENOSPC)
>   err = -ENOSPC;
>   else if (err == FFS_NAMETOOLONG)
>   err = -ENAMETOOLONG;

then patches 2-n remove individual FFS error codes.

That way nobody actually needs to review patches 2-n; all of the changes
are done in patch 1.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/15] staging: exfat: Clean up return codes - FFS_FULL

2019-10-24 Thread Valdis Kletnieks 
Start cleaning up the odd scheme of return codes, starting with FFS_FULL

Signed-off-by: Valdis Kletnieks 
---
 drivers/staging/exfat/exfat.h   |  1 -
 drivers/staging/exfat/exfat_core.c  | 10 +-
 drivers/staging/exfat/exfat_super.c | 16 
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/exfat/exfat.h b/drivers/staging/exfat/exfat.h
index 8738e41dd5a5..4aca4ae44a98 100644
--- a/drivers/staging/exfat/exfat.h
+++ b/drivers/staging/exfat/exfat.h
@@ -221,7 +221,6 @@ static inline u16 get_row_index(u16 i)
 #define FFS_PERMISSIONERR   11
 #define FFS_NOTOPENED   12
 #define FFS_MAXOPENED   13
-#define FFS_FULL14
 #define FFS_EOF 15
 #define FFS_DIRBUSY 16
 #define FFS_MEMORYERR   17
diff --git a/drivers/staging/exfat/exfat_core.c 
b/drivers/staging/exfat/exfat_core.c
index 7332e69fcbcd..af1ccd686e01 100644
--- a/drivers/staging/exfat/exfat_core.c
+++ b/drivers/staging/exfat/exfat_core.c
@@ -2369,7 +2369,7 @@ s32 create_dir(struct inode *inode, struct chain_t *p_dir,
/* find_empty_entry must be called before alloc_cluster */
dentry = find_empty_entry(inode, p_dir, num_entries);
if (dentry < 0)
-   return FFS_FULL;
+   return -ENOSPC;
 
clu.dir = CLUSTER_32(~0);
clu.size = 0;
@@ -2380,7 +2380,7 @@ s32 create_dir(struct inode *inode, struct chain_t *p_dir,
if (ret < 0)
return FFS_MEDIAERR;
else if (ret == 0)
-   return FFS_FULL;
+   return -ENOSPC;
 
ret = clear_cluster(sb, clu.dir);
if (ret != FFS_SUCCESS)
@@ -2472,7 +2472,7 @@ s32 create_file(struct inode *inode, struct chain_t 
*p_dir,
/* find_empty_entry must be called before alloc_cluster() */
dentry = find_empty_entry(inode, p_dir, num_entries);
if (dentry < 0)
-   return FFS_FULL;
+   return -ENOSPC;
 
/* (1) update the directory entry */
/* fill the dos name directory entry information of the created file.
@@ -2571,7 +2571,7 @@ s32 rename_file(struct inode *inode, struct chain_t 
*p_dir, s32 oldentry,
newentry = find_empty_entry(inode, p_dir, num_new_entries);
if (newentry < 0) {
buf_unlock(sb, sector_old);
-   return FFS_FULL;
+   return -ENOSPC;
}
 
epnew = get_entry_in_dir(sb, p_dir, newentry, _new);
@@ -2682,7 +2682,7 @@ s32 move_file(struct inode *inode, struct chain_t 
*p_olddir, s32 oldentry,
newentry = find_empty_entry(inode, p_newdir, num_new_entries);
if (newentry < 0) {
buf_unlock(sb, sector_mov);
-   return FFS_FULL;
+   return -ENOSPC;
}
 
epnew = get_entry_in_dir(sb, p_newdir, newentry, _new);
diff --git a/drivers/staging/exfat/exfat_super.c 
b/drivers/staging/exfat/exfat_super.c
index 0264be92c2be..273fe2310e76 100644
--- a/drivers/staging/exfat/exfat_super.c
+++ b/drivers/staging/exfat/exfat_super.c
@@ -1045,7 +1045,7 @@ static int ffsWriteFile(struct inode *inode, struct 
file_id_t *fid,
*wcount = write_bytes;
 
if (num_alloced == 0)
-   ret = FFS_FULL;
+   ret = -ENOSPC;
 
else if (p_fs->dev_ejected)
ret = FFS_MEDIAERR;
@@ -1801,7 +1801,7 @@ static int ffsMapCluster(struct inode *inode, s32 
clu_offset, u32 *clu)
ret = FFS_MEDIAERR;
goto out;
} else if (num_alloced == 0) {
-   ret = FFS_FULL;
+   ret = -ENOSPC;
goto out;
}
 
@@ -2360,7 +2360,7 @@ static int exfat_create(struct inode *dir, struct dentry 
*dentry, umode_t mode,
err = -EINVAL;
else if (err == FFS_FILEEXIST)
err = -EEXIST;
-   else if (err == FFS_FULL)
+   else if (err == -ENOSPC)
err = -ENOSPC;
else if (err == FFS_NAMETOOLONG)
err = -ENAMETOOLONG;
@@ -2571,7 +2571,7 @@ static int exfat_symlink(struct inode *dir, struct dentry 
*dentry,
err = -EINVAL;
else if (err == FFS_FILEEXIST)
err = -EEXIST;
-   else if (err == FFS_FULL)
+   else if (err == -ENOSPC)
err = -ENOSPC;
else
err = -EIO;
@@ -2583,7 +2583,7 @@ static int exfat_symlink(struct inode *dir, struct dentry 
*dentry,
if (err) {
ffsRemoveFile(dir, );
 
-   if (err == FFS_FULL)
+   if (err == -ENOSPC)
err = -ENOSPC;
else
err = -EIO;
@@ -2641,7 +2641,7 @@ static int