The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 1/2 v2] udf : enable error print in udf_read_tagged().
@ 2011-10-03  7:19 Namjae Jeon
  2011-10-03 12:57 ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Namjae Jeon @ 2011-10-03  7:19 UTC (permalink / raw)
  To: jack, joe; +Cc: linux-kernel, Namjae Jeon

While reading metadata, if a problem occurs,
Print out only one of the five case.(It also does not show a checksum properly.)
Because currently it have been disable by undef udf_debug.
If there is a problem with scratched disc or loader, the user needs to know which error happened.
And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.

Signed-off-by: Namjae Jeon <linkinjeon@gmail.com>
---
 fs/udf/directory.c |    4 ++--
 fs/udf/inode.c     |   14 +++++++-------
 fs/udf/misc.c      |   17 ++++++++++-------
 fs/udf/super.c     |    2 +-
 fs/udf/truncate.c  |    5 ++---
 fs/udf/udfdecl.h   |   15 +++++++--------
 fs/udf/udftime.c   |    2 +-
 fs/udf/unicode.c   |    6 +++---
 8 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index 2ffdb67..a4caf7a 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -201,7 +201,7 @@ struct short_ad *udf_get_fileshortad(uint8_t *ptr, int maxoffset, uint32_t *offs
 	struct short_ad *sa;
 
 	if ((!ptr) || (!offset)) {
-		printk(KERN_ERR "udf: udf_get_fileshortad() invalidparms\n");
+		pr_err("udf_get_fileshortad() invalidparms\n");
 		return NULL;
 	}
 
@@ -223,7 +223,7 @@ struct long_ad *udf_get_filelongad(uint8_t *ptr, int maxoffset, uint32_t *offset
 	struct long_ad *la;
 
 	if ((!ptr) || (!offset)) {
-		printk(KERN_ERR "udf: udf_get_filelongad() invalidparms\n");
+		pr_err("udf_get_filelongad() invalidparms\n");
 		return NULL;
 	}
 
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 1d1358e..58665ce 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -83,7 +83,7 @@ void udf_evict_inode(struct inode *inode)
 	end_writeback(inode);
 	if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB &&
 	    inode->i_size != iinfo->i_lenExtents) {
-		printk(KERN_WARNING "UDF-fs (%s): Inode %lu (mode %o) has "
+		pr_warn("(%s): Inode %lu (mode %o) has "
 			"inode size %llu different from extent length %llu. "
 			"Filesystem need not be standards compliant.\n",
 			inode->i_sb->s_id, inode->i_ino, inode->i_mode,
@@ -1169,7 +1169,7 @@ static void __udf_read_inode(struct inode *inode)
 	 */
 	bh = udf_read_ptagged(inode->i_sb, &iinfo->i_location, 0, &ident);
 	if (!bh) {
-		printk(KERN_ERR "udf: udf_read_inode(ino %ld) failed !bh\n",
+		pr_err("udf_read_inode(ino %ld) failed !bh\n",
 		       inode->i_ino);
 		make_bad_inode(inode);
 		return;
@@ -1177,7 +1177,7 @@ static void __udf_read_inode(struct inode *inode)
 
 	if (ident != TAG_IDENT_FE && ident != TAG_IDENT_EFE &&
 	    ident != TAG_IDENT_USE) {
-		printk(KERN_ERR "udf: udf_read_inode(ino %ld) "
+		pr_err("udf_read_inode(ino %ld) "
 				"failed ident=%d\n", inode->i_ino, ident);
 		brelse(bh);
 		make_bad_inode(inode);
@@ -1218,7 +1218,7 @@ static void __udf_read_inode(struct inode *inode)
 		}
 		brelse(ibh);
 	} else if (fe->icbTag.strategyType != cpu_to_le16(4)) {
-		printk(KERN_ERR "udf: unsupported strategy type: %d\n",
+		pr_err("unsupported strategy type: %d\n",
 		       le16_to_cpu(fe->icbTag.strategyType));
 		brelse(bh);
 		make_bad_inode(inode);
@@ -1413,7 +1413,7 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)
 		udf_debug("METADATA BITMAP FILE-----\n");
 		break;
 	default:
-		printk(KERN_ERR "udf: udf_fill_inode(ino %ld) failed unknown "
+		pr_err("udf_fill_inode(ino %ld) failed unknown "
 				"file type=%d\n", inode->i_ino,
 				fe->icbTag.fileType);
 		make_bad_inode(inode);
@@ -1438,7 +1438,7 @@ static int udf_alloc_i_data(struct inode *inode, size_t size)
 	iinfo->i_ext.i_data = kmalloc(size, GFP_KERNEL);
 
 	if (!iinfo->i_ext.i_data) {
-		printk(KERN_ERR "udf:udf_alloc_i_data (ino %ld) "
+		pr_err("udf_alloc_i_data (ino %ld) "
 				"no free memory\n", inode->i_ino);
 		return -ENOMEM;
 	}
@@ -1689,7 +1689,7 @@ out:
 	if (do_sync) {
 		sync_dirty_buffer(bh);
 		if (buffer_write_io_error(bh)) {
-			printk(KERN_WARNING "IO error syncing udf inode "
+			pr_warn("IO error syncing udf inode "
 				"[%s:%08lx]\n", inode->i_sb->s_id,
 				inode->i_ino);
 			err = -EIO;
diff --git a/fs/udf/misc.c b/fs/udf/misc.c
index 9215700..4f0345e 100644
--- a/fs/udf/misc.c
+++ b/fs/udf/misc.c
@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block,
 {
 	struct tag *tag_p;
 	struct buffer_head *bh = NULL;
+	u8 checksum;
 
 	/* Read the block */
 	if (block == 0xFFFFFFFF)
@@ -211,7 +212,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block,
 
 	bh = udf_tread(sb, block);
 	if (!bh) {
-		udf_debug("block=%d, location=%d: read failed\n",
+		pr_err("block=%d, location=%d: read failed\n",
 			  block, location);
 		return NULL;
 	}
@@ -221,22 +222,24 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block,
 	*ident = le16_to_cpu(tag_p->tagIdent);
 
 	if (location != le32_to_cpu(tag_p->tagLocation)) {
-		udf_debug("location mismatch block %u, tag %u != %u\n",
+		pr_err("location mismatch block %u, tag %u != %u\n",
 			  block, le32_to_cpu(tag_p->tagLocation), location);
 		goto error_out;
 	}
 
 	/* Verify the tag checksum */
-	if (udf_tag_checksum(tag_p) != tag_p->tagChecksum) {
-		printk(KERN_ERR "udf: tag checksum failed block %d\n", block);
+	checksum = udf_tag_checksum(tag_p);
+	if (checksum != tag_p->tagChecksum) {
+		pr_err("tag checksum failed block %d, checksum 0x%02x != 0x%02x\n",
+		block, checksum, tag_p->tagChecksum);
 		goto error_out;
 	}
 
 	/* Verify the tag version */
 	if (tag_p->descVersion != cpu_to_le16(0x0002U) &&
 	    tag_p->descVersion != cpu_to_le16(0x0003U)) {
-		udf_debug("tag version 0x%04x != 0x0002 || 0x0003 block %d\n",
-			  le16_to_cpu(tag_p->descVersion), block);
+		pr_err("tag version 0x%04x != 0x0002 || 0x0003 block %d\n",
+		le16_to_cpu(tag_p->descVersion), block);
 		goto error_out;
 	}
 
@@ -247,7 +250,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block,
 					le16_to_cpu(tag_p->descCRCLength)))
 		return bh;
 
-	udf_debug("Crc failure block %d: crc = %d, crclen = %d\n", block,
+	pr_err("Crc failure block %d: crc = %d, crclen = %d\n", block,
 	    le16_to_cpu(tag_p->descCRC), le16_to_cpu(tag_p->descCRCLength));
 
 error_out:
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 7b27b06..bbf6256 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -550,7 +550,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt,
 			uopt->dmode = option & 0777;
 			break;
 		default:
-			printk(KERN_ERR "udf: bad mount option \"%s\" "
+			pr_err("bad mount option \"%s\" "
 			       "or missing value\n", p);
 			return 0;
 		}
diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
index 8424308..65c11cd 100644
--- a/fs/udf/truncate.c
+++ b/fs/udf/truncate.c
@@ -95,8 +95,7 @@ void udf_truncate_tail_extent(struct inode *inode)
 		lbcount += elen;
 		if (lbcount > inode->i_size) {
 			if (lbcount - inode->i_size >= inode->i_sb->s_blocksize)
-				printk(KERN_WARNING
-				       "udf_truncate_tail_extent(): Too long "
+				pr_warn("udf_truncate_tail_extent(): Too long "
 				       "extent after EOF in inode %u: i_size: "
 				       "%Ld lbcount: %Ld extent %u+%u\n",
 				       (unsigned)inode->i_ino,
@@ -109,7 +108,7 @@ void udf_truncate_tail_extent(struct inode *inode)
 			extent_trunc(inode, &epos, &eloc, etype, elen, nelen);
 			epos.offset += adsize;
 			if (udf_next_aext(inode, &epos, &eloc, &elen, 1) != -1)
-				printk(KERN_ERR "udf_truncate_tail_extent(): "
+				pr_err("udf_truncate_tail_extent(): "
 				       "Extent after EOF in inode %u.\n",
 				       (unsigned)inode->i_ino);
 			break;
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index dbd52d4b..4ff06f3 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -1,6 +1,8 @@
 #ifndef __UDF_DECL_H
 #define __UDF_DECL_H
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include "ecma_167.h"
 #include "osta_udf.h"
 
@@ -19,18 +21,15 @@
 #undef UDFFS_DEBUG
 
 #ifdef UDFFS_DEBUG
-#define udf_debug(f, a...) \
-do { \
-	printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \
-		__FILE__, __LINE__, __func__); \
-	printk(f, ##a); \
-} while (0)
+#define udf_debug(fmt, ...)	\
+	pr_debug("DEBUG %s:%d:%s: ", fmt,	\
+		__FILE__, __LINE__, __func__, __VA_ARGS__);
 #else
-#define udf_debug(f, a...) /**/
+#define udf_debug(fmt, ...) /**/
 #endif
 
 #define udf_info(f, a...) \
-	printk(KERN_INFO "UDF-fs INFO " f, ##a);
+	pr_info("INFO " f, ##a);
 
 
 #define udf_fixed_to_variable(x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )
diff --git a/fs/udf/udftime.c b/fs/udf/udftime.c
index b8c828c..927c931 100644
--- a/fs/udf/udftime.c
+++ b/fs/udf/udftime.c
@@ -34,9 +34,9 @@
  * http://www.boulder.nist.gov/timefreq/pubs/bulletin/leapsecond.htm
  */
 
+#include "udfdecl.h"
 #include <linux/types.h>
 #include <linux/kernel.h>
-#include "udfdecl.h"
 
 #define EPOCH_YEAR 1970
 
diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c
index d03a90b..9455dc7 100644
--- a/fs/udf/unicode.c
+++ b/fs/udf/unicode.c
@@ -114,7 +114,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)
 	cmp_id = ocu_i->u_cmpID;
 	if (cmp_id != 8 && cmp_id != 16) {
 		memset(utf_o, 0, sizeof(struct ustr));
-		printk(KERN_ERR "udf: unknown compression code (%d) stri=%s\n",
+		pr_err("unknown compression code (%d) stri=%s\n",
 		       cmp_id, ocu_i->u_name);
 		return 0;
 	}
@@ -242,7 +242,7 @@ try_again:
 	if (utf_cnt) {
 error_out:
 		ocu[++u_len] = '?';
-		printk(KERN_DEBUG "udf: bad UTF-8 character\n");
+		pr_debug("bad UTF-8 character\n");
 	}
 
 	ocu[length - 1] = (uint8_t)u_len + 1;
@@ -267,7 +267,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,
 	cmp_id = ocu_i->u_cmpID;
 	if (cmp_id != 8 && cmp_id != 16) {
 		memset(utf_o, 0, sizeof(struct ustr));
-		printk(KERN_ERR "udf: unknown compression code (%d) stri=%s\n",
+		pr_err("unknown compression code (%d) stri=%s\n",
 		       cmp_id, ocu_i->u_name);
 		return 0;
 	}
-- 
1.7.4.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2 v2] udf : enable error print in udf_read_tagged().
  2011-10-03  7:19 [PATCH 1/2 v2] udf : enable error print in udf_read_tagged() Namjae Jeon
@ 2011-10-03 12:57 ` Joe Perches
  2011-10-03 13:58   ` NamJae Jeon
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2011-10-03 12:57 UTC (permalink / raw)
  To: Namjae Jeon; +Cc: jack, linux-kernel

On Mon, 2011-10-03 at 16:19 +0900, Namjae Jeon wrote:
> And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.
[]
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
[]
> @@ -19,18 +21,15 @@
>  #undef UDFFS_DEBUG
>  
>  #ifdef UDFFS_DEBUG
> -#define udf_debug(f, a...) \
> -do { \
> -	printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \
> -		__FILE__, __LINE__, __func__); \
> -	printk(f, ##a); \
> -} while (0)
> +#define udf_debug(fmt, ...)	\
> +	pr_debug("DEBUG %s:%d:%s: ", fmt,	\
> +		__FILE__, __LINE__, __func__, __VA_ARGS__);

This doesn't compile properly if UDFFS_DEBUG is
#defined.

No comma should be between "DEBUG %s:%d:%s: " and fmt.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2 v2] udf : enable error print in udf_read_tagged().
  2011-10-03 12:57 ` Joe Perches
@ 2011-10-03 13:58   ` NamJae Jeon
  2011-10-03 22:17     ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: NamJae Jeon @ 2011-10-03 13:58 UTC (permalink / raw)
  To: Joe Perches; +Cc: jack, linux-kernel

2011/10/3 Joe Perches <joe@perches.com>:
> On Mon, 2011-10-03 at 16:19 +0900, Namjae Jeon wrote:
>> And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.
> []
>> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> []
>> @@ -19,18 +21,15 @@
>>  #undef UDFFS_DEBUG
>>
>>  #ifdef UDFFS_DEBUG
>> -#define udf_debug(f, a...) \
>> -do { \
>> -     printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \
>> -             __FILE__, __LINE__, __func__); \
>> -     printk(f, ##a); \
>> -} while (0)
>> +#define udf_debug(fmt, ...)  \
>> +     pr_debug("DEBUG %s:%d:%s: ", fmt,       \
>> +             __FILE__, __LINE__, __func__, __VA_ARGS__);
>
> This doesn't compile properly if UDFFS_DEBUG is
> #defined.
>
> No comma should be between "DEBUG %s:%d:%s: " and fmt.
Hi. Joe.
oops! it's my mistake.
I  sent v3 patch again.
Would you review one more ?
Thanks.
>
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2 v2] udf : enable error print in udf_read_tagged().
  2011-10-03 13:58   ` NamJae Jeon
@ 2011-10-03 22:17     ` Joe Perches
  2011-10-03 23:20       ` NamJae Jeon
  0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2011-10-03 22:17 UTC (permalink / raw)
  To: NamJae Jeon; +Cc: jack, linux-kernel

On Mon, 2011-10-03 at 22:58 +0900, NamJae Jeon wrote:
> 2011/10/3 Joe Perches <joe@perches.com>:
> > On Mon, 2011-10-03 at 16:19 +0900, Namjae Jeon wrote:
> >> And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.
> > []
> >> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> > []
> >> @@ -19,18 +21,15 @@
> >>  #undef UDFFS_DEBUG
> >>
> >>  #ifdef UDFFS_DEBUG
> >> -#define udf_debug(f, a...) \
> >> -do { \
> >> -     printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \
> >> -             __FILE__, __LINE__, __func__); \
> >> -     printk(f, ##a); \
> >> -} while (0)
> >> +#define udf_debug(fmt, ...)  \
> >> +     pr_debug("DEBUG %s:%d:%s: ", fmt,       \
> >> +             __FILE__, __LINE__, __func__, __VA_ARGS__);
> >
> > This doesn't compile properly if UDFFS_DEBUG is
> > #defined.
> >
> > No comma should be between "DEBUG %s:%d:%s: " and fmt.
> Hi. Joe.
> oops! it's my mistake.
> I  sent v3 patch again.
> Would you review one more ?

It looks OK except for a few issues.

The udf_debug macro doesn't need a semicolon after
pr_debug(foo); it should be pr_debug(foo)
Same for udf_info.

The output prefixes are a mixture of "udf: " and "UDF-fs: "

Some lines are printed with 2 newlines.

I've done the patch below on top of your patch.
It should fix up these trivial things.

Perhaps you'll inspect this too.

Convert printks to pr_<level>.
Coalesce formats.
Rename udf_warning to udf_warn.
Add __attribute__((format(printf... verification to udf_warn.
Rename udf_error to udf_err.
Remove static from udf_err for consistency with other udf_<level> uses.
Use printf extension "%pV" in udf_warn and udf_err
Remove now unused static char error_buf[1024].
Add no_printk to udf_debug when not #define UDF_DEBUG.
Align multiline arguments to open parenthesis.
Add newline to message formats where appropriate.

---

 fs/udf/balloc.c    |    9 +-
 fs/udf/directory.c |    9 +-
 fs/udf/inode.c     |    3 +-
 fs/udf/namei.c     |    6 +-
 fs/udf/partition.c |   12 ++--
 fs/udf/super.c     |  213 ++++++++++++++++++++++++----------------------------
 fs/udf/udfdecl.h   |   26 ++++---
 7 files changed, 132 insertions(+), 146 deletions(-)

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index 95518a9..48bd5c7 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -59,8 +59,8 @@ static int __load_block_bitmap(struct super_block *sb,
 	int nr_groups = bitmap->s_nr_groups;
 
 	if (block_group >= nr_groups) {
-		udf_debug("block_group (%d) > nr_groups (%d)\n", block_group,
-			  nr_groups);
+		udf_debug("block_group (%d) > nr_groups (%d)\n",
+			  block_group, nr_groups);
 	}
 
 	if (bitmap->s_block_bitmap[block_group]) {
@@ -155,7 +155,7 @@ static void udf_bitmap_free_blocks(struct super_block *sb,
 			if (udf_set_bit(bit + i, bh->b_data)) {
 				udf_debug("bit %ld already set\n", bit + i);
 				udf_debug("byte=%2x\n",
-					((char *)bh->b_data)[(bit + i) >> 3]);
+					  ((char *)bh->b_data)[(bit + i) >> 3]);
 			}
 		}
 		udf_add_free_space(sb, sbi->s_partition, count);
@@ -369,7 +369,8 @@ static void udf_table_free_blocks(struct super_block *sb,
 	if (bloc->logicalBlockNum + count < count ||
 	    (bloc->logicalBlockNum + count) > partmap->s_partition_len) {
 		udf_debug("%d < %d || %d + %d > %d\n",
-			  bloc->logicalBlockNum, 0, bloc->logicalBlockNum, count,
+			  bloc->logicalBlockNum, 0,
+			  bloc->logicalBlockNum, count,
 			  partmap->s_partition_len);
 		goto error_return;
 	}
diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index a4caf7a..73136e6 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -162,8 +162,8 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)
 	int padlen;
 
 	if ((!buffer) || (!offset)) {
-		udf_debug("invalidparms\n, buffer=%p, offset=%p\n", buffer,
-			  offset);
+		udf_debug("invalidparms, buffer=%p, offset=%p\n",
+			  buffer, offset);
 		return NULL;
 	}
 
@@ -175,9 +175,8 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)
 	if (fi->descTag.tagIdent != cpu_to_le16(TAG_IDENT_FID)) {
 		udf_debug("0x%x != TAG_IDENT_FID\n",
 			  le16_to_cpu(fi->descTag.tagIdent));
-		udf_debug("offset: %u sizeof: %lu bufsize: %u\n",
-			  *offset, (unsigned long)sizeof(struct fileIdentDesc),
-			  bufsize);
+		udf_debug("offset: %u sizeof: %zu bufsize: %u\n",
+			  *offset, sizeof(struct fileIdentDesc), bufsize);
 		return NULL;
 	}
 	if ((*offset + sizeof(struct fileIdentDesc)) > bufsize)
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 58665ce..6621c2a 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -1982,8 +1982,7 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
 		*elen = le32_to_cpu(lad->extLength) & UDF_EXTENT_LENGTH_MASK;
 		break;
 	default:
-		udf_debug("alloc_type = %d unsupported\n",
-				iinfo->i_alloc_type);
+		udf_debug("alloc_type = %d unsupported\n", iinfo->i_alloc_type);
 		return -1;
 	}
 
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index f1dce84..bd936c9 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -799,9 +799,9 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
 	if (retval)
 		goto end_rmdir;
 	if (inode->i_nlink != 2)
-		udf_warning(inode->i_sb, "udf_rmdir",
-			    "empty directory has nlink != 2 (%d)",
-			    inode->i_nlink);
+		udf_warn(inode->i_sb, "udf_rmdir",
+			 "empty directory has nlink != 2 (%d)\n",
+			 inode->i_nlink);
 	clear_nlink(inode);
 	inode->i_size = 0;
 	inode_dec_link_count(dir);
diff --git a/fs/udf/partition.c b/fs/udf/partition.c
index a71090e..849039a 100644
--- a/fs/udf/partition.c
+++ b/fs/udf/partition.c
@@ -33,8 +33,8 @@ uint32_t udf_get_pblock(struct super_block *sb, uint32_t block,
 	struct udf_sb_info *sbi = UDF_SB(sb);
 	struct udf_part_map *map;
 	if (partition >= sbi->s_partitions) {
-		udf_debug("block=%d, partition=%d, offset=%d: "
-			  "invalid partition\n", block, partition, offset);
+		udf_debug("block=%d, partition=%d, offset=%d: invalid partition\n",
+			  block, partition, offset);
 		return 0xFFFFFFFF;
 	}
 	map = &sbi->s_partmaps[partition];
@@ -60,8 +60,8 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block,
 	vdata = &map->s_type_specific.s_virtual;
 
 	if (block > vdata->s_num_entries) {
-		udf_debug("Trying to access block beyond end of VAT "
-			  "(%d max %d)\n", block, vdata->s_num_entries);
+		udf_debug("Trying to access block beyond end of VAT (%d max %d)\n",
+			  block, vdata->s_num_entries);
 		return 0xFFFFFFFF;
 	}
 
@@ -322,8 +322,8 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
 	BUG_ON(!inode);
 	retblk = udf_try_read_meta(inode, block, partition, offset);
 	if (retblk == 0xFFFFFFFF) {
-		udf_warning(sb, __func__, "error reading from METADATA, "
-			"trying to read from MIRROR");
+		udf_warn(sb, __func__,
+			 "error reading from METADATA, trying to read from MIRROR\n");
 		inode = mdata->s_mirror_fe;
 		if (!inode)
 			return 0xFFFFFFFF;
diff --git a/fs/udf/super.c b/fs/udf/super.c
index bbf6256..ab921f9 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -75,8 +75,6 @@
 
 #define UDF_DEFAULT_BLOCKSIZE 2048
 
-static char error_buf[1024];
-
 /* These are the "meat" - everything else is stuffing */
 static int udf_fill_super(struct super_block *, void *, int);
 static void udf_put_super(struct super_block *);
@@ -92,8 +90,6 @@ static void udf_close_lvid(struct super_block *);
 static unsigned int udf_count_free(struct super_block *);
 static int udf_statfs(struct dentry *, struct kstatfs *);
 static int udf_show_options(struct seq_file *, struct vfsmount *);
-static void udf_error(struct super_block *sb, const char *function,
-		      const char *fmt, ...);
 
 struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
 {
@@ -244,9 +240,9 @@ static int udf_sb_alloc_partition_maps(struct super_block *sb, u32 count)
 	sbi->s_partmaps = kcalloc(count, sizeof(struct udf_part_map),
 				  GFP_KERNEL);
 	if (!sbi->s_partmaps) {
-		udf_error(sb, __func__,
-			  "Unable to allocate space for %d partition maps",
-			  count);
+		udf_err(sb, __func__,
+			"Unable to allocate space for %d partition maps\n",
+			count);
 		sbi->s_partitions = 0;
 		return -ENOMEM;
 	}
@@ -550,8 +546,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt,
 			uopt->dmode = option & 0777;
 			break;
 		default:
-			pr_err("bad mount option \"%s\" "
-			       "or missing value\n", p);
+			pr_err("bad mount option \"%s\" or missing value\n", p);
 			return 0;
 		}
 	}
@@ -645,20 +640,16 @@ static loff_t udf_check_vsd(struct super_block *sb)
 				udf_debug("ISO9660 Boot Record found\n");
 				break;
 			case 1:
-				udf_debug("ISO9660 Primary Volume Descriptor "
-					  "found\n");
+				udf_debug("ISO9660 Primary Volume Descriptor found\n");
 				break;
 			case 2:
-				udf_debug("ISO9660 Supplementary Volume "
-					  "Descriptor found\n");
+				udf_debug("ISO9660 Supplementary Volume Descriptor found\n");
 				break;
 			case 3:
-				udf_debug("ISO9660 Volume Partition Descriptor "
-					  "found\n");
+				udf_debug("ISO9660 Volume Partition Descriptor found\n");
 				break;
 			case 255:
-				udf_debug("ISO9660 Volume Descriptor Set "
-					  "Terminator found\n");
+				udf_debug("ISO9660 Volume Descriptor Set Terminator found\n");
 				break;
 			default:
 				udf_debug("ISO9660 VRS (%u) found\n",
@@ -809,8 +800,7 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
 			      pvoldesc->recordingDateAndTime)) {
 #ifdef UDFFS_DEBUG
 		struct timestamp *ts = &pvoldesc->recordingDateAndTime;
-		udf_debug("recording time %04u/%02u/%02u"
-			  " %02u:%02u (%x)\n",
+		udf_debug("recording time %04u/%02u/%02u %02u:%02u (%x)\n",
 			  le16_to_cpu(ts->year), ts->month, ts->day, ts->hour,
 			  ts->minute, le16_to_cpu(ts->typeAndTimezone));
 #endif
@@ -821,7 +811,7 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
 			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);
+				  UDF_SB(sb)->s_volume_ident);
 		}
 
 	if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
@@ -853,18 +843,16 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
 	addr.partitionReferenceNum = map->s_partition_num;
 
 	udf_debug("Metadata file location: block = %d part = %d\n",
-			  addr.logicalBlockNum, addr.partitionReferenceNum);
+		  addr.logicalBlockNum, addr.partitionReferenceNum);
 
 	mdata->s_metadata_fe = udf_iget(sb, &addr);
 
 	if (mdata->s_metadata_fe == NULL) {
-		udf_warning(sb, __func__, "metadata inode efe not found, "
-				"will try mirror inode.");
+		udf_warn(sb, __func__, "metadata inode efe not found, will try mirror inode\n");
 		fe_error = 1;
 	} else if (UDF_I(mdata->s_metadata_fe)->i_alloc_type !=
 		 ICBTAG_FLAG_AD_SHORT) {
-		udf_warning(sb, __func__, "metadata inode efe does not have "
-			"short allocation descriptors!");
+		udf_warn(sb, __func__, "metadata inode efe does not have short allocation descriptors!\n");
 		fe_error = 1;
 		iput(mdata->s_metadata_fe);
 		mdata->s_metadata_fe = NULL;
@@ -875,22 +863,22 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
 	addr.partitionReferenceNum = map->s_partition_num;
 
 	udf_debug("Mirror metadata file location: block = %d part = %d\n",
-			  addr.logicalBlockNum, addr.partitionReferenceNum);
+		  addr.logicalBlockNum, addr.partitionReferenceNum);
 
 	mdata->s_mirror_fe = udf_iget(sb, &addr);
 
 	if (mdata->s_mirror_fe == NULL) {
 		if (fe_error) {
-			udf_error(sb, __func__, "mirror inode efe not found "
-			"and metadata inode is missing too, exiting...");
+			udf_err(sb, __func__,
+				"mirror inode efe not found and metadata inode is missing too, exiting...\n");
 			goto error_exit;
 		} else
-			udf_warning(sb, __func__, "mirror inode efe not found,"
-					" but metadata inode is OK");
+			udf_warn(sb, __func__,
+				 "mirror inode efe not found, but metadata inode is OK\n");
 	} else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
 		 ICBTAG_FLAG_AD_SHORT) {
-		udf_warning(sb, __func__, "mirror inode efe does not have "
-			"short allocation descriptors!");
+		udf_warn(sb, __func__,
+			 "mirror inode efe does not have short allocation descriptors!\n");
 		iput(mdata->s_mirror_fe);
 		mdata->s_mirror_fe = NULL;
 		if (fe_error)
@@ -907,18 +895,17 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
 		addr.partitionReferenceNum = map->s_partition_num;
 
 		udf_debug("Bitmap file location: block = %d part = %d\n",
-			addr.logicalBlockNum, addr.partitionReferenceNum);
+			  addr.logicalBlockNum, addr.partitionReferenceNum);
 
 		mdata->s_bitmap_fe = udf_iget(sb, &addr);
 
 		if (mdata->s_bitmap_fe == NULL) {
 			if (sb->s_flags & MS_RDONLY)
-				udf_warning(sb, __func__, "bitmap inode efe "
-					"not found but it's ok since the disc"
-					" is mounted read-only");
+				udf_warn(sb, __func__,
+					 "bitmap inode efe not found but it's ok since the disc is mounted read-only\n");
 			else {
-				udf_error(sb, __func__, "bitmap inode efe not "
-					"found and attempted read-write mount");
+				udf_err(sb, __func__,
+					"bitmap inode efe not found and attempted read-write mount\n");
 				goto error_exit;
 			}
 		}
@@ -971,9 +958,9 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
 		bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
 
 	if (bitmap == NULL) {
-		udf_error(sb, __func__,
-			  "Unable to allocate space for bitmap "
-			  "and %d buffer_head pointers", nr_groups);
+		udf_err(sb, __func__,
+			"Unable to allocate space for bitmap and %d buffer_head pointers\n",
+			nr_groups);
 		return NULL;
 	}
 
@@ -1003,8 +990,8 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 	if (p->accessType == cpu_to_le32(PD_ACCESS_TYPE_OVERWRITABLE))
 		map->s_partition_flags |= UDF_PART_FLAG_OVERWRITABLE;
 
-	udf_debug("Partition (%d type %x) starts at physical %d, "
-		  "block length %d\n", p_index,
+	udf_debug("Partition (%d type %x) starts at physical %d, block length %d\n",
+		  p_index,
 		  map->s_partition_type, map->s_partition_root,
 		  map->s_partition_len);
 
@@ -1023,12 +1010,12 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 		map->s_uspace.s_table = udf_iget(sb, &loc);
 		if (!map->s_uspace.s_table) {
 			udf_debug("cannot load unallocSpaceTable (part %d)\n",
-					p_index);
+				  p_index);
 			return 1;
 		}
 		map->s_partition_flags |= UDF_PART_FLAG_UNALLOC_TABLE;
 		udf_debug("unallocSpaceTable (part %d) @ %ld\n",
-				p_index, map->s_uspace.s_table->i_ino);
+			  p_index, map->s_uspace.s_table->i_ino);
 	}
 
 	if (phd->unallocSpaceBitmap.extLength) {
@@ -1041,8 +1028,8 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 		bitmap->s_extPosition = le32_to_cpu(
 				phd->unallocSpaceBitmap.extPosition);
 		map->s_partition_flags |= UDF_PART_FLAG_UNALLOC_BITMAP;
-		udf_debug("unallocSpaceBitmap (part %d) @ %d\n", p_index,
-						bitmap->s_extPosition);
+		udf_debug("unallocSpaceBitmap (part %d) @ %d\n",
+			  p_index, bitmap->s_extPosition);
 	}
 
 	if (phd->partitionIntegrityTable.extLength)
@@ -1058,13 +1045,13 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 		map->s_fspace.s_table = udf_iget(sb, &loc);
 		if (!map->s_fspace.s_table) {
 			udf_debug("cannot load freedSpaceTable (part %d)\n",
-				p_index);
+				  p_index);
 			return 1;
 		}
 
 		map->s_partition_flags |= UDF_PART_FLAG_FREED_TABLE;
 		udf_debug("freedSpaceTable (part %d) @ %ld\n",
-				p_index, map->s_fspace.s_table->i_ino);
+			  p_index, map->s_fspace.s_table->i_ino);
 	}
 
 	if (phd->freedSpaceBitmap.extLength) {
@@ -1077,8 +1064,8 @@ static int udf_fill_partdesc_info(struct super_block *sb,
 		bitmap->s_extPosition = le32_to_cpu(
 				phd->freedSpaceBitmap.extPosition);
 		map->s_partition_flags |= UDF_PART_FLAG_FREED_BITMAP;
-		udf_debug("freedSpaceBitmap (part %d) @ %d\n", p_index,
-					bitmap->s_extPosition);
+		udf_debug("freedSpaceBitmap (part %d) @ %d\n",
+			  p_index, bitmap->s_extPosition);
 	}
 	return 0;
 }
@@ -1118,11 +1105,9 @@ static int udf_load_vat(struct super_block *sb, int p_index, int type1_index)
 	udf_find_vat_block(sb, p_index, type1_index, sbi->s_last_block);
 	if (!sbi->s_vat_inode &&
 	    sbi->s_last_block != blocks - 1) {
-		printk(KERN_NOTICE "UDF-fs: Failed to read VAT inode from the"
-		       " last recorded block (%lu), retrying with the last "
-		       "block of the device (%lu).\n",
-		       (unsigned long)sbi->s_last_block,
-		       (unsigned long)blocks - 1);
+		pr_notice("Failed to read VAT inode from the last recorded block (%lu), retrying with the last block of the device (%lu)\n",
+			  (unsigned long)sbi->s_last_block,
+			  (unsigned long)blocks - 1);
 		udf_find_vat_block(sb, p_index, type1_index, blocks - 1);
 	}
 	if (!sbi->s_vat_inode)
@@ -1220,8 +1205,7 @@ static int udf_load_partdesc(struct super_block *sb, sector_t block)
 	if (map->s_partition_type == UDF_METADATA_MAP25) {
 		ret = udf_load_metadata_files(sb, i);
 		if (ret) {
-			printk(KERN_ERR "UDF-fs: error loading MetaData "
-			"partition map %d\n", i);
+			pr_err("error loading MetaData partition map %d\n", i);
 			goto out_bh;
 		}
 	} else {
@@ -1234,9 +1218,7 @@ static int udf_load_partdesc(struct super_block *sb, sector_t block)
 		 * overwrite blocks instead of relocating them).
 		 */
 		sb->s_flags |= MS_RDONLY;
-		printk(KERN_NOTICE "UDF-fs: Filesystem marked read-only "
-			"because writing to pseudooverwrite partition is "
-			"not implemented.\n");
+		pr_notice("Filesystem marked read-only because writing to pseudooverwrite partition is not implemented\n");
 	}
 out_bh:
 	/* In case loading failed, we handle cleanup in udf_fill_super */
@@ -1344,9 +1326,8 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
 				struct metadataPartitionMap *mdm =
 						(struct metadataPartitionMap *)
 						&(lvd->partitionMaps[offset]);
-				udf_debug("Parsing Logical vol part %d "
-					"type %d  id=%s\n", i, type,
-					UDF_ID_METADATA);
+				udf_debug("Parsing Logical vol part %d type %d  id=%s\n",
+					  i, type, UDF_ID_METADATA);
 
 				map->s_partition_type = UDF_METADATA_MAP25;
 				map->s_partition_func = udf_get_pblock_meta25;
@@ -1365,21 +1346,20 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
 					mdm->flags & 0x01;
 
 				udf_debug("Metadata Ident suffix=0x%x\n",
-					(le16_to_cpu(
-					 ((__le16 *)
-					      mdm->partIdent.identSuffix)[0])));
+					  le16_to_cpu(*(__le16 *)
+						       mdm->partIdent.identSuffix));
 				udf_debug("Metadata part num=%d\n",
-					le16_to_cpu(mdm->partitionNum));
+					  le16_to_cpu(mdm->partitionNum));
 				udf_debug("Metadata part alloc unit size=%d\n",
-					le32_to_cpu(mdm->allocUnitSize));
+					  le32_to_cpu(mdm->allocUnitSize));
 				udf_debug("Metadata file loc=%d\n",
-					le32_to_cpu(mdm->metadataFileLoc));
+					  le32_to_cpu(mdm->metadataFileLoc));
 				udf_debug("Mirror file loc=%d\n",
-				       le32_to_cpu(mdm->metadataMirrorFileLoc));
+					  le32_to_cpu(mdm->metadataMirrorFileLoc));
 				udf_debug("Bitmap file loc=%d\n",
-				       le32_to_cpu(mdm->metadataBitmapFileLoc));
+					  le32_to_cpu(mdm->metadataBitmapFileLoc));
 				udf_debug("Duplicate Flag: %d %d\n",
-					mdata->s_dup_md_flag, mdm->flags);
+					  mdata->s_dup_md_flag, mdm->flags);
 			} else {
 				udf_debug("Unknown ident: %s\n",
 					  upm2->partIdent.ident);
@@ -1397,8 +1377,8 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
 		struct long_ad *la = (struct long_ad *)&(lvd->logicalVolContentsUse[0]);
 
 		*fileset = lelb_to_cpu(la->extLocation);
-		udf_debug("FileSet found in LogicalVolDesc at block=%d, "
-			  "partition=%d\n", fileset->logicalBlockNum,
+		udf_debug("FileSet found in LogicalVolDesc at block=%d, partition=%d\n",
+			  fileset->logicalBlockNum,
 			  fileset->partitionReferenceNum);
 	}
 	if (lvd->integritySeqExt.extLength)
@@ -1478,9 +1458,8 @@ static noinline int udf_process_sequence(struct super_block *sb, long block,
 
 		bh = udf_read_tagged(sb, block, block, &ident);
 		if (!bh) {
-			printk(KERN_ERR "udf: Block %Lu of volume descriptor "
-			       "sequence is corrupted or we could not read "
-			       "it.\n", (unsigned long long)block);
+			pr_err("Block %lu of volume descriptor sequence is corrupted or we could not read it\n",
+			       (unsigned long)block);
 			return 1;
 		}
 
@@ -1553,7 +1532,7 @@ static noinline int udf_process_sequence(struct super_block *sb, long block,
 	 * in a suitable order
 	 */
 	if (!vds[VDS_POS_PRIMARY_VOL_DESC].block) {
-		printk(KERN_ERR "udf: Primary Volume Descriptor not found!\n");
+		pr_err("Primary Volume Descriptor not found!\n");
 		return 1;
 	}
 	if (udf_load_pvoldesc(sb, vds[VDS_POS_PRIMARY_VOL_DESC].block))
@@ -1740,7 +1719,7 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
 
 	if (!sb_set_blocksize(sb, uopt->blocksize)) {
 		if (!silent)
-			printk(KERN_WARNING "UDF-fs: Bad block size\n");
+			pr_warn("Bad block size\n");
 		return 0;
 	}
 	sbi->s_last_block = uopt->lastblock;
@@ -1749,12 +1728,11 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
 		nsr_off = udf_check_vsd(sb);
 		if (!nsr_off) {
 			if (!silent)
-				printk(KERN_WARNING "UDF-fs: No VRS found\n");
+				pr_warn("No VRS found\n");
 			return 0;
 		}
 		if (nsr_off == -1)
-			udf_debug("Failed to read byte 32768. Assuming open "
-				  "disc. Skipping validity check\n");
+			udf_debug("Failed to read byte 32768. Assuming open disc. Skipping validity check\n");
 		if (!sbi->s_last_block)
 			sbi->s_last_block = udf_get_last_block(sb);
 	} else {
@@ -1765,7 +1743,7 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
 	sbi->s_anchor = uopt->anchor;
 	if (!udf_find_anchor(sb, fileset)) {
 		if (!silent)
-			printk(KERN_WARNING "UDF-fs: No anchor found\n");
+			pr_warn("No anchor found\n");
 		return 0;
 	}
 	return 1;
@@ -1937,8 +1915,8 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 
 	if (uopt.flags & (1 << UDF_FLAG_UTF8) &&
 	    uopt.flags & (1 << UDF_FLAG_NLS_MAP)) {
-		udf_error(sb, "udf_read_super",
-			  "utf8 cannot be combined with iocharset\n");
+		udf_err(sb, "udf_read_super",
+			"utf8 cannot be combined with iocharset\n");
 		goto error_out;
 	}
 #ifdef CONFIG_UDF_NLS
@@ -1987,15 +1965,14 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 		ret = udf_load_vrs(sb, &uopt, silent, &fileset);
 		if (!ret && uopt.blocksize != UDF_DEFAULT_BLOCKSIZE) {
 			if (!silent)
-				printk(KERN_NOTICE
-				       "UDF-fs: Rescanning with blocksize "
-				       "%d\n", UDF_DEFAULT_BLOCKSIZE);
+				pr_notice("Rescanning with blocksize %d\n",
+					  UDF_DEFAULT_BLOCKSIZE);
 			uopt.blocksize = UDF_DEFAULT_BLOCKSIZE;
 			ret = udf_load_vrs(sb, &uopt, silent, &fileset);
 		}
 	}
 	if (!ret) {
-		printk(KERN_WARNING "UDF-fs: No partition found (1)\n");
+		pr_warn("No partition found (1)\n");
 		goto error_out;
 	}
 
@@ -2010,8 +1987,7 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 				le16_to_cpu(lvidiu->maxUDFWriteRev); */
 
 		if (minUDFReadRev > UDF_MAX_READ_VERSION) {
-			printk(KERN_ERR "UDF-fs: minUDFReadRev=%x "
-					"(max is %x)\n",
+			pr_err("minUDFReadRev=%x (max is %x)\n",
 			       le16_to_cpu(lvidiu->minUDFReadRev),
 			       UDF_MAX_READ_VERSION);
 			goto error_out;
@@ -2027,27 +2003,25 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 	}
 
 	if (!sbi->s_partitions) {
-		printk(KERN_WARNING "UDF-fs: No partition found (2)\n");
+		pr_warn("No partition found (2)\n");
 		goto error_out;
 	}
 
 	if (sbi->s_partmaps[sbi->s_partition].s_partition_flags &
 			UDF_PART_FLAG_READ_ONLY) {
-		printk(KERN_NOTICE "UDF-fs: Partition marked readonly; "
-				   "forcing readonly mount\n");
+		pr_notice("Partition marked readonly; forcing readonly mount\n");
 		sb->s_flags |= MS_RDONLY;
 	}
 
 	if (udf_find_fileset(sb, &fileset, &rootdir)) {
-		printk(KERN_WARNING "UDF-fs: No fileset found\n");
+		pr_warn("No fileset found\n");
 		goto error_out;
 	}
 
 	if (!silent) {
 		struct timestamp ts;
 		udf_time_to_disk_stamp(&ts, sbi->s_record_time);
-		udf_info("UDF: Mounting volume '%s', "
-			 "timestamp %04u/%02u/%02u %02u:%02u (%x)\n",
+		udf_info("Mounting volume '%s', timestamp %04u/%02u/%02u %02u:%02u (%x)\n",
 			 sbi->s_volume_ident, le16_to_cpu(ts.year), ts.month, ts.day,
 			 ts.hour, ts.minute, le16_to_cpu(ts.typeAndTimezone));
 	}
@@ -2059,8 +2033,7 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 	/* perhaps it's not extensible enough, but for now ... */
 	inode = udf_iget(sb, &rootdir);
 	if (!inode) {
-		printk(KERN_ERR "UDF-fs: Error in udf_iget, block=%d, "
-				"partition=%d\n",
+		pr_err("Error in udf_iget, block=%d, partition=%d\n",
 		       rootdir.logicalBlockNum, rootdir.partitionReferenceNum);
 		goto error_out;
 	}
@@ -2068,7 +2041,7 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
 	/* Allocate a dentry for the root inode */
 	sb->s_root = d_alloc_root(inode);
 	if (!sb->s_root) {
-		printk(KERN_ERR "UDF-fs: Couldn't allocate root dentry\n");
+		pr_err("Couldn't allocate root dentry\n");
 		iput(inode);
 		goto error_out;
 	}
@@ -2096,32 +2069,40 @@ error_out:
 	return -EINVAL;
 }
 
-static void udf_error(struct super_block *sb, const char *function,
-		      const char *fmt, ...)
+void udf_err(struct super_block *sb, const char *function,
+	     const char *fmt, ...)
 {
+	struct va_format vaf;
 	va_list args;
 
-	if (!(sb->s_flags & MS_RDONLY)) {
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	if (!(sb->s_flags & MS_RDONLY))
 		/* mark sb error */
 		sb->s_dirt = 1;
-	}
-	va_start(args, fmt);
-	vsnprintf(error_buf, sizeof(error_buf), fmt, args);
+
+	pr_crit("error (device %s): %s: %pV", sb->s_id, function, &vaf);
+
 	va_end(args);
-	printk(KERN_CRIT "UDF-fs error (device %s): %s: %s\n",
-		sb->s_id, function, error_buf);
 }
 
-void udf_warning(struct super_block *sb, const char *function,
-		 const char *fmt, ...)
+void udf_warn(struct super_block *sb, const char *function,
+	      const char *fmt, ...)
 {
+	struct va_format vaf;
 	va_list args;
 
 	va_start(args, fmt);
-	vsnprintf(error_buf, sizeof(error_buf), fmt, args);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	pr_warn("warning (device %s): %s: %pV", sb->s_id, function, &vaf);
+
 	va_end(args);
-	printk(KERN_WARNING "UDF-fs warning (device %s): %s: %s\n",
-	       sb->s_id, function, error_buf);
 }
 
 static void udf_put_super(struct super_block *sb)
@@ -2213,11 +2194,11 @@ static unsigned int udf_count_free_bitmap(struct super_block *sb,
 	bh = udf_read_ptagged(sb, &loc, 0, &ident);
 
 	if (!bh) {
-		printk(KERN_ERR "udf: udf_count_free failed\n");
+		pr_err("udf: udf_count_free failed\n");
 		goto out;
 	} else if (ident != TAG_IDENT_SBD) {
 		brelse(bh);
-		printk(KERN_ERR "udf: udf_count_free failed\n");
+		pr_err("udf: udf_count_free failed\n");
 		goto out;
 	}
 
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index f77c813..f1010b2 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -20,18 +20,26 @@
 
 #undef UDFFS_DEBUG
 
+__attribute__((format(printf, 3, 4)))
+void udf_err(struct super_block *sb, const char *function,
+	     const char *fmt, ...);
+
+__attribute__((format(printf, 3, 4)))
+void udf_warn(struct super_block *sb, const char *function,
+	      const char *fmt, ...);
+
+#define udf_info(fmt, ...)					\
+	pr_info("INFO " fmt, ##__VA_ARGS__)
+
 #ifdef UDFFS_DEBUG
-#define udf_debug(fmt, ...)	\
-	pr_debug("DEBUG %s:%d:%s: " fmt,	\
-		__FILE__, __LINE__, __func__, ##__VA_ARGS__);
+#define udf_debug(fmt, ...)					\
+	pr_debug("DEBUG %s:%d:%s: " fmt,			\
+		 __FILE__, __LINE__, __func__, ##__VA_ARGS__)
 #else
-#define udf_debug(fmt, ...) /**/
+#define udf_debug(fmt, ...)					\
+	no_printk(KERN_DEBUG fmt, ##__VA_ARGS__)
 #endif
 
-#define udf_info(f, a...) \
-	pr_info("INFO " f, ##a);
-
-
 #define udf_fixed_to_variable(x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )
 #define udf_variable_to_fixed(x) ( ( ( (x) / 39 ) << 5 ) + ( (x) % 39 ) )
 
@@ -111,8 +119,6 @@ struct extent_position {
 
 /* super.c */
 
-__attribute__((format(printf, 3, 4)))
-extern void udf_warning(struct super_block *, const char *, const char *, ...);
 static inline void udf_updated_lvid(struct super_block *sb)
 {
 	struct buffer_head *bh = UDF_SB(sb)->s_lvid_bh;
-- 
1.7.6.405.gc1be0




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2 v2] udf : enable error print in udf_read_tagged().
  2011-10-03 22:17     ` Joe Perches
@ 2011-10-03 23:20       ` NamJae Jeon
  0 siblings, 0 replies; 5+ messages in thread
From: NamJae Jeon @ 2011-10-03 23:20 UTC (permalink / raw)
  To: Joe Perches; +Cc: jack, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 43874 bytes --]

2011/10/4 Joe Perches <joe@perches.com>:
> On Mon, 2011-10-03 at 22:58 +0900, NamJae Jeon wrote:
>> 2011/10/3 Joe Perches <joe@perches.com>:
>> > On Mon, 2011-10-03 at 16:19 +0900, Namjae Jeon wrote:
>> >> And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.
>> > []
>> >> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
>> > []
>> >> @@ -19,18 +21,15 @@
>> >>  #undef UDFFS_DEBUG
>> >>
>> >>  #ifdef UDFFS_DEBUG
>> >> -#define udf_debug(f, a...) \
>> >> -do { \
>> >> -     printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \
>> >> -             __FILE__, __LINE__, __func__); \
>> >> -     printk(f, ##a); \
>> >> -} while (0)
>> >> +#define udf_debug(fmt, ...)  \
>> >> +     pr_debug("DEBUG %s:%d:%s: ", fmt,       \
>> >> +             __FILE__, __LINE__, __func__, __VA_ARGS__);
>> >
>> > This doesn't compile properly if UDFFS_DEBUG is
>> > #defined.
>> >
>> > No comma should be between "DEBUG %s:%d:%s: " and fmt.
>> Hi. Joe.
>> oops! it's my mistake.
>> I  sent v3 patch again.
>> Would you review one more ?
>
> It looks OK except for a few issues.
>
> The udf_debug macro doesn't need a semicolon after
> pr_debug(foo); it should be pr_debug(foo)
> Same for udf_info.
>
> The output prefixes are a mixture of "udf: " and "UDF-fs: "
>
> Some lines are printed with 2 newlines.
>
> I've done the patch below on top of your patch.
> It should fix up these trivial things.
>
> Perhaps you'll inspect this too.
Yes, right. It is good point.
I will post new patch included your fixed.
Thanks.
>
> Convert printks to pr_<level>.
> Coalesce formats.
> Rename udf_warning to udf_warn.
> Add __attribute__((format(printf... verification to udf_warn.
> Rename udf_error to udf_err.
> Remove static from udf_err for consistency with other udf_<level> uses.
> Use printf extension "%pV" in udf_warn and udf_err
> Remove now unused static char error_buf[1024].
> Add no_printk to udf_debug when not #define UDF_DEBUG.
> Align multiline arguments to open parenthesis.
> Add newline to message formats where appropriate.
>
> ---
>
>  fs/udf/balloc.c    |    9 +-
>  fs/udf/directory.c |    9 +-
>  fs/udf/inode.c     |    3 +-
>  fs/udf/namei.c     |    6 +-
>  fs/udf/partition.c |   12 ++--
>  fs/udf/super.c     |  213 ++++++++++++++++++++++++----------------------------
>  fs/udf/udfdecl.h   |   26 ++++---
>  7 files changed, 132 insertions(+), 146 deletions(-)
>
> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 95518a9..48bd5c7 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -59,8 +59,8 @@ static int __load_block_bitmap(struct super_block *sb,
>        int nr_groups = bitmap->s_nr_groups;
>
>        if (block_group >= nr_groups) {
> -               udf_debug("block_group (%d) > nr_groups (%d)\n", block_group,
> -                         nr_groups);
> +               udf_debug("block_group (%d) > nr_groups (%d)\n",
> +                         block_group, nr_groups);
>        }
>
>        if (bitmap->s_block_bitmap[block_group]) {
> @@ -155,7 +155,7 @@ static void udf_bitmap_free_blocks(struct super_block *sb,
>                        if (udf_set_bit(bit + i, bh->b_data)) {
>                                udf_debug("bit %ld already set\n", bit + i);
>                                udf_debug("byte=%2x\n",
> -                                       ((char *)bh->b_data)[(bit + i) >> 3]);
> +                                         ((char *)bh->b_data)[(bit + i) >> 3]);
>                        }
>                }
>                udf_add_free_space(sb, sbi->s_partition, count);
> @@ -369,7 +369,8 @@ static void udf_table_free_blocks(struct super_block *sb,
>        if (bloc->logicalBlockNum + count < count ||
>            (bloc->logicalBlockNum + count) > partmap->s_partition_len) {
>                udf_debug("%d < %d || %d + %d > %d\n",
> -                         bloc->logicalBlockNum, 0, bloc->logicalBlockNum, count,
> +                         bloc->logicalBlockNum, 0,
> +                         bloc->logicalBlockNum, count,
>                          partmap->s_partition_len);
>                goto error_return;
>        }
> diff --git a/fs/udf/directory.c b/fs/udf/directory.c
> index a4caf7a..73136e6 100644
> --- a/fs/udf/directory.c
> +++ b/fs/udf/directory.c
> @@ -162,8 +162,8 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)
>        int padlen;
>
>        if ((!buffer) || (!offset)) {
> -               udf_debug("invalidparms\n, buffer=%p, offset=%p\n", buffer,
> -                         offset);
> +               udf_debug("invalidparms, buffer=%p, offset=%p\n",
> +                         buffer, offset);
>                return NULL;
>        }
>
> @@ -175,9 +175,8 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)
>        if (fi->descTag.tagIdent != cpu_to_le16(TAG_IDENT_FID)) {
>                udf_debug("0x%x != TAG_IDENT_FID\n",
>                          le16_to_cpu(fi->descTag.tagIdent));
> -               udf_debug("offset: %u sizeof: %lu bufsize: %u\n",
> -                         *offset, (unsigned long)sizeof(struct fileIdentDesc),
> -                         bufsize);
> +               udf_debug("offset: %u sizeof: %zu bufsize: %u\n",
> +                         *offset, sizeof(struct fileIdentDesc), bufsize);
>                return NULL;
>        }
>        if ((*offset + sizeof(struct fileIdentDesc)) > bufsize)
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 58665ce..6621c2a 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -1982,8 +1982,7 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
>                *elen = le32_to_cpu(lad->extLength) & UDF_EXTENT_LENGTH_MASK;
>                break;
>        default:
> -               udf_debug("alloc_type = %d unsupported\n",
> -                               iinfo->i_alloc_type);
> +               udf_debug("alloc_type = %d unsupported\n", iinfo->i_alloc_type);
>                return -1;
>        }
>
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index f1dce84..bd936c9 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -799,9 +799,9 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)
>        if (retval)
>                goto end_rmdir;
>        if (inode->i_nlink != 2)
> -               udf_warning(inode->i_sb, "udf_rmdir",
> -                           "empty directory has nlink != 2 (%d)",
> -                           inode->i_nlink);
> +               udf_warn(inode->i_sb, "udf_rmdir",
> +                        "empty directory has nlink != 2 (%d)\n",
> +                        inode->i_nlink);
>        clear_nlink(inode);
>        inode->i_size = 0;
>        inode_dec_link_count(dir);
> diff --git a/fs/udf/partition.c b/fs/udf/partition.c
> index a71090e..849039a 100644
> --- a/fs/udf/partition.c
> +++ b/fs/udf/partition.c
> @@ -33,8 +33,8 @@ uint32_t udf_get_pblock(struct super_block *sb, uint32_t block,
>        struct udf_sb_info *sbi = UDF_SB(sb);
>        struct udf_part_map *map;
>        if (partition >= sbi->s_partitions) {
> -               udf_debug("block=%d, partition=%d, offset=%d: "
> -                         "invalid partition\n", block, partition, offset);
> +               udf_debug("block=%d, partition=%d, offset=%d: invalid partition\n",
> +                         block, partition, offset);
>                return 0xFFFFFFFF;
>        }
>        map = &sbi->s_partmaps[partition];
> @@ -60,8 +60,8 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block,
>        vdata = &map->s_type_specific.s_virtual;
>
>        if (block > vdata->s_num_entries) {
> -               udf_debug("Trying to access block beyond end of VAT "
> -                         "(%d max %d)\n", block, vdata->s_num_entries);
> +               udf_debug("Trying to access block beyond end of VAT (%d max %d)\n",
> +                         block, vdata->s_num_entries);
>                return 0xFFFFFFFF;
>        }
>
> @@ -322,8 +322,8 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,
>        BUG_ON(!inode);
>        retblk = udf_try_read_meta(inode, block, partition, offset);
>        if (retblk == 0xFFFFFFFF) {
> -               udf_warning(sb, __func__, "error reading from METADATA, "
> -                       "trying to read from MIRROR");
> +               udf_warn(sb, __func__,
> +                        "error reading from METADATA, trying to read from MIRROR\n");
>                inode = mdata->s_mirror_fe;
>                if (!inode)
>                        return 0xFFFFFFFF;
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index bbf6256..ab921f9 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -75,8 +75,6 @@
>
>  #define UDF_DEFAULT_BLOCKSIZE 2048
>
> -static char error_buf[1024];
> -
>  /* These are the "meat" - everything else is stuffing */
>  static int udf_fill_super(struct super_block *, void *, int);
>  static void udf_put_super(struct super_block *);
> @@ -92,8 +90,6 @@ static void udf_close_lvid(struct super_block *);
>  static unsigned int udf_count_free(struct super_block *);
>  static int udf_statfs(struct dentry *, struct kstatfs *);
>  static int udf_show_options(struct seq_file *, struct vfsmount *);
> -static void udf_error(struct super_block *sb, const char *function,
> -                     const char *fmt, ...);
>
>  struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi)
>  {
> @@ -244,9 +240,9 @@ static int udf_sb_alloc_partition_maps(struct super_block *sb, u32 count)
>        sbi->s_partmaps = kcalloc(count, sizeof(struct udf_part_map),
>                                  GFP_KERNEL);
>        if (!sbi->s_partmaps) {
> -               udf_error(sb, __func__,
> -                         "Unable to allocate space for %d partition maps",
> -                         count);
> +               udf_err(sb, __func__,
> +                       "Unable to allocate space for %d partition maps\n",
> +                       count);
>                sbi->s_partitions = 0;
>                return -ENOMEM;
>        }
> @@ -550,8 +546,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt,
>                        uopt->dmode = option & 0777;
>                        break;
>                default:
> -                       pr_err("bad mount option \"%s\" "
> -                              "or missing value\n", p);
> +                       pr_err("bad mount option \"%s\" or missing value\n", p);
>                        return 0;
>                }
>        }
> @@ -645,20 +640,16 @@ static loff_t udf_check_vsd(struct super_block *sb)
>                                udf_debug("ISO9660 Boot Record found\n");
>                                break;
>                        case 1:
> -                               udf_debug("ISO9660 Primary Volume Descriptor "
> -                                         "found\n");
> +                               udf_debug("ISO9660 Primary Volume Descriptor found\n");
>                                break;
>                        case 2:
> -                               udf_debug("ISO9660 Supplementary Volume "
> -                                         "Descriptor found\n");
> +                               udf_debug("ISO9660 Supplementary Volume Descriptor found\n");
>                                break;
>                        case 3:
> -                               udf_debug("ISO9660 Volume Partition Descriptor "
> -                                         "found\n");
> +                               udf_debug("ISO9660 Volume Partition Descriptor found\n");
>                                break;
>                        case 255:
> -                               udf_debug("ISO9660 Volume Descriptor Set "
> -                                         "Terminator found\n");
> +                               udf_debug("ISO9660 Volume Descriptor Set Terminator found\n");
>                                break;
>                        default:
>                                udf_debug("ISO9660 VRS (%u) found\n",
> @@ -809,8 +800,7 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
>                              pvoldesc->recordingDateAndTime)) {
>  #ifdef UDFFS_DEBUG
>                struct timestamp *ts = &pvoldesc->recordingDateAndTime;
> -               udf_debug("recording time %04u/%02u/%02u"
> -                         " %02u:%02u (%x)\n",
> +               udf_debug("recording time %04u/%02u/%02u %02u:%02u (%x)\n",
>                          le16_to_cpu(ts->year), ts->month, ts->day, ts->hour,
>                          ts->minute, le16_to_cpu(ts->typeAndTimezone));
>  #endif
> @@ -821,7 +811,7 @@ static int udf_load_pvoldesc(struct super_block *sb, sector_t block)
>                        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);
> +                                 UDF_SB(sb)->s_volume_ident);
>                }
>
>        if (!udf_build_ustr(instr, pvoldesc->volSetIdent, 128))
> @@ -853,18 +843,16 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
>        addr.partitionReferenceNum = map->s_partition_num;
>
>        udf_debug("Metadata file location: block = %d part = %d\n",
> -                         addr.logicalBlockNum, addr.partitionReferenceNum);
> +                 addr.logicalBlockNum, addr.partitionReferenceNum);
>
>        mdata->s_metadata_fe = udf_iget(sb, &addr);
>
>        if (mdata->s_metadata_fe == NULL) {
> -               udf_warning(sb, __func__, "metadata inode efe not found, "
> -                               "will try mirror inode.");
> +               udf_warn(sb, __func__, "metadata inode efe not found, will try mirror inode\n");
>                fe_error = 1;
>        } else if (UDF_I(mdata->s_metadata_fe)->i_alloc_type !=
>                 ICBTAG_FLAG_AD_SHORT) {
> -               udf_warning(sb, __func__, "metadata inode efe does not have "
> -                       "short allocation descriptors!");
> +               udf_warn(sb, __func__, "metadata inode efe does not have short allocation descriptors!\n");
>                fe_error = 1;
>                iput(mdata->s_metadata_fe);
>                mdata->s_metadata_fe = NULL;
> @@ -875,22 +863,22 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
>        addr.partitionReferenceNum = map->s_partition_num;
>
>        udf_debug("Mirror metadata file location: block = %d part = %d\n",
> -                         addr.logicalBlockNum, addr.partitionReferenceNum);
> +                 addr.logicalBlockNum, addr.partitionReferenceNum);
>
>        mdata->s_mirror_fe = udf_iget(sb, &addr);
>
>        if (mdata->s_mirror_fe == NULL) {
>                if (fe_error) {
> -                       udf_error(sb, __func__, "mirror inode efe not found "
> -                       "and metadata inode is missing too, exiting...");
> +                       udf_err(sb, __func__,
> +                               "mirror inode efe not found and metadata inode is missing too, exiting...\n");
>                        goto error_exit;
>                } else
> -                       udf_warning(sb, __func__, "mirror inode efe not found,"
> -                                       " but metadata inode is OK");
> +                       udf_warn(sb, __func__,
> +                                "mirror inode efe not found, but metadata inode is OK\n");
>        } else if (UDF_I(mdata->s_mirror_fe)->i_alloc_type !=
>                 ICBTAG_FLAG_AD_SHORT) {
> -               udf_warning(sb, __func__, "mirror inode efe does not have "
> -                       "short allocation descriptors!");
> +               udf_warn(sb, __func__,
> +                        "mirror inode efe does not have short allocation descriptors!\n");
>                iput(mdata->s_mirror_fe);
>                mdata->s_mirror_fe = NULL;
>                if (fe_error)
> @@ -907,18 +895,17 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)
>                addr.partitionReferenceNum = map->s_partition_num;
>
>                udf_debug("Bitmap file location: block = %d part = %d\n",
> -                       addr.logicalBlockNum, addr.partitionReferenceNum);
> +                         addr.logicalBlockNum, addr.partitionReferenceNum);
>
>                mdata->s_bitmap_fe = udf_iget(sb, &addr);
>
>                if (mdata->s_bitmap_fe == NULL) {
>                        if (sb->s_flags & MS_RDONLY)
> -                               udf_warning(sb, __func__, "bitmap inode efe "
> -                                       "not found but it's ok since the disc"
> -                                       " is mounted read-only");
> +                               udf_warn(sb, __func__,
> +                                        "bitmap inode efe not found but it's ok since the disc is mounted read-only\n");
>                        else {
> -                               udf_error(sb, __func__, "bitmap inode efe not "
> -                                       "found and attempted read-write mount");
> +                               udf_err(sb, __func__,
> +                                       "bitmap inode efe not found and attempted read-write mount\n");
>                                goto error_exit;
>                        }
>                }
> @@ -971,9 +958,9 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
>                bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
>
>        if (bitmap == NULL) {
> -               udf_error(sb, __func__,
> -                         "Unable to allocate space for bitmap "
> -                         "and %d buffer_head pointers", nr_groups);
> +               udf_err(sb, __func__,
> +                       "Unable to allocate space for bitmap and %d buffer_head pointers\n",
> +                       nr_groups);
>                return NULL;
>        }
>
> @@ -1003,8 +990,8 @@ static int udf_fill_partdesc_info(struct super_block *sb,
>        if (p->accessType == cpu_to_le32(PD_ACCESS_TYPE_OVERWRITABLE))
>                map->s_partition_flags |= UDF_PART_FLAG_OVERWRITABLE;
>
> -       udf_debug("Partition (%d type %x) starts at physical %d, "
> -                 "block length %d\n", p_index,
> +       udf_debug("Partition (%d type %x) starts at physical %d, block length %d\n",
> +                 p_index,
>                  map->s_partition_type, map->s_partition_root,
>                  map->s_partition_len);
>
> @@ -1023,12 +1010,12 @@ static int udf_fill_partdesc_info(struct super_block *sb,
>                map->s_uspace.s_table = udf_iget(sb, &loc);
>                if (!map->s_uspace.s_table) {
>                        udf_debug("cannot load unallocSpaceTable (part %d)\n",
> -                                       p_index);
> +                                 p_index);
>                        return 1;
>                }
>                map->s_partition_flags |= UDF_PART_FLAG_UNALLOC_TABLE;
>                udf_debug("unallocSpaceTable (part %d) @ %ld\n",
> -                               p_index, map->s_uspace.s_table->i_ino);
> +                         p_index, map->s_uspace.s_table->i_ino);
>        }
>
>        if (phd->unallocSpaceBitmap.extLength) {
> @@ -1041,8 +1028,8 @@ static int udf_fill_partdesc_info(struct super_block *sb,
>                bitmap->s_extPosition = le32_to_cpu(
>                                phd->unallocSpaceBitmap.extPosition);
>                map->s_partition_flags |= UDF_PART_FLAG_UNALLOC_BITMAP;
> -               udf_debug("unallocSpaceBitmap (part %d) @ %d\n", p_index,
> -                                               bitmap->s_extPosition);
> +               udf_debug("unallocSpaceBitmap (part %d) @ %d\n",
> +                         p_index, bitmap->s_extPosition);
>        }
>
>        if (phd->partitionIntegrityTable.extLength)
> @@ -1058,13 +1045,13 @@ static int udf_fill_partdesc_info(struct super_block *sb,
>                map->s_fspace.s_table = udf_iget(sb, &loc);
>                if (!map->s_fspace.s_table) {
>                        udf_debug("cannot load freedSpaceTable (part %d)\n",
> -                               p_index);
> +                                 p_index);
>                        return 1;
>                }
>
>                map->s_partition_flags |= UDF_PART_FLAG_FREED_TABLE;
>                udf_debug("freedSpaceTable (part %d) @ %ld\n",
> -                               p_index, map->s_fspace.s_table->i_ino);
> +                         p_index, map->s_fspace.s_table->i_ino);
>        }
>
>        if (phd->freedSpaceBitmap.extLength) {
> @@ -1077,8 +1064,8 @@ static int udf_fill_partdesc_info(struct super_block *sb,
>                bitmap->s_extPosition = le32_to_cpu(
>                                phd->freedSpaceBitmap.extPosition);
>                map->s_partition_flags |= UDF_PART_FLAG_FREED_BITMAP;
> -               udf_debug("freedSpaceBitmap (part %d) @ %d\n", p_index,
> -                                       bitmap->s_extPosition);
> +               udf_debug("freedSpaceBitmap (part %d) @ %d\n",
> +                         p_index, bitmap->s_extPosition);
>        }
>        return 0;
>  }
> @@ -1118,11 +1105,9 @@ static int udf_load_vat(struct super_block *sb, int p_index, int type1_index)
>        udf_find_vat_block(sb, p_index, type1_index, sbi->s_last_block);
>        if (!sbi->s_vat_inode &&
>            sbi->s_last_block != blocks - 1) {
> -               printk(KERN_NOTICE "UDF-fs: Failed to read VAT inode from the"
> -                      " last recorded block (%lu), retrying with the last "
> -                      "block of the device (%lu).\n",
> -                      (unsigned long)sbi->s_last_block,
> -                      (unsigned long)blocks - 1);
> +               pr_notice("Failed to read VAT inode from the last recorded block (%lu), retrying with the last block of the device (%lu)\n",
> +                         (unsigned long)sbi->s_last_block,
> +                         (unsigned long)blocks - 1);
>                udf_find_vat_block(sb, p_index, type1_index, blocks - 1);
>        }
>        if (!sbi->s_vat_inode)
> @@ -1220,8 +1205,7 @@ static int udf_load_partdesc(struct super_block *sb, sector_t block)
>        if (map->s_partition_type == UDF_METADATA_MAP25) {
>                ret = udf_load_metadata_files(sb, i);
>                if (ret) {
> -                       printk(KERN_ERR "UDF-fs: error loading MetaData "
> -                       "partition map %d\n", i);
> +                       pr_err("error loading MetaData partition map %d\n", i);
>                        goto out_bh;
>                }
>        } else {
> @@ -1234,9 +1218,7 @@ static int udf_load_partdesc(struct super_block *sb, sector_t block)
>                 * overwrite blocks instead of relocating them).
>                 */
>                sb->s_flags |= MS_RDONLY;
> -               printk(KERN_NOTICE "UDF-fs: Filesystem marked read-only "
> -                       "because writing to pseudooverwrite partition is "
> -                       "not implemented.\n");
> +               pr_notice("Filesystem marked read-only because writing to pseudooverwrite partition is not implemented\n");
>        }
>  out_bh:
>        /* In case loading failed, we handle cleanup in udf_fill_super */
> @@ -1344,9 +1326,8 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
>                                struct metadataPartitionMap *mdm =
>                                                (struct metadataPartitionMap *)
>                                                &(lvd->partitionMaps[offset]);
> -                               udf_debug("Parsing Logical vol part %d "
> -                                       "type %d  id=%s\n", i, type,
> -                                       UDF_ID_METADATA);
> +                               udf_debug("Parsing Logical vol part %d type %d  id=%s\n",
> +                                         i, type, UDF_ID_METADATA);
>
>                                map->s_partition_type = UDF_METADATA_MAP25;
>                                map->s_partition_func = udf_get_pblock_meta25;
> @@ -1365,21 +1346,20 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
>                                        mdm->flags & 0x01;
>
>                                udf_debug("Metadata Ident suffix=0x%x\n",
> -                                       (le16_to_cpu(
> -                                        ((__le16 *)
> -                                             mdm->partIdent.identSuffix)[0])));
> +                                         le16_to_cpu(*(__le16 *)
> +                                                      mdm->partIdent.identSuffix));
>                                udf_debug("Metadata part num=%d\n",
> -                                       le16_to_cpu(mdm->partitionNum));
> +                                         le16_to_cpu(mdm->partitionNum));
>                                udf_debug("Metadata part alloc unit size=%d\n",
> -                                       le32_to_cpu(mdm->allocUnitSize));
> +                                         le32_to_cpu(mdm->allocUnitSize));
>                                udf_debug("Metadata file loc=%d\n",
> -                                       le32_to_cpu(mdm->metadataFileLoc));
> +                                         le32_to_cpu(mdm->metadataFileLoc));
>                                udf_debug("Mirror file loc=%d\n",
> -                                      le32_to_cpu(mdm->metadataMirrorFileLoc));
> +                                         le32_to_cpu(mdm->metadataMirrorFileLoc));
>                                udf_debug("Bitmap file loc=%d\n",
> -                                      le32_to_cpu(mdm->metadataBitmapFileLoc));
> +                                         le32_to_cpu(mdm->metadataBitmapFileLoc));
>                                udf_debug("Duplicate Flag: %d %d\n",
> -                                       mdata->s_dup_md_flag, mdm->flags);
> +                                         mdata->s_dup_md_flag, mdm->flags);
>                        } else {
>                                udf_debug("Unknown ident: %s\n",
>                                          upm2->partIdent.ident);
> @@ -1397,8 +1377,8 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
>                struct long_ad *la = (struct long_ad *)&(lvd->logicalVolContentsUse[0]);
>
>                *fileset = lelb_to_cpu(la->extLocation);
> -               udf_debug("FileSet found in LogicalVolDesc at block=%d, "
> -                         "partition=%d\n", fileset->logicalBlockNum,
> +               udf_debug("FileSet found in LogicalVolDesc at block=%d, partition=%d\n",
> +                         fileset->logicalBlockNum,
>                          fileset->partitionReferenceNum);
>        }
>        if (lvd->integritySeqExt.extLength)
> @@ -1478,9 +1458,8 @@ static noinline int udf_process_sequence(struct super_block *sb, long block,
>
>                bh = udf_read_tagged(sb, block, block, &ident);
>                if (!bh) {
> -                       printk(KERN_ERR "udf: Block %Lu of volume descriptor "
> -                              "sequence is corrupted or we could not read "
> -                              "it.\n", (unsigned long long)block);
> +                       pr_err("Block %lu of volume descriptor sequence is corrupted or we could not read it\n",
> +                              (unsigned long)block);
>                        return 1;
>                }
>
> @@ -1553,7 +1532,7 @@ static noinline int udf_process_sequence(struct super_block *sb, long block,
>         * in a suitable order
>         */
>        if (!vds[VDS_POS_PRIMARY_VOL_DESC].block) {
> -               printk(KERN_ERR "udf: Primary Volume Descriptor not found!\n");
> +               pr_err("Primary Volume Descriptor not found!\n");
>                return 1;
>        }
>        if (udf_load_pvoldesc(sb, vds[VDS_POS_PRIMARY_VOL_DESC].block))
> @@ -1740,7 +1719,7 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
>
>        if (!sb_set_blocksize(sb, uopt->blocksize)) {
>                if (!silent)
> -                       printk(KERN_WARNING "UDF-fs: Bad block size\n");
> +                       pr_warn("Bad block size\n");
>                return 0;
>        }
>        sbi->s_last_block = uopt->lastblock;
> @@ -1749,12 +1728,11 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
>                nsr_off = udf_check_vsd(sb);
>                if (!nsr_off) {
>                        if (!silent)
> -                               printk(KERN_WARNING "UDF-fs: No VRS found\n");
> +                               pr_warn("No VRS found\n");
>                        return 0;
>                }
>                if (nsr_off == -1)
> -                       udf_debug("Failed to read byte 32768. Assuming open "
> -                                 "disc. Skipping validity check\n");
> +                       udf_debug("Failed to read byte 32768. Assuming open disc. Skipping validity check\n");
>                if (!sbi->s_last_block)
>                        sbi->s_last_block = udf_get_last_block(sb);
>        } else {
> @@ -1765,7 +1743,7 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
>        sbi->s_anchor = uopt->anchor;
>        if (!udf_find_anchor(sb, fileset)) {
>                if (!silent)
> -                       printk(KERN_WARNING "UDF-fs: No anchor found\n");
> +                       pr_warn("No anchor found\n");
>                return 0;
>        }
>        return 1;
> @@ -1937,8 +1915,8 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
>
>        if (uopt.flags & (1 << UDF_FLAG_UTF8) &&
>            uopt.flags & (1 << UDF_FLAG_NLS_MAP)) {
> -               udf_error(sb, "udf_read_super",
> -                         "utf8 cannot be combined with iocharset\n");
> +               udf_err(sb, "udf_read_super",
> +                       "utf8 cannot be combined with iocharset\n");
>                goto error_out;
>        }
>  #ifdef CONFIG_UDF_NLS
> @@ -1987,15 +1965,14 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
>                ret = udf_load_vrs(sb, &uopt, silent, &fileset);
>                if (!ret && uopt.blocksize != UDF_DEFAULT_BLOCKSIZE) {
>                        if (!silent)
> -                               printk(KERN_NOTICE
> -                                      "UDF-fs: Rescanning with blocksize "
> -                                      "%d\n", UDF_DEFAULT_BLOCKSIZE);
> +                               pr_notice("Rescanning with blocksize %d\n",
> +                                         UDF_DEFAULT_BLOCKSIZE);
>                        uopt.blocksize = UDF_DEFAULT_BLOCKSIZE;
>                        ret = udf_load_vrs(sb, &uopt, silent, &fileset);
>                }
>        }
>        if (!ret) {
> -               printk(KERN_WARNING "UDF-fs: No partition found (1)\n");
> +               pr_warn("No partition found (1)\n");
>                goto error_out;
>        }
>
> @@ -2010,8 +1987,7 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
>                                le16_to_cpu(lvidiu->maxUDFWriteRev); */
>
>                if (minUDFReadRev > UDF_MAX_READ_VERSION) {
> -                       printk(KERN_ERR "UDF-fs: minUDFReadRev=%x "
> -                                       "(max is %x)\n",
> +                       pr_err("minUDFReadRev=%x (max is %x)\n",
>                               le16_to_cpu(lvidiu->minUDFReadRev),
>                               UDF_MAX_READ_VERSION);
>                        goto error_out;
> @@ -2027,27 +2003,25 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
>        }
>
>        if (!sbi->s_partitions) {
> -               printk(KERN_WARNING "UDF-fs: No partition found (2)\n");
> +               pr_warn("No partition found (2)\n");
>                goto error_out;
>        }
>
>        if (sbi->s_partmaps[sbi->s_partition].s_partition_flags &
>                        UDF_PART_FLAG_READ_ONLY) {
> -               printk(KERN_NOTICE "UDF-fs: Partition marked readonly; "
> -                                  "forcing readonly mount\n");
> +               pr_notice("Partition marked readonly; forcing readonly mount\n");
>                sb->s_flags |= MS_RDONLY;
>        }
>
>        if (udf_find_fileset(sb, &fileset, &rootdir)) {
> -               printk(KERN_WARNING "UDF-fs: No fileset found\n");
> +               pr_warn("No fileset found\n");
>                goto error_out;
>        }
>
>        if (!silent) {
>                struct timestamp ts;
>                udf_time_to_disk_stamp(&ts, sbi->s_record_time);
> -               udf_info("UDF: Mounting volume '%s', "
> -                        "timestamp %04u/%02u/%02u %02u:%02u (%x)\n",
> +               udf_info("Mounting volume '%s', timestamp %04u/%02u/%02u %02u:%02u (%x)\n",
>                         sbi->s_volume_ident, le16_to_cpu(ts.year), ts.month, ts.day,
>                         ts.hour, ts.minute, le16_to_cpu(ts.typeAndTimezone));
>        }
> @@ -2059,8 +2033,7 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
>        /* perhaps it's not extensible enough, but for now ... */
>        inode = udf_iget(sb, &rootdir);
>        if (!inode) {
> -               printk(KERN_ERR "UDF-fs: Error in udf_iget, block=%d, "
> -                               "partition=%d\n",
> +               pr_err("Error in udf_iget, block=%d, partition=%d\n",
>                       rootdir.logicalBlockNum, rootdir.partitionReferenceNum);
>                goto error_out;
>        }
> @@ -2068,7 +2041,7 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)
>        /* Allocate a dentry for the root inode */
>        sb->s_root = d_alloc_root(inode);
>        if (!sb->s_root) {
> -               printk(KERN_ERR "UDF-fs: Couldn't allocate root dentry\n");
> +               pr_err("Couldn't allocate root dentry\n");
>                iput(inode);
>                goto error_out;
>        }
> @@ -2096,32 +2069,40 @@ error_out:
>        return -EINVAL;
>  }
>
> -static void udf_error(struct super_block *sb, const char *function,
> -                     const char *fmt, ...)
> +void udf_err(struct super_block *sb, const char *function,
> +            const char *fmt, ...)
>  {
> +       struct va_format vaf;
>        va_list args;
>
> -       if (!(sb->s_flags & MS_RDONLY)) {
> +       va_start(args, fmt);
> +
> +       vaf.fmt = fmt;
> +       vaf.va = &args;
> +
> +       if (!(sb->s_flags & MS_RDONLY))
>                /* mark sb error */
>                sb->s_dirt = 1;
> -       }
> -       va_start(args, fmt);
> -       vsnprintf(error_buf, sizeof(error_buf), fmt, args);
> +
> +       pr_crit("error (device %s): %s: %pV", sb->s_id, function, &vaf);
> +
>        va_end(args);
> -       printk(KERN_CRIT "UDF-fs error (device %s): %s: %s\n",
> -               sb->s_id, function, error_buf);
>  }
>
> -void udf_warning(struct super_block *sb, const char *function,
> -                const char *fmt, ...)
> +void udf_warn(struct super_block *sb, const char *function,
> +             const char *fmt, ...)
>  {
> +       struct va_format vaf;
>        va_list args;
>
>        va_start(args, fmt);
> -       vsnprintf(error_buf, sizeof(error_buf), fmt, args);
> +
> +       vaf.fmt = fmt;
> +       vaf.va = &args;
> +
> +       pr_warn("warning (device %s): %s: %pV", sb->s_id, function, &vaf);
> +
>        va_end(args);
> -       printk(KERN_WARNING "UDF-fs warning (device %s): %s: %s\n",
> -              sb->s_id, function, error_buf);
>  }
>
>  static void udf_put_super(struct super_block *sb)
> @@ -2213,11 +2194,11 @@ static unsigned int udf_count_free_bitmap(struct super_block *sb,
>        bh = udf_read_ptagged(sb, &loc, 0, &ident);
>
>        if (!bh) {
> -               printk(KERN_ERR "udf: udf_count_free failed\n");
> +               pr_err("udf: udf_count_free failed\n");
>                goto out;
>        } else if (ident != TAG_IDENT_SBD) {
>                brelse(bh);
> -               printk(KERN_ERR "udf: udf_count_free failed\n");
> +               pr_err("udf: udf_count_free failed\n");
>                goto out;
>        }
>
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> index f77c813..f1010b2 100644
> --- a/fs/udf/udfdecl.h
> +++ b/fs/udf/udfdecl.h
> @@ -20,18 +20,26 @@
>
>  #undef UDFFS_DEBUG
>
> +__attribute__((format(printf, 3, 4)))
> +void udf_err(struct super_block *sb, const char *function,
> +            const char *fmt, ...);
> +
> +__attribute__((format(printf, 3, 4)))
> +void udf_warn(struct super_block *sb, const char *function,
> +             const char *fmt, ...);
> +
> +#define udf_info(fmt, ...)                                     \
> +       pr_info("INFO " fmt, ##__VA_ARGS__)
> +
>  #ifdef UDFFS_DEBUG
> -#define udf_debug(fmt, ...)    \
> -       pr_debug("DEBUG %s:%d:%s: " fmt,        \
> -               __FILE__, __LINE__, __func__, ##__VA_ARGS__);
> +#define udf_debug(fmt, ...)                                    \
> +       pr_debug("DEBUG %s:%d:%s: " fmt,                        \
> +                __FILE__, __LINE__, __func__, ##__VA_ARGS__)
>  #else
> -#define udf_debug(fmt, ...) /**/
> +#define udf_debug(fmt, ...)                                    \
> +       no_printk(KERN_DEBUG fmt, ##__VA_ARGS__)
>  #endif
>
> -#define udf_info(f, a...) \
> -       pr_info("INFO " f, ##a);
> -
> -
>  #define udf_fixed_to_variable(x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )
>  #define udf_variable_to_fixed(x) ( ( ( (x) / 39 ) << 5 ) + ( (x) % 39 ) )
>
> @@ -111,8 +119,6 @@ struct extent_position {
>
>  /* super.c */
>
> -__attribute__((format(printf, 3, 4)))
> -extern void udf_warning(struct super_block *, const char *, const char *, ...);
>  static inline void udf_updated_lvid(struct super_block *sb)
>  {
>        struct buffer_head *bh = UDF_SB(sb)->s_lvid_bh;
> --
> 1.7.6.405.gc1be0
>
>
>
>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-10-03 23:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-03  7:19 [PATCH 1/2 v2] udf : enable error print in udf_read_tagged() Namjae Jeon
2011-10-03 12:57 ` Joe Perches
2011-10-03 13:58   ` NamJae Jeon
2011-10-03 22:17     ` Joe Perches
2011-10-03 23:20       ` NamJae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox