[PATCH V2 0/9 linux-next] udf: improve error management

[PATCH V2 0/9 linux-next] udf: improve error management

[Fabian Frederick]

This small patchset tries to solve error management in udf
unicode and callsites.

Thanks a lot to Jan Kara for all suggestions and code reviewing !

V2:
	-Cc to lkml
	-Patch 2 and 8 updated (See commit log)
	-Adding patch 9

Of course this patchset would need extended testing.

Fabian Frederick (9):
  udf: udf_get_filename(): return -ENOMEM when allocation fails
  udf: remove unneccessary test in udf_build_ustr_exact()
  udf: unicode: update function name in comments
  udf: improve error management in udf_CS0toUTF8()
  udf: improve error management in udf_CS0toNLS()
  udf: bug on exotic flag in udf_get_filename()
  udf: return error when newIndex is 0 in udf_translate_to_linux()
  udf: propagate udf_get_filename() errors
  udf: add function definition for udf_find_entry()

 fs/udf/dir.c     |  2 +-
 fs/udf/namei.c   | 85 +++++++++++++++++++++++++++++++++++++++++++-------------
 fs/udf/super.c   | 23 +++++++++------
 fs/udf/symlink.c |  3 ++
 fs/udf/unicode.c | 52 +++++++++++++++++-----------------
 5 files changed, 109 insertions(+), 56 deletions(-)

-- 
1.9.1

[PATCH V2 1/9 linux-next] udf: udf_get_filename(): return -ENOMEM when allocation fails

[Fabian Frederick]

udf_pc_to_char() now returns error accordingly.
udf_readdir() and udf_find_entry() process is done on
positive result.

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/dir.c     |  2 +-
 fs/udf/namei.c   |  3 ++-
 fs/udf/symlink.c |  3 +++
 fs/udf/unicode.c | 12 +++++++-----
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index 541a12b..fcf227e 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
 		}
 
 		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
-		if (!flen)
+		if (flen <= 0)
 			continue;
 
 		tloc = lelb_to_cpu(cfi.icb.extLocation);
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index fbf3d90..59b340c 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 			continue;
 
 		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
-		if (flen && udf_match(flen, fname, child->len, child->name))
+		if ((flen > 0) && udf_match(flen, fname, child->len,
+					    child->name))
 			goto out_ok;
 	}
 
diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
index 8dfbc40..862535b 100644
--- a/fs/udf/symlink.c
+++ b/fs/udf/symlink.c
@@ -82,6 +82,9 @@ static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
 			comp_len = udf_get_filename(sb, pc->componentIdent,
 						    pc->lengthComponentIdent,
 						    p, tolen);
+			if (comp_len < 0)
+				return comp_len;
+
 			p += comp_len;
 			tolen -= comp_len;
 			if (tolen == 0)
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index b84fee3..4911c1d 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -338,15 +338,17 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 		     uint8_t *dname, int dlen)
 {
 	struct ustr *filename, *unifilename;
-	int len = 0;
+	int ret = 0;
 
 	filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
 	if (!filename)
-		return 0;
+		return -ENOMEM;
 
 	unifilename = kmalloc(sizeof(struct ustr), GFP_NOFS);
-	if (!unifilename)
+	if (!unifilename) {
+		ret = -ENOMEM;
 		goto out1;
+	}
 
 	if (udf_build_ustr_exact(unifilename, sname, slen))
 		goto out2;
@@ -367,14 +369,14 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 	} else
 		goto out2;
 
-	len = udf_translate_to_linux(dname, dlen,
+	ret = udf_translate_to_linux(dname, dlen,
 				     filename->u_name, filename->u_len,
 				     unifilename->u_name, unifilename->u_len);
 out2:
 	kfree(unifilename);
 out1:
 	kfree(filename);
-	return len;
+	return ret;
 }
 
 int udf_put_filename(struct super_block *sb, const uint8_t *sname,
-- 
1.9.1

[PATCH V2 2/9 linux-next] udf: remove unnecessary test in udf_build_ustr_exact()

[Fabian Frederick]

We can remove parameter checks:

udf_build_ustr_exact() is only called by udf_get_filename()
which now assures dest is not NULL

udf_find_entry() and udf_readdir() call udf_get_filename()
after checking sname
udf_symlink_filler() calls udf_pc_to_char() with sname=kmap(page).

udf_find_entry() and udf_readdir() call udf_get_filename with UDF_NAME_LEN
udf_pc_to_char() with PAGE_SIZE

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
V2: Return -EIO if slen = 0 in udf_get_filename()
    (suggested by Jan Kara)

 fs/udf/unicode.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 4911c1d..e2c079a 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -68,17 +68,12 @@ int udf_build_ustr(struct ustr *dest, dstring *ptr, int size)
 /*
  * udf_build_ustr_exact
  */
-static int udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
+static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
 {
-	if ((!dest) || (!ptr) || (!exactsize))
-		return -1;
-
 	memset(dest, 0, sizeof(struct ustr));
 	dest->u_cmpID = ptr[0];
 	dest->u_len = exactsize - 1;
 	memcpy(dest->u_name, ptr + 1, exactsize - 1);
-
-	return 0;
 }
 
 /*
@@ -340,6 +335,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 	struct ustr *filename, *unifilename;
 	int ret = 0;
 
+	if (!slen)
+		return -EIO;
+
 	filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
 	if (!filename)
 		return -ENOMEM;
@@ -350,9 +348,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 		goto out1;
 	}
 
-	if (udf_build_ustr_exact(unifilename, sname, slen))
-		goto out2;
-
+	udf_build_ustr_exact(unifilename, sname, slen);
 	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
 		if (!udf_CS0toUTF8(filename, unifilename)) {
 			udf_debug("Failed in udf_get_filename: sname = %s\n",
-- 
1.9.1

[PATCH V2 3/9 linux-next] udf: unicode: update function name in comments

[Fabian Frederick]

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/unicode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index e2c079a..41c3bef 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -77,7 +77,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
 }
 
 /*
- * udf_ocu_to_utf8
+ * udf_CS0toUTF8
  *
  * PURPOSE
  *	Convert OSTA Compressed Unicode to the UTF-8 equivalent.
@@ -149,7 +149,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
 
 /*
  *
- * udf_utf8_to_ocu
+ * udf_UTF8toCS0
  *
  * PURPOSE
  *	Convert UTF-8 to the OSTA Compressed Unicode equivalent.
-- 
1.9.1

[PATCH V2 4/9 linux-next] udf: improve error management in udf_CS0toUTF8()

[Fabian Frederick]

udf_CS0toUTF8() now returns -EINVAL on error.
udf_load_pvoldesc() and udf_get_filename() do the same.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/super.c   | 23 ++++++++++++++---------
 fs/udf/unicode.c |  9 +++++----
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 6299f34..c6a8f5f 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -927,17 +927,22 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
 #endif
 	}
 
-	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32))
-		if (udf_CS0toUTF8(outstr, instr)) {
-			strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
-				outstr->u_len > 31 ? 31 : outstr->u_len);
-			udf_debug("volIdent[] = '%s'\n",
-				  UDF_SB(sb)->s_volume_ident);
-		}
+	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32)) {
+		ret = udf_CS0toUTF8(outstr, instr);
+		if (ret < 0)
+			goto out_bh;
+
+		strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
+			outstr->u_len > 31 ? 31 : outstr->u_len);
+		udf_debug("volIdent[] = '%s'\n", UDF_SB(sb)->s_volume_ident);
+	}
 
 	if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
-		if (udf_CS0toUTF8(outstr, instr))
-			udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
+		ret = udf_CS0toUTF8(outstr, instr);
+		if (ret < 0)
+			goto out_bh;
+
+		udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
 
 	ret = 0;
 out_bh:
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 41c3bef..9008a36 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -89,7 +89,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
  * 				both of type "struct ustr *"
  *
  * POST-CONDITIONS
- *	<return>		Zero on success.
+ *	<return>		>= 0 on success.
  *
  * HISTORY
  *	November 12, 1997 - Andrew E. Mileski
@@ -104,7 +104,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
 	ocu_len = ocu_i->u_len;
 	if (ocu_len == 0) {
 		memset(utf_o, 0, sizeof(struct ustr));
-		return 0;
+		return -EINVAL;
 	}
 
 	cmp_id = ocu_i->u_cmpID;
@@ -112,7 +112,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
 		memset(utf_o, 0, sizeof(struct ustr));
 		pr_err("unknown compression code (%d) stri=%s\n",
 		       cmp_id, ocu_i->u_name);
-		return 0;
+		return -EINVAL;
 	}
 
 	ocu = ocu_i->u_name;
@@ -350,7 +350,8 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 
 	udf_build_ustr_exact(unifilename, sname, slen);
 	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
-		if (!udf_CS0toUTF8(filename, unifilename)) {
+		ret = udf_CS0toUTF8(filename, unifilename);
+		if (ret < 0) {
 			udf_debug("Failed in udf_get_filename: sname = %s\n",
 				  sname);
 			goto out2;
-- 
1.9.1

[PATCH V2 5/9 linux-next] udf: improve error management in udf_CS0toNLS()

[Fabian Frederick]

Only callsite udf_get_filename() now returns
error code as well.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/unicode.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 9008a36..488a838 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -257,7 +257,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
 	ocu_len = ocu_i->u_len;
 	if (ocu_len == 0) {
 		memset(utf_o, 0, sizeof(struct ustr));
-		return 0;
+		return -EINVAL;
 	}
 
 	cmp_id = ocu_i->u_cmpID;
@@ -265,7 +265,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
 		memset(utf_o, 0, sizeof(struct ustr));
 		pr_err("unknown compression code (%d) stri=%s\n",
 		       cmp_id, ocu_i->u_name);
-		return 0;
+		return -EINVAL;
 	}
 
 	ocu = ocu_i->u_name;
@@ -357,8 +357,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 			goto out2;
 		}
 	} else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
-		if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
-				  unifilename)) {
+		ret = udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
+				   unifilename);
+		if (ret < 0) {
 			udf_debug("Failed in udf_get_filename: sname = %s\n",
 				  sname);
 			goto out2;
-- 
1.9.1

[PATCH V2 6/9 linux-next] udf: bug on exotic flag in udf_get_filename()

[Fabian Frederick]

UDF volume is only mounted with UDF_FLAG_UTF8
or UDF_FLAG_NLS_MAP (see fill udf_fill_super().
BUG() if we have something different in udf_get_filename()

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/unicode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 488a838..f37123b 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -365,7 +365,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 			goto out2;
 		}
 	} else
-		goto out2;
+		BUG();
 
 	ret = udf_translate_to_linux(dname, dlen,
 				     filename->u_name, filename->u_len,
-- 
1.9.1

[PATCH V2 7/9 linux-next] udf: return error when newIndex is 0 in udf_translate_to_linux()

[Fabian Frederick]

udf_get_filename() and its callsites considered 0 as an error
without propagating an error value.

udf_translate_to_linux() now returns -EINVAL when newIndex is 0.
other functions are updated accordingly.

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/dir.c     | 2 +-
 fs/udf/namei.c   | 2 +-
 fs/udf/unicode.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index fcf227e..541d9c6 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
 		}
 
 		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
-		if (flen <= 0)
+		if (flen < 0)
 			continue;
 
 		tloc = lelb_to_cpu(cfi.icb.extLocation);
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 59b340c..dd648b7 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -234,7 +234,7 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 			continue;
 
 		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
-		if ((flen > 0) && udf_match(flen, fname, child->len,
+		if ((flen >= 0) && udf_match(flen, fname, child->len,
 					    child->name))
 			goto out_ok;
 	}
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index f37123b..3b1efbb 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -333,7 +333,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 		     uint8_t *dname, int dlen)
 {
 	struct ustr *filename, *unifilename;
-	int ret = 0;
+	int ret;
 
 	if (!slen)
 		return -EIO;
@@ -492,5 +492,5 @@ static int udf_translate_to_linux(uint8_t *newName, int newLen,
 		}
 	}
 
-	return newIndex;
+	return newIndex ? : -EINVAL;
 }
-- 
1.9.1

[PATCH V2 8/9 linux-next] udf: propagate udf_get_filename() errors

[Fabian Frederick]

-Return udf_get_filename() error from udf_readdir()
-Return -ENOMEM from udf_find_entry() when unable to allocate
fname and udf_get_filename() error
-udf_find_entry() callsites are also updated:
udf_lookup(), udf_rmdir(), udf_unlink() and udf_rename()

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
V2:
	-Don't set error in udf_readdir()
	-Improve code flow
	-Merge if(nfi) if (!inode) in udf_rename()

 fs/udf/namei.c | 70 ++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 51 insertions(+), 19 deletions(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index dd648b7..891c067 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -167,8 +167,11 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 	fibh->soffset = fibh->eoffset = f_pos & (sb->s_blocksize - 1);
 	if (dinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
 		if (inode_bmap(dir, f_pos >> sb->s_blocksize_bits, &epos,
-		    &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30))
+		    &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30)) {
+			fi = ERR_PTR(-EINVAL);
 			goto out_err;
+		}
+
 		block = udf_get_lb_pblock(sb, &eloc, offset);
 		if ((++offset << sb->s_blocksize_bits) < elen) {
 			if (dinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT)
@@ -179,19 +182,25 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 			offset = 0;
 
 		fibh->sbh = fibh->ebh = udf_tread(sb, block);
-		if (!fibh->sbh)
+		if (!fibh->sbh) {
+			fi = ERR_PTR(-EINVAL);
 			goto out_err;
+		}
 	}
 
 	fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
-	if (!fname)
+	if (!fname) {
+		fi = ERR_PTR(-ENOMEM);
 		goto out_err;
+	}
 
 	while (f_pos < size) {
 		fi = udf_fileident_read(dir, &f_pos, fibh, cfi, &epos, &eloc,
 					&elen, &offset);
-		if (!fi)
+		if (!fi) {
+			fi = ERR_PTR(-EINVAL);
 			goto out_err;
+		}
 
 		liu = le16_to_cpu(cfi->lengthOfImpUse);
 		lfi = cfi->lengthFileIdent;
@@ -234,13 +243,18 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 			continue;
 
 		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
+		if (flen < 0) {
+			fi = ERR_PTR(flen);
+			goto out_err;
+		}
+
 		if ((flen >= 0) && udf_match(flen, fname, child->len,
 					    child->name))
 			goto out_ok;
 	}
 
-out_err:
 	fi = NULL;
+out_err:
 	if (fibh->sbh != fibh->ebh)
 		brelse(fibh->ebh);
 	brelse(fibh->sbh);
@@ -257,6 +271,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
 	struct inode *inode = NULL;
 	struct fileIdentDesc cfi;
 	struct udf_fileident_bh fibh;
+	struct fileIdentDesc *fi;
 
 	if (dentry->d_name.len > UDF_NAME_LEN - 2)
 		return ERR_PTR(-ENAMETOOLONG);
@@ -276,7 +291,11 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
 	} else
 #endif /* UDF_RECOVERY */
 
-	if (udf_find_entry(dir, &dentry->d_name, &fibh, &cfi)) {
+	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
+	if (IS_ERR(fi))
+		return ERR_CAST(fi);
+
+	if (fi) {
 		struct kernel_lb_addr loc;
 
 		if (fibh.sbh != fibh.ebh)
@@ -774,8 +793,11 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
 
 	retval = -ENOENT;
 	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
-	if (!fi)
+	if (IS_ERR_OR_NULL(fi)) {
+		if (fi)
+			retval = PTR_ERR(fi);
 		goto out;
+	}
 
 	retval = -EIO;
 	tloc = lelb_to_cpu(cfi.icb.extLocation);
@@ -817,8 +839,12 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)
 
 	retval = -ENOENT;
 	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
-	if (!fi)
+
+	if (IS_ERR_OR_NULL(fi)) {
+		if (fi)
+			retval = PTR_ERR(fi);
 		goto out;
+	}
 
 	retval = -EIO;
 	tloc = lelb_to_cpu(cfi.icb.extLocation);
@@ -1047,24 +1073,30 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct udf_inode_info *old_iinfo = UDF_I(old_inode);
 
 	ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi);
-	if (ofi) {
-		if (ofibh.sbh != ofibh.ebh)
-			brelse(ofibh.ebh);
-		brelse(ofibh.sbh);
+	if (IS_ERR(ofi)) {
+		retval = PTR_ERR(ofi);
+		goto end_rename;
 	}
+
+	if (ofibh.sbh != ofibh.ebh)
+		brelse(ofibh.ebh);
+
+	brelse(ofibh.sbh);
 	tloc = lelb_to_cpu(ocfi.icb.extLocation);
 	if (!ofi || udf_get_lb_pblock(old_dir->i_sb, &tloc, 0)
 	    != old_inode->i_ino)
 		goto end_rename;
 
 	nfi = udf_find_entry(new_dir, &new_dentry->d_name, &nfibh, &ncfi);
-	if (nfi) {
-		if (!new_inode) {
-			if (nfibh.sbh != nfibh.ebh)
-				brelse(nfibh.ebh);
-			brelse(nfibh.sbh);
-			nfi = NULL;
-		}
+	if (IS_ERR(nfi)) {
+		retval = PTR_ERR(nfi);
+		goto end_rename;
+	}
+	if (nfi && !new_inode) {
+		if (nfibh.sbh != nfibh.ebh)
+			brelse(nfibh.ebh);
+		brelse(nfibh.sbh);
+		nfi = NULL;
 	}
 	if (S_ISDIR(old_inode->i_mode)) {
 		int offset = udf_ext0_offset(old_inode);
-- 
1.9.1

[PATCH V2 9/9 linux-next] udf: add function definition for udf_find_entry()

[Fabian Frederick]

Function return changed lately.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/namei.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 891c067..1f8e4d0 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -138,6 +138,18 @@ int udf_write_fi(struct inode *inode, struct fileIdentDesc *cfi,
 	return 0;
 }
 
+/**
+ * udf_find_entry - find entry in given directory.
+ *
+ * @dir:	directory inode
+ * @child:	qstr of the name
+ * @fibh:	udf file identifier buffer head
+ * @cfi:	current file identifier descriptor
+ *
+ * Return pointer to file identifier, NULL when nothing found or error code.
+ */
+
+
 static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 					    const struct qstr *child,
 					    struct udf_fileident_bh *fibh,
-- 
1.9.1

Re: [PATCH V2 1/9 linux-next] udf: udf_get_filename(): return -ENOMEM when allocation fails

[Jan Kara]

On Wed 08-04-15 21:23:51, Fabian Frederick wrote:
> udf_pc_to_char() now returns error accordingly.
> udf_readdir() and udf_find_entry() process is done on
> positive result.
  Thanks. Added to my tree with improved changelog.

								Honza
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  fs/udf/dir.c     |  2 +-
>  fs/udf/namei.c   |  3 ++-
>  fs/udf/symlink.c |  3 +++
>  fs/udf/unicode.c | 12 +++++++-----
>  4 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> index 541a12b..fcf227e 100644
> --- a/fs/udf/dir.c
> +++ b/fs/udf/dir.c
> @@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
>  		}
>  
>  		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> -		if (!flen)
> +		if (flen <= 0)
>  			continue;
>  
>  		tloc = lelb_to_cpu(cfi.icb.extLocation);
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index fbf3d90..59b340c 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
>  			continue;
>  
>  		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> -		if (flen && udf_match(flen, fname, child->len, child->name))
> +		if ((flen > 0) && udf_match(flen, fname, child->len,
> +					    child->name))
>  			goto out_ok;
>  	}
>  
> diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
> index 8dfbc40..862535b 100644
> --- a/fs/udf/symlink.c
> +++ b/fs/udf/symlink.c
> @@ -82,6 +82,9 @@ static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
>  			comp_len = udf_get_filename(sb, pc->componentIdent,
>  						    pc->lengthComponentIdent,
>  						    p, tolen);
> +			if (comp_len < 0)
> +				return comp_len;
> +
>  			p += comp_len;
>  			tolen -= comp_len;
>  			if (tolen == 0)
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index b84fee3..4911c1d 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -338,15 +338,17 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  		     uint8_t *dname, int dlen)
>  {
>  	struct ustr *filename, *unifilename;
> -	int len = 0;
> +	int ret = 0;
>  
>  	filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
>  	if (!filename)
> -		return 0;
> +		return -ENOMEM;
>  
>  	unifilename = kmalloc(sizeof(struct ustr), GFP_NOFS);
> -	if (!unifilename)
> +	if (!unifilename) {
> +		ret = -ENOMEM;
>  		goto out1;
> +	}
>  
>  	if (udf_build_ustr_exact(unifilename, sname, slen))
>  		goto out2;
> @@ -367,14 +369,14 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  	} else
>  		goto out2;
>  
> -	len = udf_translate_to_linux(dname, dlen,
> +	ret = udf_translate_to_linux(dname, dlen,
>  				     filename->u_name, filename->u_len,
>  				     unifilename->u_name, unifilename->u_len);
>  out2:
>  	kfree(unifilename);
>  out1:
>  	kfree(filename);
> -	return len;
> +	return ret;
>  }
>  
>  int udf_put_filename(struct super_block *sb, const uint8_t *sname,
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 2/9 linux-next] udf: remove unnecessary test in udf_build_ustr_exact()

[Jan Kara]

On Wed 08-04-15 21:23:52, Fabian Frederick wrote:
> We can remove parameter checks:
> 
> udf_build_ustr_exact() is only called by udf_get_filename()
> which now assures dest is not NULL
> 
> udf_find_entry() and udf_readdir() call udf_get_filename()
> after checking sname
> udf_symlink_filler() calls udf_pc_to_char() with sname=kmap(page).
> 
> udf_find_entry() and udf_readdir() call udf_get_filename with UDF_NAME_LEN
> udf_pc_to_char() with PAGE_SIZE
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  Thanks. Added to my tree.

								Honza

> ---
> V2: Return -EIO if slen = 0 in udf_get_filename()
>     (suggested by Jan Kara)
> 
>  fs/udf/unicode.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 4911c1d..e2c079a 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -68,17 +68,12 @@ int udf_build_ustr(struct ustr *dest, dstring *ptr, int size)
>  /*
>   * udf_build_ustr_exact
>   */
> -static int udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
> +static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
>  {
> -	if ((!dest) || (!ptr) || (!exactsize))
> -		return -1;
> -
>  	memset(dest, 0, sizeof(struct ustr));
>  	dest->u_cmpID = ptr[0];
>  	dest->u_len = exactsize - 1;
>  	memcpy(dest->u_name, ptr + 1, exactsize - 1);
> -
> -	return 0;
>  }
>  
>  /*
> @@ -340,6 +335,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  	struct ustr *filename, *unifilename;
>  	int ret = 0;
>  
> +	if (!slen)
> +		return -EIO;
> +
>  	filename = kmalloc(sizeof(struct ustr), GFP_NOFS);
>  	if (!filename)
>  		return -ENOMEM;
> @@ -350,9 +348,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  		goto out1;
>  	}
>  
> -	if (udf_build_ustr_exact(unifilename, sname, slen))
> -		goto out2;
> -
> +	udf_build_ustr_exact(unifilename, sname, slen);
>  	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
>  		if (!udf_CS0toUTF8(filename, unifilename)) {
>  			udf_debug("Failed in udf_get_filename: sname = %s\n",
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 3/9 linux-next] udf: unicode: update function name in comments

[Jan Kara]

On Wed 08-04-15 21:23:53, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  Thanks. Applied.
								Honza

> ---
>  fs/udf/unicode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index e2c079a..41c3bef 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -77,7 +77,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
>  }
>  
>  /*
> - * udf_ocu_to_utf8
> + * udf_CS0toUTF8
>   *
>   * PURPOSE
>   *	Convert OSTA Compressed Unicode to the UTF-8 equivalent.
> @@ -149,7 +149,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
>  
>  /*
>   *
> - * udf_utf8_to_ocu
> + * udf_UTF8toCS0
>   *
>   * PURPOSE
>   *	Convert UTF-8 to the OSTA Compressed Unicode equivalent.
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 4/9 linux-next] udf: improve error management in udf_CS0toUTF8()

[Jan Kara]

On Wed 08-04-15 21:23:54, Fabian Frederick wrote:
> udf_CS0toUTF8() now returns -EINVAL on error.
> udf_load_pvoldesc() and udf_get_filename() do the same.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  After some thought it's probably better API that udf_CS0toUTF8() simply
returns 0 when 0-lenght string is provided as an input (there's nothing
invalid in that from the point of view of that conversion functions). We
cannot actually ever call udf_CS0toUTF8() with such string currently but I
think it's better for future. So I've changed this and merged the patch.

								Honza
> ---
>  fs/udf/super.c   | 23 ++++++++++++++---------
>  fs/udf/unicode.c |  9 +++++----
>  2 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 6299f34..c6a8f5f 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -927,17 +927,22 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
>  #endif
>  	}
>  
> -	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32))
> -		if (udf_CS0toUTF8(outstr, instr)) {
> -			strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> -				outstr->u_len > 31 ? 31 : outstr->u_len);
> -			udf_debug("volIdent[] = '%s'\n",
> -				  UDF_SB(sb)->s_volume_ident);
> -		}
> +	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32)) {
> +		ret = udf_CS0toUTF8(outstr, instr);
> +		if (ret < 0)
> +			goto out_bh;
> +
> +		strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> +			outstr->u_len > 31 ? 31 : outstr->u_len);
> +		udf_debug("volIdent[] = '%s'\n", UDF_SB(sb)->s_volume_ident);
> +	}
>  
>  	if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
> -		if (udf_CS0toUTF8(outstr, instr))
> -			udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
> +		ret = udf_CS0toUTF8(outstr, instr);
> +		if (ret < 0)
> +			goto out_bh;
> +
> +		udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
>  
>  	ret = 0;
>  out_bh:
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 41c3bef..9008a36 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -89,7 +89,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
>   * 				both of type "struct ustr *"
>   *
>   * POST-CONDITIONS
> - *	<return>		Zero on success.
> + *	<return>		>= 0 on success.
>   *
>   * HISTORY
>   *	November 12, 1997 - Andrew E. Mileski
> @@ -104,7 +104,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
>  	ocu_len = ocu_i->u_len;
>  	if (ocu_len == 0) {
>  		memset(utf_o, 0, sizeof(struct ustr));
> -		return 0;
> +		return -EINVAL;
>  	}
>  
>  	cmp_id = ocu_i->u_cmpID;
> @@ -112,7 +112,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
>  		memset(utf_o, 0, sizeof(struct ustr));
>  		pr_err("unknown compression code (%d) stri=%s\n",
>  		       cmp_id, ocu_i->u_name);
> -		return 0;
> +		return -EINVAL;
>  	}
>  
>  	ocu = ocu_i->u_name;
> @@ -350,7 +350,8 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  
>  	udf_build_ustr_exact(unifilename, sname, slen);
>  	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
> -		if (!udf_CS0toUTF8(filename, unifilename)) {
> +		ret = udf_CS0toUTF8(filename, unifilename);
> +		if (ret < 0) {
>  			udf_debug("Failed in udf_get_filename: sname = %s\n",
>  				  sname);
>  			goto out2;
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 5/9 linux-next] udf: improve error management in udf_CS0toNLS()

[Jan Kara]

On Wed 08-04-15 21:23:55, Fabian Frederick wrote:
> Only callsite udf_get_filename() now returns
> error code as well.
  Did same modification as in the previous patch. Otherwise OK, so I've
merged the patch.

								Honza
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  fs/udf/unicode.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 9008a36..488a838 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -257,7 +257,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
>  	ocu_len = ocu_i->u_len;
>  	if (ocu_len == 0) {
>  		memset(utf_o, 0, sizeof(struct ustr));
> -		return 0;
> +		return -EINVAL;
>  	}
>  
>  	cmp_id = ocu_i->u_cmpID;
> @@ -265,7 +265,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
>  		memset(utf_o, 0, sizeof(struct ustr));
>  		pr_err("unknown compression code (%d) stri=%s\n",
>  		       cmp_id, ocu_i->u_name);
> -		return 0;
> +		return -EINVAL;
>  	}
>  
>  	ocu = ocu_i->u_name;
> @@ -357,8 +357,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  			goto out2;
>  		}
>  	} else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
> -		if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> -				  unifilename)) {
> +		ret = udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> +				   unifilename);
> +		if (ret < 0) {
>  			udf_debug("Failed in udf_get_filename: sname = %s\n",
>  				  sname);
>  			goto out2;
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 6/9 linux-next] udf: bug on exotic flag in udf_get_filename()

[Jan Kara]

On Wed 08-04-15 21:23:56, Fabian Frederick wrote:
> UDF volume is only mounted with UDF_FLAG_UTF8
> or UDF_FLAG_NLS_MAP (see fill udf_fill_super().
> BUG() if we have something different in udf_get_filename()
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  Thanks. Merged.
								Honza

> ---
>  fs/udf/unicode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index 488a838..f37123b 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -365,7 +365,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  			goto out2;
>  		}
>  	} else
> -		goto out2;
> +		BUG();
>  
>  	ret = udf_translate_to_linux(dname, dlen,
>  				     filename->u_name, filename->u_len,
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 7/9 linux-next] udf: return error when newIndex is 0 in udf_translate_to_linux()

[Jan Kara]

On Wed 08-04-15 21:23:57, Fabian Frederick wrote:
> udf_get_filename() and its callsites considered 0 as an error
> without propagating an error value.
> 
> udf_translate_to_linux() now returns -EINVAL when newIndex is 0.
> other functions are updated accordingly.
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  I've updated modified this patch and the changelog to do the check for
zero length filename in udf_get_filename() and not in
udf_translate_to_linux(). Since the second is just a general conversion
function and zero length string as a result is fine with that. But zero
length file name isn't correct and that's why udf_get_filename() should
generate the error. I've also updated the changelog accordingly.

								Honza

> ---
>  fs/udf/dir.c     | 2 +-
>  fs/udf/namei.c   | 2 +-
>  fs/udf/unicode.c | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> index fcf227e..541d9c6 100644
> --- a/fs/udf/dir.c
> +++ b/fs/udf/dir.c
> @@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
>  		}
>  
>  		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> -		if (flen <= 0)
> +		if (flen < 0)
>  			continue;
>  
>  		tloc = lelb_to_cpu(cfi.icb.extLocation);
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 59b340c..dd648b7 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -234,7 +234,7 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
>  			continue;
>  
>  		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> -		if ((flen > 0) && udf_match(flen, fname, child->len,
> +		if ((flen >= 0) && udf_match(flen, fname, child->len,
>  					    child->name))
>  			goto out_ok;
>  	}
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index f37123b..3b1efbb 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -333,7 +333,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
>  		     uint8_t *dname, int dlen)
>  {
>  	struct ustr *filename, *unifilename;
> -	int ret = 0;
> +	int ret;
>  
>  	if (!slen)
>  		return -EIO;
> @@ -492,5 +492,5 @@ static int udf_translate_to_linux(uint8_t *newName, int newLen,
>  		}
>  	}
>  
> -	return newIndex;
> +	return newIndex ? : -EINVAL;
>  }
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 9/9 linux-next] udf: add function definition for udf_find_entry()

[Jan Kara]

On Wed 08-04-15 21:23:59, Fabian Frederick wrote:
> Function return changed lately.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  I've significantly extended the documentation of udf_find_entry() and
folded this patch into the previous one.

								Honza
> ---
>  fs/udf/namei.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 891c067..1f8e4d0 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -138,6 +138,18 @@ int udf_write_fi(struct inode *inode, struct fileIdentDesc *cfi,
>  	return 0;
>  }
>  
> +/**
> + * udf_find_entry - find entry in given directory.
> + *
> + * @dir:	directory inode
> + * @child:	qstr of the name
> + * @fibh:	udf file identifier buffer head
> + * @cfi:	current file identifier descriptor
> + *
> + * Return pointer to file identifier, NULL when nothing found or error code.
> + */
> +
> +
>  static struct fileIdentDesc *udf_find_entry(struct inode *dir,
>  					    const struct qstr *child,
>  					    struct udf_fileident_bh *fibh,
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH V2 8/9 linux-next] udf: propagate udf_get_filename() errors

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 5467 bytes --]

On Wed 08-04-15 21:23:58, Fabian Frederick wrote:
> -Return udf_get_filename() error from udf_readdir()
> -Return -ENOMEM from udf_find_entry() when unable to allocate
> fname and udf_get_filename() error
> -udf_find_entry() callsites are also updated:
> udf_lookup(), udf_rmdir(), udf_unlink() and udf_rename()
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  Thanks. I've merged this patch with slight modification -
udf_find_entry() should return -EIO instead of -EINVAL in all the cases.
I've also folded the documentation of udf_find_entry() into this patch.
Attached is the result.

								Honza
> ---
> V2:
> 	-Don't set error in udf_readdir()
> 	-Improve code flow
> 	-Merge if(nfi) if (!inode) in udf_rename()
> 
>  fs/udf/namei.c | 70 ++++++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 51 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index dd648b7..891c067 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -167,8 +167,11 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
>  	fibh->soffset = fibh->eoffset = f_pos & (sb->s_blocksize - 1);
>  	if (dinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
>  		if (inode_bmap(dir, f_pos >> sb->s_blocksize_bits, &epos,
> -		    &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30))
> +		    &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30)) {
> +			fi = ERR_PTR(-EINVAL);
>  			goto out_err;
> +		}
> +
>  		block = udf_get_lb_pblock(sb, &eloc, offset);
>  		if ((++offset << sb->s_blocksize_bits) < elen) {
>  			if (dinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT)
> @@ -179,19 +182,25 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
>  			offset = 0;
>  
>  		fibh->sbh = fibh->ebh = udf_tread(sb, block);
> -		if (!fibh->sbh)
> +		if (!fibh->sbh) {
> +			fi = ERR_PTR(-EINVAL);
>  			goto out_err;
> +		}
>  	}
>  
>  	fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
> -	if (!fname)
> +	if (!fname) {
> +		fi = ERR_PTR(-ENOMEM);
>  		goto out_err;
> +	}
>  
>  	while (f_pos < size) {
>  		fi = udf_fileident_read(dir, &f_pos, fibh, cfi, &epos, &eloc,
>  					&elen, &offset);
> -		if (!fi)
> +		if (!fi) {
> +			fi = ERR_PTR(-EINVAL);
>  			goto out_err;
> +		}
>  
>  		liu = le16_to_cpu(cfi->lengthOfImpUse);
>  		lfi = cfi->lengthFileIdent;
> @@ -234,13 +243,18 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
>  			continue;
>  
>  		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> +		if (flen < 0) {
> +			fi = ERR_PTR(flen);
> +			goto out_err;
> +		}
> +
>  		if ((flen >= 0) && udf_match(flen, fname, child->len,
>  					    child->name))
>  			goto out_ok;
>  	}
>  
> -out_err:
>  	fi = NULL;
> +out_err:
>  	if (fibh->sbh != fibh->ebh)
>  		brelse(fibh->ebh);
>  	brelse(fibh->sbh);
> @@ -257,6 +271,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
>  	struct inode *inode = NULL;
>  	struct fileIdentDesc cfi;
>  	struct udf_fileident_bh fibh;
> +	struct fileIdentDesc *fi;
>  
>  	if (dentry->d_name.len > UDF_NAME_LEN - 2)
>  		return ERR_PTR(-ENAMETOOLONG);
> @@ -276,7 +291,11 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
>  	} else
>  #endif /* UDF_RECOVERY */
>  
> -	if (udf_find_entry(dir, &dentry->d_name, &fibh, &cfi)) {
> +	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
> +	if (IS_ERR(fi))
> +		return ERR_CAST(fi);
> +
> +	if (fi) {
>  		struct kernel_lb_addr loc;
>  
>  		if (fibh.sbh != fibh.ebh)
> @@ -774,8 +793,11 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
>  
>  	retval = -ENOENT;
>  	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
> -	if (!fi)
> +	if (IS_ERR_OR_NULL(fi)) {
> +		if (fi)
> +			retval = PTR_ERR(fi);
>  		goto out;
> +	}
>  
>  	retval = -EIO;
>  	tloc = lelb_to_cpu(cfi.icb.extLocation);
> @@ -817,8 +839,12 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)
>  
>  	retval = -ENOENT;
>  	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
> -	if (!fi)
> +
> +	if (IS_ERR_OR_NULL(fi)) {
> +		if (fi)
> +			retval = PTR_ERR(fi);
>  		goto out;
> +	}
>  
>  	retval = -EIO;
>  	tloc = lelb_to_cpu(cfi.icb.extLocation);
> @@ -1047,24 +1073,30 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	struct udf_inode_info *old_iinfo = UDF_I(old_inode);
>  
>  	ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi);
> -	if (ofi) {
> -		if (ofibh.sbh != ofibh.ebh)
> -			brelse(ofibh.ebh);
> -		brelse(ofibh.sbh);
> +	if (IS_ERR(ofi)) {
> +		retval = PTR_ERR(ofi);
> +		goto end_rename;
>  	}
> +
> +	if (ofibh.sbh != ofibh.ebh)
> +		brelse(ofibh.ebh);
> +
> +	brelse(ofibh.sbh);
>  	tloc = lelb_to_cpu(ocfi.icb.extLocation);
>  	if (!ofi || udf_get_lb_pblock(old_dir->i_sb, &tloc, 0)
>  	    != old_inode->i_ino)
>  		goto end_rename;
>  
>  	nfi = udf_find_entry(new_dir, &new_dentry->d_name, &nfibh, &ncfi);
> -	if (nfi) {
> -		if (!new_inode) {
> -			if (nfibh.sbh != nfibh.ebh)
> -				brelse(nfibh.ebh);
> -			brelse(nfibh.sbh);
> -			nfi = NULL;
> -		}
> +	if (IS_ERR(nfi)) {
> +		retval = PTR_ERR(nfi);
> +		goto end_rename;
> +	}
> +	if (nfi && !new_inode) {
> +		if (nfibh.sbh != nfibh.ebh)
> +			brelse(nfibh.ebh);
> +		brelse(nfibh.sbh);
> +		nfi = NULL;
>  	}
>  	if (S_ISDIR(old_inode->i_mode)) {
>  		int offset = udf_ext0_offset(old_inode);
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[-- Attachment #2: 0001-udf-Return-error-from-udf_find_entry.patch --]
[-- Type: text/x-patch, Size: 6229 bytes --]

>From d0cf0af6334553955107a9136f9da85c2e5dfd73 Mon Sep 17 00:00:00 2001
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 8 Apr 2015 21:23:58 +0200
Subject: [PATCH] udf: Return error from udf_find_entry()

Return appropriate error from udf_find_entry() instead of just NULL.
That way we can distinguish the fact that some error happened when
looking up filename (and return error to userspace) from the fact that
we just didn't find the filename. Also update callers of
udf_find_entry() accordingly.

[JK: Improved udf_find_entry() documentation]

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/namei.c | 91 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 71 insertions(+), 20 deletions(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 5e833bd1c4e9..5767c36d5652 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -138,6 +138,25 @@ int udf_write_fi(struct inode *inode, struct fileIdentDesc *cfi,
 	return 0;
 }
 
+/**
+ * udf_find_entry - find entry in given directory.
+ *
+ * @dir:	directory inode to search in
+ * @child:	qstr of the name
+ * @fibh:	buffer head / inode with file identifier descriptor we found
+ * @cfi:	found file identifier descriptor with given name
+ *
+ * This function searches in the directory @dir for a file name @child. When
+ * found, @fibh points to the buffer head(s) (bh is NULL for in ICB
+ * directories) containing the file identifier descriptor (FID). In that case
+ * the function returns pointer to the FID in the buffer or inode - but note
+ * that FID may be split among two buffers (blocks) so accessing it via that
+ * pointer isn't easily possible. This pointer can be used only as an iterator
+ * for other directory manipulation functions. For inspection of the FID @cfi
+ * can be used - the found FID is copied there.
+ *
+ * Returns pointer to FID, NULL when nothing found, or error code.
+ */
 static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 					    const struct qstr *child,
 					    struct udf_fileident_bh *fibh,
@@ -167,8 +186,11 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 	fibh->soffset = fibh->eoffset = f_pos & (sb->s_blocksize - 1);
 	if (dinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
 		if (inode_bmap(dir, f_pos >> sb->s_blocksize_bits, &epos,
-		    &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30))
+		    &eloc, &elen, &offset) != (EXT_RECORDED_ALLOCATED >> 30)) {
+			fi = ERR_PTR(-EIO);
 			goto out_err;
+		}
+
 		block = udf_get_lb_pblock(sb, &eloc, offset);
 		if ((++offset << sb->s_blocksize_bits) < elen) {
 			if (dinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT)
@@ -179,19 +201,25 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 			offset = 0;
 
 		fibh->sbh = fibh->ebh = udf_tread(sb, block);
-		if (!fibh->sbh)
+		if (!fibh->sbh) {
+			fi = ERR_PTR(-EIO);
 			goto out_err;
+		}
 	}
 
 	fname = kmalloc(UDF_NAME_LEN, GFP_NOFS);
-	if (!fname)
+	if (!fname) {
+		fi = ERR_PTR(-ENOMEM);
 		goto out_err;
+	}
 
 	while (f_pos < size) {
 		fi = udf_fileident_read(dir, &f_pos, fibh, cfi, &epos, &eloc,
 					&elen, &offset);
-		if (!fi)
+		if (!fi) {
+			fi = ERR_PTR(-EIO);
 			goto out_err;
+		}
 
 		liu = le16_to_cpu(cfi->lengthOfImpUse);
 		lfi = cfi->lengthFileIdent;
@@ -234,12 +262,17 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 			continue;
 
 		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
-		if (flen > 0 && udf_match(flen, fname, child->len, child->name))
+		if (flen < 0) {
+			fi = ERR_PTR(flen);
+			goto out_err;
+		}
+
+		if (udf_match(flen, fname, child->len, child->name))
 			goto out_ok;
 	}
 
-out_err:
 	fi = NULL;
+out_err:
 	if (fibh->sbh != fibh->ebh)
 		brelse(fibh->ebh);
 	brelse(fibh->sbh);
@@ -256,6 +289,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
 	struct inode *inode = NULL;
 	struct fileIdentDesc cfi;
 	struct udf_fileident_bh fibh;
+	struct fileIdentDesc *fi;
 
 	if (dentry->d_name.len > UDF_NAME_LEN - 2)
 		return ERR_PTR(-ENAMETOOLONG);
@@ -275,7 +309,11 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
 	} else
 #endif /* UDF_RECOVERY */
 
-	if (udf_find_entry(dir, &dentry->d_name, &fibh, &cfi)) {
+	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
+	if (IS_ERR(fi))
+		return ERR_CAST(fi);
+
+	if (fi) {
 		struct kernel_lb_addr loc;
 
 		if (fibh.sbh != fibh.ebh)
@@ -774,8 +812,11 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
 
 	retval = -ENOENT;
 	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
-	if (!fi)
+	if (IS_ERR_OR_NULL(fi)) {
+		if (fi)
+			retval = PTR_ERR(fi);
 		goto out;
+	}
 
 	retval = -EIO;
 	tloc = lelb_to_cpu(cfi.icb.extLocation);
@@ -817,8 +858,12 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)
 
 	retval = -ENOENT;
 	fi = udf_find_entry(dir, &dentry->d_name, &fibh, &cfi);
-	if (!fi)
+
+	if (IS_ERR_OR_NULL(fi)) {
+		if (fi)
+			retval = PTR_ERR(fi);
 		goto out;
+	}
 
 	retval = -EIO;
 	tloc = lelb_to_cpu(cfi.icb.extLocation);
@@ -1049,24 +1094,30 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct udf_inode_info *old_iinfo = UDF_I(old_inode);
 
 	ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi);
-	if (ofi) {
-		if (ofibh.sbh != ofibh.ebh)
-			brelse(ofibh.ebh);
-		brelse(ofibh.sbh);
+	if (IS_ERR(ofi)) {
+		retval = PTR_ERR(ofi);
+		goto end_rename;
 	}
+
+	if (ofibh.sbh != ofibh.ebh)
+		brelse(ofibh.ebh);
+
+	brelse(ofibh.sbh);
 	tloc = lelb_to_cpu(ocfi.icb.extLocation);
 	if (!ofi || udf_get_lb_pblock(old_dir->i_sb, &tloc, 0)
 	    != old_inode->i_ino)
 		goto end_rename;
 
 	nfi = udf_find_entry(new_dir, &new_dentry->d_name, &nfibh, &ncfi);
-	if (nfi) {
-		if (!new_inode) {
-			if (nfibh.sbh != nfibh.ebh)
-				brelse(nfibh.ebh);
-			brelse(nfibh.sbh);
-			nfi = NULL;
-		}
+	if (IS_ERR(nfi)) {
+		retval = PTR_ERR(nfi);
+		goto end_rename;
+	}
+	if (nfi && !new_inode) {
+		if (nfibh.sbh != nfibh.ebh)
+			brelse(nfibh.ebh);
+		brelse(nfibh.sbh);
+		nfi = NULL;
 	}
 	if (S_ISDIR(old_inode->i_mode)) {
 		int offset = udf_ext0_offset(old_inode);
-- 
2.1.4

Re: [PATCH V2 4/9 linux-next] udf: improve error management in udf_CS0toUTF8()

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 3737 bytes --]

On Thu 09-04-15 10:36:07, Jan Kara wrote:
> On Wed 08-04-15 21:23:54, Fabian Frederick wrote:
> > udf_CS0toUTF8() now returns -EINVAL on error.
> > udf_load_pvoldesc() and udf_get_filename() do the same.
> > 
> > Suggested-by: Jan Kara <jack@suse.cz>
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
>   After some thought it's probably better API that udf_CS0toUTF8() simply
> returns 0 when 0-lenght string is provided as an input (there's nothing
> invalid in that from the point of view of that conversion functions). We
> cannot actually ever call udf_CS0toUTF8() with such string currently but I
> think it's better for future. So I've changed this and merged the patch.
  Just for reference, attached is the resulting patch.

								Honza

> > ---
> >  fs/udf/super.c   | 23 ++++++++++++++---------
> >  fs/udf/unicode.c |  9 +++++----
> >  2 files changed, 19 insertions(+), 13 deletions(-)
> > 
> > diff --git a/fs/udf/super.c b/fs/udf/super.c
> > index 6299f34..c6a8f5f 100644
> > --- a/fs/udf/super.c
> > +++ b/fs/udf/super.c
> > @@ -927,17 +927,22 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
> >  #endif
> >  	}
> >  
> > -	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32))
> > -		if (udf_CS0toUTF8(outstr, instr)) {
> > -			strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> > -				outstr->u_len > 31 ? 31 : outstr->u_len);
> > -			udf_debug("volIdent[] = '%s'\n",
> > -				  UDF_SB(sb)->s_volume_ident);
> > -		}
> > +	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32)) {
> > +		ret = udf_CS0toUTF8(outstr, instr);
> > +		if (ret < 0)
> > +			goto out_bh;
> > +
> > +		strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
> > +			outstr->u_len > 31 ? 31 : outstr->u_len);
> > +		udf_debug("volIdent[] = '%s'\n", UDF_SB(sb)->s_volume_ident);
> > +	}
> >  
> >  	if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
> > -		if (udf_CS0toUTF8(outstr, instr))
> > -			udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
> > +		ret = udf_CS0toUTF8(outstr, instr);
> > +		if (ret < 0)
> > +			goto out_bh;
> > +
> > +		udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
> >  
> >  	ret = 0;
> >  out_bh:
> > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> > index 41c3bef..9008a36 100644
> > --- a/fs/udf/unicode.c
> > +++ b/fs/udf/unicode.c
> > @@ -89,7 +89,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
> >   * 				both of type "struct ustr *"
> >   *
> >   * POST-CONDITIONS
> > - *	<return>		Zero on success.
> > + *	<return>		>= 0 on success.
> >   *
> >   * HISTORY
> >   *	November 12, 1997 - Andrew E. Mileski
> > @@ -104,7 +104,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
> >  	ocu_len = ocu_i->u_len;
> >  	if (ocu_len == 0) {
> >  		memset(utf_o, 0, sizeof(struct ustr));
> > -		return 0;
> > +		return -EINVAL;
> >  	}
> >  
> >  	cmp_id = ocu_i->u_cmpID;
> > @@ -112,7 +112,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
> >  		memset(utf_o, 0, sizeof(struct ustr));
> >  		pr_err("unknown compression code (%d) stri=%s\n",
> >  		       cmp_id, ocu_i->u_name);
> > -		return 0;
> > +		return -EINVAL;
> >  	}
> >  
> >  	ocu = ocu_i->u_name;
> > @@ -350,7 +350,8 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> >  
> >  	udf_build_ustr_exact(unifilename, sname, slen);
> >  	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
> > -		if (!udf_CS0toUTF8(filename, unifilename)) {
> > +		ret = udf_CS0toUTF8(filename, unifilename);
> > +		if (ret < 0) {
> >  			udf_debug("Failed in udf_get_filename: sname = %s\n",
> >  				  sname);
> >  			goto out2;
> > -- 
> > 1.9.1
> > 
> -- 
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[-- Attachment #2: 0004-udf-improve-error-management-in-udf_CS0toUTF8.patch --]
[-- Type: text/x-patch, Size: 2783 bytes --]

>From 2fd89b7d8600488b00eb2d826750f3800fd5fd9b Mon Sep 17 00:00:00 2001
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 8 Apr 2015 21:23:54 +0200
Subject: [PATCH 4/8] udf: improve error management in udf_CS0toUTF8()

udf_CS0toUTF8() now returns -EINVAL on error.
udf_load_pvoldesc() and udf_get_filename() do the same.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c   | 23 ++++++++++++++---------
 fs/udf/unicode.c |  7 ++++---
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 6299f341967b..c6a8f5f79443 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -927,17 +927,22 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
 #endif
 	}
 
-	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32))
-		if (udf_CS0toUTF8(outstr, instr)) {
-			strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
-				outstr->u_len > 31 ? 31 : outstr->u_len);
-			udf_debug("volIdent[] = '%s'\n",
-				  UDF_SB(sb)->s_volume_ident);
-		}
+	if (!udf_build_ustr(instr, pvoldesc->volIdent, 32)) {
+		ret = udf_CS0toUTF8(outstr, instr);
+		if (ret < 0)
+			goto out_bh;
+
+		strncpy(UDF_SB(sb)->s_volume_ident, outstr->u_name,
+			outstr->u_len > 31 ? 31 : outstr->u_len);
+		udf_debug("volIdent[] = '%s'\n", UDF_SB(sb)->s_volume_ident);
+	}
 
 	if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
