[PATCH 1/5 linux-next] udf: remove unused variable in udf_table_free_blocks()

[PATCH 1/5 linux-next] udf: remove unused variable in udf_table_free_blocks()

[Fabian Frederick]

Fix set but not used warning.

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

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 1ba2baa..02948f0 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -358,7 +358,6 @@ static void udf_table_free_blocks(struct super_block *sb,
 	struct kernel_lb_addr eloc;
 	struct extent_position oepos, epos;
 	int8_t etype;
-	int i;
 	struct udf_inode_info *iinfo;
 
 	mutex_lock(&sbi->s_alloc_mutex);
@@ -425,7 +424,6 @@ static void udf_table_free_blocks(struct super_block *sb,
 		}
 
 		if (epos.bh != oepos.bh) {
-			i = -1;
 			oepos.block = epos.block;
 			brelse(oepos.bh);
 			get_bh(epos.bh);
-- 
1.9.1

[PATCH 2/5 linux-next] udf: use sector_t for udf_bitmap_prealloc_blocks() return value

[Fabian Frederick]

udf_bitmap_prealloc_blocks() can only return positive value and is only
used to assign sector_t allocated.

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/balloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 02948f0..30f4641 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -170,7 +170,7 @@ error_return:
 	mutex_unlock(&sbi->s_alloc_mutex);
 }
 
-static int udf_bitmap_prealloc_blocks(struct super_block *sb,
+static sector_t udf_bitmap_prealloc_blocks(struct super_block *sb,
 				      struct udf_bitmap *bitmap,
 				      uint16_t partition, uint32_t first_block,
 				      uint32_t block_count)
-- 
1.9.1

[PATCH 3/5 linux-next] udf: remove else after return in __load_block_bitmap()

[Fabian Frederick]

else after return is not needed.

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

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 30f4641..1cd8916 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -63,15 +63,14 @@ static int __load_block_bitmap(struct super_block *sb,
 			  block_group, nr_groups);
 	}
 
-	if (bitmap->s_block_bitmap[block_group]) {
+	if (bitmap->s_block_bitmap[block_group])
 		return block_group;
-	} else {
-		retval = read_block_bitmap(sb, bitmap, block_group,
-					   block_group);
-		if (retval < 0)
-			return retval;
-		return block_group;
-	}
+
+	retval = read_block_bitmap(sb, bitmap, block_group, block_group);
+	if (retval < 0)
+		return retval;
+
+	return block_group;
 }
 
 static inline int load_block_bitmap(struct super_block *sb,
-- 
1.9.1

[PATCH 4/5 linux-next] udf: rename udf_get_filename()

[Fabian Frederick]

udf_readdir(), udf_find_entry() and udf_pc_to_char() use
udf_get_filename to obtain name length. Give that function
an appropriate name.

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

diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index 05e90ed..edf4232 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -168,7 +168,8 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
 			continue;
 		}
 
-		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
+		flen = udf_get_filename_length(sb, nameptr, lfi, fname,
+					       UDF_NAME_LEN);
 		if (!flen)
 			continue;
 
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 33b246b..189b98b 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
 		if (!lfi)
 			continue;
 
-		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
+		flen = udf_get_filename_length(sb, nameptr, lfi, fname,
+					       UDF_NAME_LEN);
 		if (flen && udf_match(flen, fname, child->len, child->name))
 			goto out_ok;
 	}
diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
index ac10ca9..d986916a 100644
--- a/fs/udf/symlink.c
+++ b/fs/udf/symlink.c
@@ -80,9 +80,10 @@ static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
 			elen += pc->lengthComponentIdent;
 			if (elen > fromlen)
 				return -EIO;
-			comp_len = udf_get_filename(sb, pc->componentIdent,
-						    pc->lengthComponentIdent,
-						    p, tolen);
+			comp_len = udf_get_filename_length(sb,
+						pc->componentIdent,
+						pc->lengthComponentIdent,
+						p, tolen);
 			p += comp_len;
 			tolen -= comp_len;
 			if (tolen == 0)
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 47bb3f5..70dc260 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -211,8 +211,8 @@ udf_get_lb_pblock(struct super_block *sb, struct kernel_lb_addr *loc,
 }
 
 /* unicode.c */
-extern int udf_get_filename(struct super_block *, uint8_t *, int, uint8_t *,
-			    int);
+extern int udf_get_filename_length(struct super_block *, uint8_t *, int,
+				   uint8_t *, int);
 extern int udf_put_filename(struct super_block *, const uint8_t *, uint8_t *,
 			    int);
 extern int udf_build_ustr(struct ustr *, dstring *, int);
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index b84fee3..209c0c7 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -334,8 +334,8 @@ try_again:
 	return u_len + 1;
 }
 
-int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
-		     uint8_t *dname, int dlen)
+int udf_get_filename_length(struct super_block *sb, uint8_t *sname, int slen,
+			    uint8_t *dname, int dlen)
 {
 	struct ustr *filename, *unifilename;
 	int len = 0;
-- 
1.9.1

[PATCH 5/5 linux-next] udf: remove redundant buffer_head.h includes

[Fabian Frederick]

buffer_head.h was already included in udfdecl.h

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 fs/udf/balloc.c    | 1 -
 fs/udf/dir.c       | 1 -
 fs/udf/directory.c | 1 -
 fs/udf/file.c      | 1 -
 fs/udf/inode.c     | 1 -
 fs/udf/misc.c      | 1 -
 fs/udf/namei.c     | 1 -
 fs/udf/partition.c | 1 -
 fs/udf/super.c     | 1 -
 fs/udf/symlink.c   | 1 -
 fs/udf/truncate.c  | 1 -
 11 files changed, 11 deletions(-)

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 1cd8916..59ac281 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -21,7 +21,6 @@
 
 #include "udfdecl.h"
 
-#include <linux/buffer_head.h>
 #include <linux/bitops.h>
 
 #include "udf_i.h"
diff --git a/fs/udf/dir.c b/fs/udf/dir.c
index edf4232..8af629d 100644
--- a/fs/udf/dir.c
+++ b/fs/udf/dir.c
@@ -30,7 +30,6 @@
 #include <linux/errno.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
-#include <linux/buffer_head.h>
 
 #include "udf_i.h"
 #include "udf_sb.h"
diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index 3e44f57..c763fda 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -16,7 +16,6 @@
 
 #include <linux/fs.h>
 #include <linux/string.h>
-#include <linux/buffer_head.h>
 
 struct fileIdentDesc *udf_fileident_read(struct inode *dir, loff_t *nf_pos,
 					 struct udf_fileident_bh *fibh,
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 08f3555..dda8ea7 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -33,7 +33,6 @@
 #include <linux/capability.h>
 #include <linux/errno.h>
 #include <linux/pagemap.h>
-#include <linux/buffer_head.h>
 #include <linux/aio.h>
 
 #include "udf_i.h"
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index a445d59..0001ece 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -33,7 +33,6 @@
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/pagemap.h>
-#include <linux/buffer_head.h>
 #include <linux/writeback.h>
 #include <linux/slab.h>
 #include <linux/crc-itu-t.h>
diff --git a/fs/udf/misc.c b/fs/udf/misc.c
index c175b4d..71d1c25 100644
--- a/fs/udf/misc.c
+++ b/fs/udf/misc.c
@@ -23,7 +23,6 @@
 
 #include <linux/fs.h>
 #include <linux/string.h>
-#include <linux/buffer_head.h>
 #include <linux/crc-itu-t.h>
 
 #include "udf_i.h"
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 189b98b..3bb7c02 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -27,7 +27,6 @@
 #include <linux/errno.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
-#include <linux/buffer_head.h>
 #include <linux/sched.h>
 #include <linux/crc-itu-t.h>
 #include <linux/exportfs.h>
diff --git a/fs/udf/partition.c b/fs/udf/partition.c
index d6caf01..5f861ed2 100644
--- a/fs/udf/partition.c
+++ b/fs/udf/partition.c
@@ -24,7 +24,6 @@
 
 #include <linux/fs.h>
 #include <linux/string.h>
-#include <linux/buffer_head.h>
 #include <linux/mutex.h>
 
 uint32_t udf_get_pblock(struct super_block *sb, uint32_t block,
diff --git a/fs/udf/super.c b/fs/udf/super.c
index f169411..6299f34 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -48,7 +48,6 @@
 #include <linux/stat.h>
 #include <linux/cdrom.h>
 #include <linux/nls.h>
-#include <linux/buffer_head.h>
 #include <linux/vfs.h>
 #include <linux/vmalloc.h>
 #include <linux/errno.h>
diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
index d986916a..cef3c69 100644
--- a/fs/udf/symlink.c
+++ b/fs/udf/symlink.c
@@ -27,7 +27,6 @@
 #include <linux/mm.h>
 #include <linux/stat.h>
 #include <linux/pagemap.h>
-#include <linux/buffer_head.h>
 #include "udf_i.h"
 
 static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
index 8a9657d..42b8c57 100644
--- a/fs/udf/truncate.c
+++ b/fs/udf/truncate.c
@@ -22,7 +22,6 @@
 #include "udfdecl.h"
 #include <linux/fs.h>
 #include <linux/mm.h>
-#include <linux/buffer_head.h>
 
 #include "udf_i.h"
 #include "udf_sb.h"
-- 
1.9.1

Re: [PATCH 1/5 linux-next] udf: remove unused variable in udf_table_free_blocks()

[Jan Kara]

On Tue 10-03-15 21:44:31, Fabian Frederick wrote:
> Fix set but not used warning.
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  Thanks. I've added the patch to my tree.

								Honza

> ---
>  fs/udf/balloc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 1ba2baa..02948f0 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -358,7 +358,6 @@ static void udf_table_free_blocks(struct super_block *sb,
>  	struct kernel_lb_addr eloc;
>  	struct extent_position oepos, epos;
>  	int8_t etype;
> -	int i;
>  	struct udf_inode_info *iinfo;
>  
>  	mutex_lock(&sbi->s_alloc_mutex);
> @@ -425,7 +424,6 @@ static void udf_table_free_blocks(struct super_block *sb,
>  		}
>  
>  		if (epos.bh != oepos.bh) {
> -			i = -1;
>  			oepos.block = epos.block;
>  			brelse(oepos.bh);
>  			get_bh(epos.bh);
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 2/5 linux-next] udf: use sector_t for udf_bitmap_prealloc_blocks() return value

[Jan Kara]

On Tue 10-03-15 21:44:32, Fabian Frederick wrote:
> udf_bitmap_prealloc_blocks() can only return positive value and is only
> used to assign sector_t allocated.
  The return value is number of allocated blocks. There's no good reason to
change that to sector_t... The variable 'allocated' in
udf_prealloc_blocks() could be int if anything.

								Honza
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  fs/udf/balloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 02948f0..30f4641 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -170,7 +170,7 @@ error_return:
>  	mutex_unlock(&sbi->s_alloc_mutex);
>  }
>  
> -static int udf_bitmap_prealloc_blocks(struct super_block *sb,
> +static sector_t udf_bitmap_prealloc_blocks(struct super_block *sb,
>  				      struct udf_bitmap *bitmap,
>  				      uint16_t partition, uint32_t first_block,
>  				      uint32_t block_count)
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 3/5 linux-next] udf: remove else after return in __load_block_bitmap()

[Jan Kara]

On Tue 10-03-15 21:44:33, Fabian Frederick wrote:
> else after return is not needed.
  Thanks. Applied to my tree.

								Honza

> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  fs/udf/balloc.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 30f4641..1cd8916 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -63,15 +63,14 @@ static int __load_block_bitmap(struct super_block *sb,
>  			  block_group, nr_groups);
>  	}
>  
> -	if (bitmap->s_block_bitmap[block_group]) {
> +	if (bitmap->s_block_bitmap[block_group])
>  		return block_group;
> -	} else {
> -		retval = read_block_bitmap(sb, bitmap, block_group,
> -					   block_group);
> -		if (retval < 0)
> -			return retval;
> -		return block_group;
> -	}
> +
> +	retval = read_block_bitmap(sb, bitmap, block_group, block_group);
> +	if (retval < 0)
> +		return retval;
> +
> +	return block_group;
>  }
>  
>  static inline int load_block_bitmap(struct super_block *sb,
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 4/5 linux-next] udf: rename udf_get_filename()

[Jan Kara]

On Tue 10-03-15 21:44:34, Fabian Frederick wrote:
> udf_readdir(), udf_find_entry() and udf_pc_to_char() use
> udf_get_filename to obtain name length. Give that function
> an appropriate name.
  Hum, have you read what that function does? It actually converts the name
to a different format and returns converted length. So your name is IMHO
more confusing - it's as if sprintf() was called sprintf_length()... Not
applied.

								Honza

> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>  fs/udf/dir.c     | 3 ++-
>  fs/udf/namei.c   | 3 ++-
>  fs/udf/symlink.c | 7 ++++---
>  fs/udf/udfdecl.h | 4 ++--
>  fs/udf/unicode.c | 4 ++--
>  5 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> index 05e90ed..edf4232 100644
> --- a/fs/udf/dir.c
> +++ b/fs/udf/dir.c
> @@ -168,7 +168,8 @@ static int udf_readdir(struct file *file, struct dir_context *ctx)
>  			continue;
>  		}
>  
> -		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> +		flen = udf_get_filename_length(sb, nameptr, lfi, fname,
> +					       UDF_NAME_LEN);
>  		if (!flen)
>  			continue;
>  
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 33b246b..189b98b 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode *dir,
>  		if (!lfi)
>  			continue;
>  
> -		flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> +		flen = udf_get_filename_length(sb, nameptr, lfi, fname,
> +					       UDF_NAME_LEN);
>  		if (flen && udf_match(flen, fname, child->len, child->name))
>  			goto out_ok;
>  	}
> diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
> index ac10ca9..d986916a 100644
> --- a/fs/udf/symlink.c
> +++ b/fs/udf/symlink.c
> @@ -80,9 +80,10 @@ static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
>  			elen += pc->lengthComponentIdent;
>  			if (elen > fromlen)
>  				return -EIO;
> -			comp_len = udf_get_filename(sb, pc->componentIdent,
> -						    pc->lengthComponentIdent,
> -						    p, tolen);
> +			comp_len = udf_get_filename_length(sb,
> +						pc->componentIdent,
> +						pc->lengthComponentIdent,
> +						p, tolen);
>  			p += comp_len;
>  			tolen -= comp_len;
>  			if (tolen == 0)
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> index 47bb3f5..70dc260 100644
> --- a/fs/udf/udfdecl.h
> +++ b/fs/udf/udfdecl.h
> @@ -211,8 +211,8 @@ udf_get_lb_pblock(struct super_block *sb, struct kernel_lb_addr *loc,
>  }
>  
>  /* unicode.c */
> -extern int udf_get_filename(struct super_block *, uint8_t *, int, uint8_t *,
> -			    int);
> +extern int udf_get_filename_length(struct super_block *, uint8_t *, int,
> +				   uint8_t *, int);
>  extern int udf_put_filename(struct super_block *, const uint8_t *, uint8_t *,
>  			    int);
>  extern int udf_build_ustr(struct ustr *, dstring *, int);
> diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> index b84fee3..209c0c7 100644
> --- a/fs/udf/unicode.c
> +++ b/fs/udf/unicode.c
> @@ -334,8 +334,8 @@ try_again:
>  	return u_len + 1;
>  }
>  
> -int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> -		     uint8_t *dname, int dlen)
> +int udf_get_filename_length(struct super_block *sb, uint8_t *sname, int slen,
> +			    uint8_t *dname, int dlen)
>  {
>  	struct ustr *filename, *unifilename;
>  	int len = 0;
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 5/5 linux-next] udf: remove redundant buffer_head.h includes