-		if (udf_CS0toUTF8(outstr, instr))
-			udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
+		ret = udf_CS0toUTF8(outstr, instr);
+		if (ret < 0)
+			goto out_bh;
+
+		udf_debug("volSetIdent[] = '%s'\n", outstr->u_name);
 
 	ret = 0;
 out_bh:
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 41c3bef1d226..35cc9477b066 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -89,7 +89,7 @@ static void udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)
  * 				both of type "struct ustr *"
  *
  * POST-CONDITIONS
- *	<return>		Zero on success.
+ *	<return>		>= 0 on success.
  *
  * HISTORY
  *	November 12, 1997 - Andrew E. Mileski
@@ -112,7 +112,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
 		memset(utf_o, 0, sizeof(struct ustr));
 		pr_err("unknown compression code (%d) stri=%s\n",
 		       cmp_id, ocu_i->u_name);
-		return 0;
+		return -EINVAL;
 	}
 
 	ocu = ocu_i->u_name;
@@ -350,7 +350,8 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 
 	udf_build_ustr_exact(unifilename, sname, slen);
 	if (UDF_QUERY_FLAG(sb, UDF_FLAG_UTF8)) {
-		if (!udf_CS0toUTF8(filename, unifilename)) {
+		ret = udf_CS0toUTF8(filename, unifilename);
+		if (ret < 0) {
 			udf_debug("Failed in udf_get_filename: sname = %s\n",
 				  sname);
 			goto out2;
-- 
2.1.4

Re: [PATCH V2 5/9 linux-next] udf: improve error management in udf_CS0toNLS()

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]

On Thu 09-04-15 10:37:42, Jan Kara wrote:
> On Wed 08-04-15 21:23:55, Fabian Frederick wrote:
> > Only callsite udf_get_filename() now returns
> > error code as well.
>   Did same modification as in the previous patch. Otherwise OK, so I've
> merged the patch.
  The resulting patch is attached for reference.

								Honza

> > Suggested-by: Jan Kara <jack@suse.cz>
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > ---
> >  fs/udf/unicode.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> > index 9008a36..488a838 100644
> > --- a/fs/udf/unicode.c
> > +++ b/fs/udf/unicode.c
> > @@ -257,7 +257,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
> >  	ocu_len = ocu_i->u_len;
> >  	if (ocu_len == 0) {
> >  		memset(utf_o, 0, sizeof(struct ustr));
> > -		return 0;
> > +		return -EINVAL;
> >  	}
> >  
> >  	cmp_id = ocu_i->u_cmpID;
> > @@ -265,7 +265,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
> >  		memset(utf_o, 0, sizeof(struct ustr));
> >  		pr_err("unknown compression code (%d) stri=%s\n",
> >  		       cmp_id, ocu_i->u_name);
> > -		return 0;
> > +		return -EINVAL;
> >  	}
> >  
> >  	ocu = ocu_i->u_name;
> > @@ -357,8 +357,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> >  			goto out2;
> >  		}
> >  	} else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
> > -		if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> > -				  unifilename)) {
> > +		ret = udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
> > +				   unifilename);
> > +		if (ret < 0) {
> >  			udf_debug("Failed in udf_get_filename: sname = %s\n",
> >  				  sname);
> >  			goto out2;
> > -- 
> > 1.9.1
> > 
> -- 
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[-- Attachment #2: 0005-udf-improve-error-management-in-udf_CS0toNLS.patch --]
[-- Type: text/x-patch, Size: 1342 bytes --]

>From 2cd813297b6cb07d189057402f70100fa15a826b Mon Sep 17 00:00:00 2001
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 8 Apr 2015 21:23:55 +0200
Subject: [PATCH 5/8] udf: improve error management in udf_CS0toNLS()

Only callsite udf_get_filename() now returns error code as well.

Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/unicode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 35cc9477b066..658aa14cc474 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -265,7 +265,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
 		memset(utf_o, 0, sizeof(struct ustr));
 		pr_err("unknown compression code (%d) stri=%s\n",
 		       cmp_id, ocu_i->u_name);
-		return 0;
+		return -EINVAL;
 	}
 
 	ocu = ocu_i->u_name;
@@ -357,8 +357,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 			goto out2;
 		}
 	} else if (UDF_QUERY_FLAG(sb, UDF_FLAG_NLS_MAP)) {
-		if (!udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
-				  unifilename)) {
+		ret = udf_CS0toNLS(UDF_SB(sb)->s_nls_map, filename,
+				   unifilename);
+		if (ret < 0) {
 			udf_debug("Failed in udf_get_filename: sname = %s\n",
 				  sname);
 			goto out2;
-- 
2.1.4

Re: [PATCH V2 7/9 linux-next] udf: return error when newIndex is 0 in udf_translate_to_linux()

[Jan Kara]

[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]

On Thu 09-04-15 10:51:05, Jan Kara wrote:
> On Wed 08-04-15 21:23:57, Fabian Frederick wrote:
> > udf_get_filename() and its callsites considered 0 as an error
> > without propagating an error value.
> > 
> > udf_translate_to_linux() now returns -EINVAL when newIndex is 0.
> > other functions are updated accordingly.
> > 
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
>   I've updated modified this patch and the changelog to do the check for
> zero length filename in udf_get_filename() and not in
> udf_translate_to_linux(). Since the second is just a general conversion
> function and zero length string as a result is fine with that. But zero
> length file name isn't correct and that's why udf_get_filename() should
> generate the error. I've also updated the changelog accordingly.
  Attached is the resulting patch for reference.

								Honza

> >  fs/udf/dir.c     | 2 +-
> >  fs/udf/namei.c   | 2 +-
> >  fs/udf/unicode.c | 4 ++--
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> > index fcf227e..541d9c6 100644
> > --- a/fs/udf/dir.c
> > +++ b/fs/udf/dir.c
> > @@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
> >  		}
> >  
> >  		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> > -		if (flen <= 0)
> > +		if (flen < 0)
> >  			continue;
> >  
> >  		tloc = lelb_to_cpu(cfi.icb.extLocation);
> > diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> > index 59b340c..dd648b7 100644
> > --- a/fs/udf/namei.c
> > +++ b/fs/udf/namei.c
> > @@ -234,7 +234,7 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
> >  			continue;
> >  
> >  		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> > -		if ((flen > 0) && udf_match(flen, fname, child->len,
> > +		if ((flen >= 0) && udf_match(flen, fname, child->len,
> >  					    child->name))
> >  			goto out_ok;
> >  	}
> > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> > index f37123b..3b1efbb 100644
> > --- a/fs/udf/unicode.c
> > +++ b/fs/udf/unicode.c
> > @@ -333,7 +333,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> >  		     uint8_t *dname, int dlen)
> >  {
> >  	struct ustr *filename, *unifilename;
> > -	int ret = 0;
> > +	int ret;
> >  
> >  	if (!slen)
> >  		return -EIO;
> > @@ -492,5 +492,5 @@ static int udf_translate_to_linux(uint8_t *newName, int newLen,
> >  		}
> >  	}
> >  
> > -	return newIndex;
> > +	return newIndex ? : -EINVAL;
> >  }
> > -- 
> > 1.9.1
> > 
> -- 
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[-- Attachment #2: 0007-udf-Make-udf_get_filename-return-error-instead-of-0-.patch --]
[-- Type: text/x-patch, Size: 1777 bytes --]

>From 837a2883619b33784250db651a18d2a80c822f6d Mon Sep 17 00:00:00 2001
From: Fabian Frederick <fabf@skynet.be>
Date: Wed, 8 Apr 2015 21:23:57 +0200
Subject: [PATCH 7/8] udf: Make udf_get_filename() return error instead of 0
 length file name

Zero length file name isn't really valid. So check the length of the
final file name generated by udf_translate_to_linux() and return -EINVAL
instead of zero length file name. Update caller of udf_get_filename() to
not check for 0 return value.

Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/dir.c     | 2 +-
 fs/udf/unicode.c | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index fcf227eb2c51..541d9c65014d 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -168,7 +168,7 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
 		}
 
 		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
-		if (flen <= 0)
+		if (flen < 0)
 			continue;
 
 		tloc = lelb_to_cpu(cfi.icb.extLocation);
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index 97b23b0f9713..ab478e62baae 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -333,7 +333,7 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 		     uint8_t *dname, int dlen)
 {
 	struct ustr *filename, *unifilename;
-	int ret = 0;
+	int ret;
 
 	if (!slen)
 		return -EIO;
@@ -370,6 +370,9 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
 	ret = udf_translate_to_linux(dname, dlen,
 				     filename->u_name, filename->u_len,
 				     unifilename->u_name, unifilename->u_len);
+	/* Zero length filename isn't valid... */
+	if (ret == 0)
+		ret = -EINVAL;
 out2:
 	kfree(unifilename);
 out1:
-- 
2.1.4