[Jan Kara]

On Tue 10-03-15 21:44:35, Fabian Frederick wrote:
> buffer_head.h was already included in udfdecl.h
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
  OK, applied to my tree. Thanks.

								Honza

> ---
>  fs/udf/balloc.c    | 1 -
>  fs/udf/dir.c       | 1 -
>  fs/udf/directory.c | 1 -
>  fs/udf/file.c      | 1 -
>  fs/udf/inode.c     | 1 -
>  fs/udf/misc.c      | 1 -
>  fs/udf/namei.c     | 1 -
>  fs/udf/partition.c | 1 -
>  fs/udf/super.c     | 1 -
>  fs/udf/symlink.c   | 1 -
>  fs/udf/truncate.c  | 1 -
>  11 files changed, 11 deletions(-)
> 
> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 1cd8916..59ac281 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -21,7 +21,6 @@
>  
>  #include "udfdecl.h"
>  
> -#include <linux/buffer_head.h>
>  #include <linux/bitops.h>
>  
>  #include "udf_i.h"
> diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> index edf4232..8af629d 100644
> --- a/fs/udf/dir.c
> +++ b/fs/udf/dir.c
> @@ -30,7 +30,6 @@
>  #include <linux/errno.h>
>  #include <linux/mm.h>
>  #include <linux/slab.h>
> -#include <linux/buffer_head.h>
>  
>  #include "udf_i.h"
>  #include "udf_sb.h"
> diff --git a/fs/udf/directory.c b/fs/udf/directory.c
> index 3e44f57..c763fda 100644
> --- a/fs/udf/directory.c
> +++ b/fs/udf/directory.c
> @@ -16,7 +16,6 @@
>  
>  #include <linux/fs.h>
>  #include <linux/string.h>
> -#include <linux/buffer_head.h>
>  
>  struct fileIdentDesc *udf_fileident_read(struct inode *dir, loff_t *nf_pos,
>  					 struct udf_fileident_bh *fibh,
> diff --git a/fs/udf/file.c b/fs/udf/file.c
> index 08f3555..dda8ea7 100644
> --- a/fs/udf/file.c
> +++ b/fs/udf/file.c
> @@ -33,7 +33,6 @@
>  #include <linux/capability.h>
>  #include <linux/errno.h>
>  #include <linux/pagemap.h>
> -#include <linux/buffer_head.h>
>  #include <linux/aio.h>
>  
>  #include "udf_i.h"
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index a445d59..0001ece 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -33,7 +33,6 @@
>  #include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/pagemap.h>
> -#include <linux/buffer_head.h>
>  #include <linux/writeback.h>
>  #include <linux/slab.h>
>  #include <linux/crc-itu-t.h>
> diff --git a/fs/udf/misc.c b/fs/udf/misc.c
> index c175b4d..71d1c25 100644
> --- a/fs/udf/misc.c
> +++ b/fs/udf/misc.c
> @@ -23,7 +23,6 @@
>  
>  #include <linux/fs.h>
>  #include <linux/string.h>
> -#include <linux/buffer_head.h>
>  #include <linux/crc-itu-t.h>
>  
>  #include "udf_i.h"
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 189b98b..3bb7c02 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -27,7 +27,6 @@
>  #include <linux/errno.h>
>  #include <linux/mm.h>
>  #include <linux/slab.h>
> -#include <linux/buffer_head.h>
>  #include <linux/sched.h>
>  #include <linux/crc-itu-t.h>
>  #include <linux/exportfs.h>
> diff --git a/fs/udf/partition.c b/fs/udf/partition.c
> index d6caf01..5f861ed2 100644
> --- a/fs/udf/partition.c
> +++ b/fs/udf/partition.c
> @@ -24,7 +24,6 @@
>  
>  #include <linux/fs.h>
>  #include <linux/string.h>
> -#include <linux/buffer_head.h>
>  #include <linux/mutex.h>
>  
>  uint32_t udf_get_pblock(struct super_block *sb, uint32_t block,
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index f169411..6299f34 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -48,7 +48,6 @@
>  #include <linux/stat.h>
>  #include <linux/cdrom.h>
>  #include <linux/nls.h>
> -#include <linux/buffer_head.h>
>  #include <linux/vfs.h>
>  #include <linux/vmalloc.h>
>  #include <linux/errno.h>
> diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
> index d986916a..cef3c69 100644
> --- a/fs/udf/symlink.c
> +++ b/fs/udf/symlink.c
> @@ -27,7 +27,6 @@
>  #include <linux/mm.h>
>  #include <linux/stat.h>
>  #include <linux/pagemap.h>
> -#include <linux/buffer_head.h>
>  #include "udf_i.h"
>  
>  static int udf_pc_to_char(struct super_block *sb, unsigned char *from,
> diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
> index 8a9657d..42b8c57 100644
> --- a/fs/udf/truncate.c
> +++ b/fs/udf/truncate.c
> @@ -22,7 +22,6 @@
>  #include "udfdecl.h"
>  #include <linux/fs.h>
>  #include <linux/mm.h>
> -#include <linux/buffer_head.h>
>  
>  #include "udf_i.h"
>  #include "udf_sb.h"
> -- 
> 1.9.1
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 4/5 linux-next] udf: rename udf_get_filename()

[Fabian Frederick]

> On 14 March 2015 at 07:52 Jan Kara <jack@suse.cz> wrote:
>
>
> On Tue 10-03-15 21:44:34, Fabian Frederick wrote:
> > udf_readdir(), udf_find_entry() and udf_pc_to_char() use
> > udf_get_filename to obtain name length. Give that function
> > an appropriate name.
>   Hum, have you read what that function does? It actually converts the name
> to a different format and returns converted length. So your name is IMHO
> more confusing - it's as if sprintf() was called sprintf_length()... Not
> applied.
>
>                                                               Honza

Hi Jan,

Ok for the name but AFAICS there's still a problem with error management in
udf_get_filename().
We return 0 when not able to allocate filename and callsites don't seem to
relate the real problem.
Maybe we could either BUG_ON(!filename), BUG_ON(!unifilename) directly in
udf_get_filename() or return -ENOMEM ?
 
udf_readdir() could return -ENOMEM instead of 0 but functions like
udf_find_entry() would need some updates ...

Another solution would be to have length as argument and return error...

Regards,
Fabian

>
> >
> > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > ---
> >  fs/udf/dir.c     | 3 ++-
> >  fs/udf/namei.c   | 3 ++-
> >  fs/udf/symlink.c | 7 ++++---
> >  fs/udf/udfdecl.h | 4 ++--
> >  fs/udf/unicode.c | 4 ++--
> >  5 files changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/udf/dir.c b/fs/udf/dir.c
> > index 05e90ed..edf4232 100644
> > --- a/fs/udf/dir.c
> > +++ b/fs/udf/dir.c
> > @@ -168,7 +168,8 @@ static int udf_readdir(struct file *file, struct
> > dir_context *ctx)
> >                     continue;
> >             }
> > 
> > -           flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> > +           flen = udf_get_filename_length(sb, nameptr, lfi, fname,
> > +                                          UDF_NAME_LEN);
> >             if (!flen)
> >                     continue;
> > 
> > diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> > index 33b246b..189b98b 100644
> > --- a/fs/udf/namei.c
> > +++ b/fs/udf/namei.c
> > @@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode
> > *dir,
> >             if (!lfi)
> >                     continue;
> > 
> > -           flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN);
> > +           flen = udf_get_filename_length(sb, nameptr, lfi, fname,
> > +                                          UDF_NAME_LEN);
> >             if (flen && udf_match(flen, fname, child->len, child->name))
> >                     goto out_ok;
> >     }
> > diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c
> > index ac10ca9..d986916a 100644
> > --- a/fs/udf/symlink.c
> > +++ b/fs/udf/symlink.c
> > @@ -80,9 +80,10 @@ static int udf_pc_to_char(struct super_block *sb,
> > unsigned char *from,
> >                     elen += pc->lengthComponentIdent;
> >                     if (elen > fromlen)
> >                             return -EIO;
> > -                   comp_len = udf_get_filename(sb, pc->componentIdent,
> > -                                               pc->lengthComponentIdent,
> > -                                               p, tolen);
> > +                   comp_len = udf_get_filename_length(sb,
> > +                                           pc->componentIdent,
> > +                                           pc->lengthComponentIdent,
> > +                                           p, tolen);
> >                     p += comp_len;
> >                     tolen -= comp_len;
> >                     if (tolen == 0)
> > diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> > index 47bb3f5..70dc260 100644
> > --- a/fs/udf/udfdecl.h
> > +++ b/fs/udf/udfdecl.h
> > @@ -211,8 +211,8 @@ udf_get_lb_pblock(struct super_block *sb, struct
> > kernel_lb_addr *loc,
> >  }
> > 
> >  /* unicode.c */
> > -extern int udf_get_filename(struct super_block *, uint8_t *, int, uint8_t
> > *,
> > -                       int);
> > +extern int udf_get_filename_length(struct super_block *, uint8_t *, int,
> > +                              uint8_t *, int);
> >  extern int udf_put_filename(struct super_block *, const uint8_t *, uint8_t
> >*,
> >                         int);
> >  extern int udf_build_ustr(struct ustr *, dstring *, int);
> > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
> > index b84fee3..209c0c7 100644
> > --- a/fs/udf/unicode.c
> > +++ b/fs/udf/unicode.c
> > @@ -334,8 +334,8 @@ try_again:
> >     return u_len + 1;
> >  }
> > 
> > -int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen,
> > -                uint8_t *dname, int dlen)
> > +int udf_get_filename_length(struct super_block *sb, uint8_t *sname, int
> > slen,
> > +                       uint8_t *dname, int dlen)
> >  {
> >     struct ustr *filename, *unifilename;
> >     int len = 0;
> > --
> > 1.9.1
> >
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR

Re: [PATCH 4/5 linux-next] udf: rename udf_get_filename()

[Jan Kara]

  Hi Fabian,

On Sun 15-03-15 09:34:35, Fabian Frederick wrote:
> > On 14 March 2015 at 07:52 Jan Kara <jack@suse.cz> wrote:
> >
> >
> > On Tue 10-03-15 21:44:34, Fabian Frederick wrote:
> > > udf_readdir(), udf_find_entry() and udf_pc_to_char() use
> > > udf_get_filename to obtain name length. Give that function
> > > an appropriate name.
> >   Hum, have you read what that function does? It actually converts the name
> > to a different format and returns converted length. So your name is IMHO
> > more confusing - it's as if sprintf() was called sprintf_length()... Not
> > applied.
> 
> Ok for the name but AFAICS there's still a problem with error management in
> udf_get_filename().
> We return 0 when not able to allocate filename and callsites don't seem to
> relate the real problem.
  Umm, we have three callsites:
udf_readdir() - that skips the name if returned length is 0. Arguably we
  should return error to userspace if we didn't emit any name yet. But then
  all other error handling in that function should behave like that.
udf_find_entry() - again we skip name if returned length is 0. Again we
  could do better in error handling but that would require rewriting
  udf_find_entry() to propagate errors up the stack instead of just
  returning NULL.
udf_pc_to_char() - This forgets to check and can return error. I'm happy to
  take a fix for that (or I will write it unless you do).

So returning error value < 0 for error from udf_get_filename() would look
OK to me. But given how udf_readdir() and udf_find_entry() behave it won't
help too much. But it's a step in the right direction. Thanks for spotting
this.

								Honza

-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 4/5 linux-next] udf: rename udf_get_filename()

[Fabian Frederick]

> On 16 March 2015 at 08:46 Jan Kara <jack@suse.cz> wrote:
>
>
>   Hi Fabian,
>
> On Sun 15-03-15 09:34:35, Fabian Frederick wrote:
> > > On 14 March 2015 at 07:52 Jan Kara <jack@suse.cz> wrote:
> > >
> > >
> > > On Tue 10-03-15 21:44:34, Fabian Frederick wrote:
> > > > udf_readdir(), udf_find_entry() and udf_pc_to_char() use
> > > > udf_get_filename to obtain name length. Give that function
> > > > an appropriate name.
> > >   Hum, have you read what that function does? It actually converts the
> > >name
> > > to a different format and returns converted length. So your name is IMHO
> > > more confusing - it's as if sprintf() was called sprintf_length()... Not
> > > applied.
> >
> > Ok for the name but AFAICS there's still a problem with error management in
> > udf_get_filename().
> > We return 0 when not able to allocate filename and callsites don't seem to
> > relate the real problem.
>   Umm, we have three callsites:
> udf_readdir() - that skips the name if returned length is 0. Arguably we
>   should return error to userspace if we didn't emit any name yet. But then
>   all other error handling in that function should behave like that.
> udf_find_entry() - again we skip name if returned length is 0. Again we
>   could do better in error handling but that would require rewriting
>   udf_find_entry() to propagate errors up the stack instead of just
>   returning NULL.
> udf_pc_to_char() - This forgets to check and can return error. I'm happy to
>   take a fix for that (or I will write it unless you do).
>
> So returning error value < 0 for error from udf_get_filename() would look
> OK to me. But given how udf_readdir() and udf_find_entry() behave it won't
> help too much. But it's a step in the right direction. Thanks for spotting
> this.

Thanks a lot Jan. I hope the patch I just sent is ok.

Regards,
Fabian
>
>                                                               Honza
>
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR