2010-11-10 23:46:24

by Joe Perches

[permalink] [raw]
Subject: [PATCH 0/6] fs: Add printf format/argument verification

Various logging messages functions use printf style varargs
without declaring them __attribute__((format(printf...))).
Add the attribute so the compiler can warn about mismatches
and then fix any mismatch warnings.

Joe Perches (6):
fs/adfs: Add printf format/argument verification and fix fallout
fs/affs: Add printf format/argument verification and fix fallout
fs/befs: Add printf format/argument verification and fix fallout
fs/ecryptfs: Add printf format/argument verification and fix fallout
fs/jfs: Add printf format/argument verification
fs/udf: Add printf format/argument verification

fs/adfs/adfs.h | 1 +
fs/adfs/dir.c | 2 +-
fs/affs/affs.h | 2 +
fs/affs/file.c | 14 ++++---
fs/befs/befs.h | 3 ++
fs/befs/btree.c | 5 ++-
fs/befs/datastream.c | 76 ++++++++++++++++++++++++-----------------
fs/befs/inode.c | 11 ++++--
fs/befs/io.c | 12 ++++--
fs/befs/linuxvfs.c | 19 ++++++----
fs/ecryptfs/crypto.c | 20 ++++++----
fs/ecryptfs/ecryptfs_kernel.h | 1 +
fs/ecryptfs/file.c | 6 ++--
fs/ecryptfs/keystore.c | 7 ++--
fs/ecryptfs/main.c | 7 ++--
fs/ecryptfs/mmap.c | 13 ++++---
fs/jfs/jfs_superblock.h | 2 +
fs/udf/udfdecl.h | 2 +
18 files changed, 124 insertions(+), 79 deletions(-)

--
1.7.3.1.g432b3.dirty


2010-11-10 23:46:28

by Joe Perches

[permalink] [raw]
Subject: [PATCH 3/6] fs/befs: Add printf format/argument verification and fix fallout

Add __attribute__((format... to befs_error, befs_warning and befs_debug.
Change formats and arguments of u64 to %llu and (unsigned long long).

Signed-off-by: Joe Perches <[email protected]>
---
fs/befs/befs.h | 3 ++
fs/befs/btree.c | 5 ++-
fs/befs/datastream.c | 76 +++++++++++++++++++++++++++++--------------------
fs/befs/inode.c | 11 ++++--
fs/befs/io.c | 12 +++++--
fs/befs/linuxvfs.c | 19 +++++++-----
6 files changed, 77 insertions(+), 49 deletions(-)

diff --git a/fs/befs/befs.h b/fs/befs/befs.h
index d9a40ab..c7619b2 100644
--- a/fs/befs/befs.h
+++ b/fs/befs/befs.h
@@ -88,8 +88,11 @@ enum befs_err {

/****************************/
/* debug.c */
+__attribute__ ((format(printf, 2, 3)))
void befs_error(const struct super_block *sb, const char *fmt, ...);
+__attribute__ ((format(printf, 2, 3)))
void befs_warning(const struct super_block *sb, const char *fmt, ...);
+__attribute__ ((format(printf, 2, 3)))
void befs_debug(const struct super_block *sb, const char *fmt, ...);

void befs_dump_super_block(const struct super_block *sb, befs_super_block *);
diff --git a/fs/befs/btree.c b/fs/befs/btree.c
index 4202db7..95cbe72 100644
--- a/fs/befs/btree.c
+++ b/fs/befs/btree.c
@@ -468,8 +468,9 @@ befs_btree_read(struct super_block *sb, befs_data_stream * ds,
*keysize = 0;
*value = 0;
befs_debug(sb,
- "<--- befs_btree_read() END of keys at %Lu",
- key_sum + this_node->head.all_key_count);
+ "<--- befs_btree_read() END of keys at %llu",
+ (unsigned long long)
+ (key_sum + this_node->head.all_key_count));
brelse(this_node->bh);
kfree(this_node);
return BEFS_BT_END;
diff --git a/fs/befs/datastream.c b/fs/befs/datastream.c
index 59096b5..a0ea94f 100644
--- a/fs/befs/datastream.c
+++ b/fs/befs/datastream.c
@@ -52,26 +52,27 @@ befs_read_datastream(struct super_block *sb, befs_data_stream * ds,
befs_block_run run;
befs_blocknr_t block; /* block coresponding to pos */

- befs_debug(sb, "---> befs_read_datastream() %Lu", pos);
+ befs_debug(sb, "---> befs_read_datastream() %llu",
+ (unsigned long long)pos);
block = pos >> BEFS_SB(sb)->block_shift;
if (off)
*off = pos - (block << BEFS_SB(sb)->block_shift);

if (befs_fblock2brun(sb, ds, block, &run) != BEFS_OK) {
- befs_error(sb, "BeFS: Error finding disk addr of block %lu",
- block);
+ befs_error(sb, "BeFS: Error finding disk addr of block %llu",
+ (unsigned long long)block);
befs_debug(sb, "<--- befs_read_datastream() ERROR");
return NULL;
}
bh = befs_bread_iaddr(sb, run);
if (!bh) {
- befs_error(sb, "BeFS: Error reading block %lu from datastream",
- block);
+ befs_error(sb, "BeFS: Error reading block %llu from datastream",
+ (unsigned long long)block);
return NULL;
}

- befs_debug(sb, "<--- befs_read_datastream() read data, starting at %Lu",
- pos);
+ befs_debug(sb, "<--- befs_read_datastream() read data, starting at %llu",
+ (unsigned long long)pos);

return bh;
}
@@ -105,8 +106,9 @@ befs_fblock2brun(struct super_block *sb, befs_data_stream * data,

} else {
befs_error(sb,
- "befs_fblock2brun() was asked to find block %lu, "
- "which is not mapped by the datastream\n", fblock);
+ "befs_fblock2brun() was asked to find block %llu, "
+ "which is not mapped by the datastream\n",
+ (unsigned long long)fblock);
err = BEFS_ERR;
}
return err;
@@ -128,13 +130,15 @@ befs_read_lsymlink(struct super_block * sb, befs_data_stream * ds, void *buff,
befs_off_t bytes_read = 0; /* bytes readed */
u16 plen;
struct buffer_head *bh = NULL;
- befs_debug(sb, "---> befs_read_lsymlink() length: %Lu", len);
+ befs_debug(sb, "---> befs_read_lsymlink() length: %llu",
+ (unsigned long long)len);

while (bytes_read < len) {
bh = befs_read_datastream(sb, ds, bytes_read, NULL);
if (!bh) {
befs_error(sb, "BeFS: Error reading datastream block "
- "starting from %Lu", bytes_read);
+ "starting from %llu",
+ (unsigned long long)bytes_read);
befs_debug(sb, "<--- befs_read_lsymlink() ERROR");
return bytes_read;

@@ -146,7 +150,8 @@ befs_read_lsymlink(struct super_block * sb, befs_data_stream * ds, void *buff,
bytes_read += plen;
}

- befs_debug(sb, "<--- befs_read_lsymlink() read %u bytes", bytes_read);
+ befs_debug(sb, "<--- befs_read_lsymlink() read %llu bytes",
+ (unsigned long long)bytes_read);
return bytes_read;
}

@@ -206,7 +211,8 @@ befs_count_blocks(struct super_block * sb, befs_data_stream * ds)
}

blocks = datablocks + metablocks;
- befs_debug(sb, "<--- befs_count_blocks() %u blocks", blocks);
+ befs_debug(sb, "<--- befs_count_blocks() %llu blocks",
+ (unsigned long long)blocks);

return blocks;
}
@@ -251,7 +257,8 @@ befs_find_brun_direct(struct super_block *sb, befs_data_stream * data,
befs_blocknr_t max_block =
data->max_direct_range >> BEFS_SB(sb)->block_shift;

- befs_debug(sb, "---> befs_find_brun_direct(), find %lu", blockno);
+ befs_debug(sb, "---> befs_find_brun_direct(), find %llu",
+ (unsigned long long)blockno);

if (blockno > max_block) {
befs_error(sb, "befs_find_brun_direct() passed block outside of"
@@ -268,7 +275,8 @@ befs_find_brun_direct(struct super_block *sb, befs_data_stream * data,
run->len = array[i].len - offset;

befs_debug(sb, "---> befs_find_brun_direct(), "
- "found %lu at direct[%d]", blockno, i);
+ "found %llu at direct[%d]",
+ (unsigned long long)blockno, i);
return BEFS_OK;
}
}
@@ -316,7 +324,8 @@ befs_find_brun_indirect(struct super_block *sb,
befs_blocknr_t indirblockno = iaddr2blockno(sb, &indirect);
int arraylen = befs_iaddrs_per_block(sb);

- befs_debug(sb, "---> befs_find_brun_indirect(), find %lu", blockno);
+ befs_debug(sb, "---> befs_find_brun_indirect(), find %llu",
+ (unsigned long long)blockno);

indir_start_blk = data->max_direct_range >> BEFS_SB(sb)->block_shift;
search_blk = blockno - indir_start_blk;
@@ -327,8 +336,8 @@ befs_find_brun_indirect(struct super_block *sb,
if (indirblock == NULL) {
befs_debug(sb,
"---> befs_find_brun_indirect() failed to "
- "read disk block %lu from the indirect brun",
- indirblockno + i);
+ "read disk block %llu from the indirect brun",
+ (unsigned long long)(indirblockno + i));
return BEFS_ERR;
}

@@ -349,8 +358,9 @@ befs_find_brun_indirect(struct super_block *sb,
brelse(indirblock);
befs_debug(sb,
"<--- befs_find_brun_indirect() found "
- "file block %lu at indirect[%d]",
- blockno, j + (i * arraylen));
+ "file block %llu at indirect[%d]",
+ (unsigned long long)blockno,
+ j + (i * arraylen));
return BEFS_OK;
}
sum += len;
@@ -361,7 +371,7 @@ befs_find_brun_indirect(struct super_block *sb,

/* Only fallthrough is an error */
befs_error(sb, "BeFS: befs_find_brun_indirect() failed to find "
- "file block %lu", blockno);
+ "file block %llu", (unsigned long long)blockno);

befs_debug(sb, "<--- befs_find_brun_indirect() ERROR");
return BEFS_ERR;
@@ -444,7 +454,8 @@ befs_find_brun_dblindirect(struct super_block *sb,
size_t diblklen = iblklen * befs_iaddrs_per_block(sb)
* BEFS_DBLINDIR_BRUN_LEN;

- befs_debug(sb, "---> befs_find_brun_dblindirect() find %lu", blockno);
+ befs_debug(sb, "---> befs_find_brun_dblindirect() find %llu",
+ (unsigned long long)blockno);

/* First, discover which of the double_indir->indir blocks
* contains pos. Then figure out how much of pos that
@@ -470,10 +481,11 @@ befs_find_brun_dblindirect(struct super_block *sb,
dbl_which_block);
if (dbl_indir_block == NULL) {
befs_error(sb, "befs_read_brun_dblindirect() couldn't read the "
- "double-indirect block at blockno %lu",
- iaddr2blockno(sb,
- &data->double_indirect) +
- dbl_which_block);
+ "double-indirect block at blockno %llu",
+ (unsigned long long)
+ (iaddr2blockno(sb,
+ &data->double_indirect) +
+ dbl_which_block));
brelse(dbl_indir_block);
return BEFS_ERR;
}
@@ -498,8 +510,9 @@ befs_find_brun_dblindirect(struct super_block *sb,
befs_bread(sb, iaddr2blockno(sb, &indir_run) + which_block);
if (indir_block == NULL) {
befs_error(sb, "befs_read_brun_dblindirect() couldn't read the "
- "indirect block at blockno %lu",
- iaddr2blockno(sb, &indir_run) + which_block);
+ "indirect block at blockno %llu",
+ (unsigned long long)
+ (iaddr2blockno(sb, &indir_run) + which_block));
brelse(indir_block);
return BEFS_ERR;
}
@@ -518,9 +531,10 @@ befs_find_brun_dblindirect(struct super_block *sb,
run->start += offset;
run->len -= offset;

- befs_debug(sb, "Found file block %lu in double_indirect[%d][%d],"
- " double_indirect_leftover = %lu",
- blockno, dblindir_indx, indir_indx, dblindir_leftover);
+ befs_debug(sb, "Found file block %llu in double_indirect[%d][%d],"
+ " double_indirect_leftover = %llu",
+ (unsigned long long)blockno, dblindir_indx, indir_indx,
+ (unsigned long long)dblindir_leftover);

return BEFS_OK;
}
diff --git a/fs/befs/inode.c b/fs/befs/inode.c
index 94c17f9..4258023 100644
--- a/fs/befs/inode.c
+++ b/fs/befs/inode.c
@@ -25,7 +25,8 @@ befs_check_inode(struct super_block *sb, befs_inode * raw_inode,
/* check magic header. */
if (magic1 != BEFS_INODE_MAGIC1) {
befs_error(sb,
- "Inode has a bad magic header - inode = %lu", inode);
+ "Inode has a bad magic header - inode = %llu",
+ (unsigned long long)inode);
return BEFS_BAD_INODE;
}

@@ -34,8 +35,9 @@ befs_check_inode(struct super_block *sb, befs_inode * raw_inode,
*/
if (inode != iaddr2blockno(sb, &ino_num)) {
befs_error(sb, "inode blocknr field disagrees with vfs "
- "VFS: %lu, Inode %lu",
- inode, iaddr2blockno(sb, &ino_num));
+ "VFS: %llu, Inode %llu",
+ (unsigned long long)inode,
+ (unsigned long long)iaddr2blockno(sb, &ino_num));
return BEFS_BAD_INODE;
}

@@ -44,7 +46,8 @@ befs_check_inode(struct super_block *sb, befs_inode * raw_inode,
*/

if (!(flags & BEFS_INODE_IN_USE)) {
- befs_error(sb, "inode is not used - inode = %lu", inode);
+ befs_error(sb, "inode is not used - inode = %llu",
+ (unsigned long long)inode);
return BEFS_BAD_INODE;
}

diff --git a/fs/befs/io.c b/fs/befs/io.c
index ddef98a..3c16fcb 100644
--- a/fs/befs/io.c
+++ b/fs/befs/io.c
@@ -42,12 +42,14 @@ befs_bread_iaddr(struct super_block *sb, befs_inode_addr iaddr)

block = iaddr2blockno(sb, &iaddr);

- befs_debug(sb, "befs_read_iaddr: offset = %lu", block);
+ befs_debug(sb, "befs_read_iaddr: offset = %llu",
+ (unsigned long long)block);

bh = sb_bread(sb, block);

if (bh == NULL) {
- befs_error(sb, "Failed to read block %lu", block);
+ befs_error(sb, "Failed to read block %llu",
+ (unsigned long long)block);
goto error;
}

@@ -64,12 +66,14 @@ befs_bread(struct super_block *sb, befs_blocknr_t block)
{
struct buffer_head *bh = NULL;

- befs_debug(sb, "---> Enter befs_read() %Lu", block);
+ befs_debug(sb, "---> Enter befs_read() %llu",
+ (unsigned long long)block);

bh = sb_bread(sb, block);

if (bh == NULL) {
- befs_error(sb, "Failed to read block %lu", block);
+ befs_error(sb, "Failed to read block %llu",
+ (unsigned long long)block);
goto error;
}

diff --git a/fs/befs/linuxvfs.c b/fs/befs/linuxvfs.c
index aa4e7c7..ee265eb 100644
--- a/fs/befs/linuxvfs.c
+++ b/fs/befs/linuxvfs.c
@@ -125,19 +125,20 @@ befs_get_block(struct inode *inode, sector_t block,
int res = 0;
ulong disk_off;

- befs_debug(sb, "---> befs_get_block() for inode %lu, block %ld",
- inode->i_ino, block);
+ befs_debug(sb, "---> befs_get_block() for inode %lu, block %llu",
+ inode->i_ino, (unsigned long long)block);

if (block < 0) {
befs_error(sb, "befs_get_block() was asked for a block "
- "number less than zero: block %ld in inode %lu",
- block, inode->i_ino);
+ "number less than zero: block %llu in inode %lu",
+ (unsigned long long)block, inode->i_ino);
return -EIO;
}

if (create) {
befs_error(sb, "befs_get_block() was asked to write to "
- "block %ld in inode %lu", block, inode->i_ino);
+ "block %llu in inode %lu",
+ (unsigned long long)block, inode->i_ino);
return -EPERM;
}

@@ -145,7 +146,8 @@ befs_get_block(struct inode *inode, sector_t block,
if (res != BEFS_OK) {
befs_error(sb,
"<--- befs_get_block() for inode %lu, block "
- "%ld ERROR", inode->i_ino, block);
+ "%llu ERROR", inode->i_ino,
+ (unsigned long long)block);
return -EFBIG;
}

@@ -153,8 +155,9 @@ befs_get_block(struct inode *inode, sector_t block,

map_bh(bh_result, inode->i_sb, disk_off);

- befs_debug(sb, "<--- befs_get_block() for inode %lu, block %ld, "
- "disk address %lu", inode->i_ino, block, disk_off);
+ befs_debug(sb, "<--- befs_get_block() for inode %lu, block %llu, "
+ "disk address %lu", inode->i_ino, (unsigned long long)block,
+ disk_off);

return 0;
}
--
1.7.3.1.g432b3.dirty

2010-11-10 23:46:33

by Joe Perches

[permalink] [raw]
Subject: [PATCH 4/6] fs/ecryptfs: Add printf format/argument verification and fix fallout

Add __attribute__((format... to __ecryptfs_printk
Make formats and arguments match.
Add casts to (unsigned long long) for %llu.

Signed-off-by: Joe Perches <[email protected]>
---
fs/ecryptfs/crypto.c | 20 ++++++++++++--------
fs/ecryptfs/ecryptfs_kernel.h | 1 +
fs/ecryptfs/file.c | 6 +++---
fs/ecryptfs/keystore.c | 7 ++++---
fs/ecryptfs/main.c | 7 ++++---
fs/ecryptfs/mmap.c | 13 +++++++------
6 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index cbadc1b..523bcb0 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -414,8 +414,9 @@ static int ecryptfs_encrypt_extent(struct page *enc_extent_page,
(extent_base + extent_offset));
if (rc) {
ecryptfs_printk(KERN_ERR, "Error attempting to "
- "derive IV for extent [0x%.16x]; "
- "rc = [%d]\n", (extent_base + extent_offset),
+ "derive IV for extent [0x%.16llx]; "
+ "rc = [%d]\n",
+ (unsigned long long)(extent_base + extent_offset),
rc);
goto out;
}
@@ -443,8 +444,9 @@ static int ecryptfs_encrypt_extent(struct page *enc_extent_page,
}
rc = 0;
if (unlikely(ecryptfs_verbosity > 0)) {
- ecryptfs_printk(KERN_DEBUG, "Encrypt extent [0x%.16x]; "
- "rc = [%d]\n", (extent_base + extent_offset),
+ ecryptfs_printk(KERN_DEBUG, "Encrypt extent [0x%.16llx]; "
+ "rc = [%d]\n",
+ (unsigned long long)(extent_base + extent_offset),
rc);
ecryptfs_printk(KERN_DEBUG, "First 8 bytes after "
"encryption:\n");
@@ -541,8 +543,9 @@ static int ecryptfs_decrypt_extent(struct page *page,
(extent_base + extent_offset));
if (rc) {
ecryptfs_printk(KERN_ERR, "Error attempting to "
- "derive IV for extent [0x%.16x]; "
- "rc = [%d]\n", (extent_base + extent_offset),
+ "derive IV for extent [0x%.16llx]; "
+ "rc = [%d]\n",
+ (unsigned long long)(extent_base + extent_offset),
rc);
goto out;
}
@@ -571,8 +574,9 @@ static int ecryptfs_decrypt_extent(struct page *page,
}
rc = 0;
if (unlikely(ecryptfs_verbosity > 0)) {
- ecryptfs_printk(KERN_DEBUG, "Decrypt extent [0x%.16x]; "
- "rc = [%d]\n", (extent_base + extent_offset),
+ ecryptfs_printk(KERN_DEBUG, "Decrypt extent [0x%.16llx]; "
+ "rc = [%d]\n",
+ (unsigned long long)(extent_base + extent_offset),
rc);
ecryptfs_printk(KERN_DEBUG, "First 8 bytes after "
"decryption:\n");
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 413a3c4..86c7101 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -584,6 +584,7 @@ ecryptfs_set_dentry_lower_mnt(struct dentry *dentry, struct vfsmount *lower_mnt)

#define ecryptfs_printk(type, fmt, arg...) \
__ecryptfs_printk(type "%s: " fmt, __func__, ## arg);
+__attribute__ ((format(printf, 1, 2)))
void __ecryptfs_printk(const char *fmt, ...);

extern const struct file_operations ecryptfs_main_fops;
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 91da029..d445672 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -243,9 +243,9 @@ static int ecryptfs_open(struct inode *inode, struct file *file)
}
}
mutex_unlock(&crypt_stat->cs_mutex);
- ecryptfs_printk(KERN_DEBUG, "inode w/ addr = [0x%p], i_ino = [0x%.16x] "
- "size: [0x%.16x]\n", inode, inode->i_ino,
- i_size_read(inode));
+ ecryptfs_printk(KERN_DEBUG, "inode w/ addr = [0x%p], i_ino = [0x%.16lx] "
+ "size: [0x%.16llx]\n", inode, inode->i_ino,
+ (unsigned long long)i_size_read(inode));
goto out;
out_free:
kmem_cache_free(ecryptfs_file_info_cache,
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index b1f6858..e73f24c 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -59,7 +59,7 @@ static int process_request_key_err(long err_code)
break;
default:
ecryptfs_printk(KERN_WARNING, "Unknown error code: "
- "[0x%.16x]\n", err_code);
+ "[0x%.16lx]\n", err_code);
rc = -EINVAL;
}
return rc;
@@ -1864,8 +1864,9 @@ found_matching_auth_tok:
"session key for authentication token with sig "
"[%.*s]; rc = [%d]. Removing auth tok "
"candidate from the list and searching for "
- "the next match.\n", candidate_auth_tok_sig,
- ECRYPTFS_SIG_SIZE_HEX, rc);
+ "the next myouatch.\n",
+ ECRYPTFS_SIG_SIZE_HEX, candidate_auth_tok_sig,
+ rc);
list_for_each_entry_safe(auth_tok_list_item,
auth_tok_list_item_tmp,
&auth_tok_list, list) {
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index a9dbd62..cfe636d 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -828,9 +828,10 @@ static int __init ecryptfs_init(void)
ecryptfs_printk(KERN_ERR, "The eCryptfs extent size is "
"larger than the host's page size, and so "
"eCryptfs cannot run on this system. The "
- "default eCryptfs extent size is [%d] bytes; "
- "the page size is [%d] bytes.\n",
- ECRYPTFS_DEFAULT_EXTENT_SIZE, PAGE_CACHE_SIZE);
+ "default eCryptfs extent size is [%u] bytes; "
+ "the page size is [%lu] bytes.\n",
+ ECRYPTFS_DEFAULT_EXTENT_SIZE,
+ (unsigned long)PAGE_CACHE_SIZE);
goto out;
}
rc = ecryptfs_init_kmem_caches();
diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
index b1d8275..667fc79 100644
--- a/fs/ecryptfs/mmap.c
+++ b/fs/ecryptfs/mmap.c
@@ -65,7 +65,7 @@ static int ecryptfs_writepage(struct page *page, struct writeback_control *wbc)
rc = ecryptfs_encrypt_page(page);
if (rc) {
ecryptfs_printk(KERN_WARNING, "Error encrypting "
- "page (upper index [0x%.16x])\n", page->index);
+ "page (upper index [0x%.16lx])\n", page->index);
ClearPageUptodate(page);
goto out;
}
@@ -237,7 +237,7 @@ out:
ClearPageUptodate(page);
else
SetPageUptodate(page);
- ecryptfs_printk(KERN_DEBUG, "Unlocking page with index = [0x%.16x]\n",
+ ecryptfs_printk(KERN_DEBUG, "Unlocking page with index = [0x%.16lx]\n",
page->index);
unlock_page(page);
return rc;
@@ -488,7 +488,7 @@ static int ecryptfs_write_end(struct file *file,
} else
ecryptfs_printk(KERN_DEBUG, "Not a new file\n");
ecryptfs_printk(KERN_DEBUG, "Calling fill_zeros_to_end_of_page"
- "(page w/ index = [0x%.16x], to = [%d])\n", index, to);
+ "(page w/ index = [0x%.16lx], to = [%d])\n", index, to);
if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
rc = ecryptfs_write_lower_page_segment(ecryptfs_inode, page, 0,
to);
@@ -503,19 +503,20 @@ static int ecryptfs_write_end(struct file *file,
rc = fill_zeros_to_end_of_page(page, to);
if (rc) {
ecryptfs_printk(KERN_WARNING, "Error attempting to fill "
- "zeros in page with index = [0x%.16x]\n", index);
+ "zeros in page with index = [0x%.16lx]\n", index);
goto out;
}
rc = ecryptfs_encrypt_page(page);
if (rc) {
ecryptfs_printk(KERN_WARNING, "Error encrypting page (upper "
- "index [0x%.16x])\n", index);
+ "index [0x%.16lx])\n", index);
goto out;
}
if (pos + copied > i_size_read(ecryptfs_inode)) {
i_size_write(ecryptfs_inode, pos + copied);
ecryptfs_printk(KERN_DEBUG, "Expanded file size to "
- "[0x%.16x]\n", i_size_read(ecryptfs_inode));
+ "[0x%.16llx]\n",
+ (unsigned long long)i_size_read(ecryptfs_inode));
}
rc = ecryptfs_write_inode_size_to_metadata(ecryptfs_inode);
if (rc)
--
1.7.3.1.g432b3.dirty

2010-11-10 23:46:31

by Joe Perches

[permalink] [raw]
Subject: [PATCH 5/6] fs/jfs: Add printf format/argument verification

Add __attribute__((format... to jfs_error.

All arguments matched formats, no other changes necessary.

Signed-off-by: Joe Perches <[email protected]>
---
fs/jfs/jfs_superblock.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/jfs/jfs_superblock.h b/fs/jfs/jfs_superblock.h
index 884fc21..1da2538 100644
--- a/fs/jfs/jfs_superblock.h
+++ b/fs/jfs/jfs_superblock.h
@@ -108,6 +108,8 @@ struct jfs_superblock {

extern int readSuper(struct super_block *, struct buffer_head **);
extern int updateSuper(struct super_block *, uint);
+
+__attribute__((format(printf, 2, 3)))
extern void jfs_error(struct super_block *, const char *, ...);
extern int jfs_mount(struct super_block *);
extern int jfs_mount_rw(struct super_block *, int);
--
1.7.3.1.g432b3.dirty

2010-11-10 23:46:26

by Joe Perches

[permalink] [raw]
Subject: [PATCH 2/6] fs/affs: Add printf format/argument verification and fix fallout

Add __attribute__((format... to affs_error and affs_warning.
Change formats to match types of arguments.

Signed-off-by: Joe Perches <[email protected]>
---
fs/affs/affs.h | 2 ++
fs/affs/file.c | 14 ++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/affs/affs.h b/fs/affs/affs.h
index a8cbdeb..c3f746b 100644
--- a/fs/affs/affs.h
+++ b/fs/affs/affs.h
@@ -138,7 +138,9 @@ extern void affs_fix_checksum(struct super_block *sb, struct buffer_head *bh);
extern void secs_to_datestamp(time_t secs, struct affs_date *ds);
extern mode_t prot_to_mode(u32 prot);
extern void mode_to_prot(struct inode *inode);
+__attribute__ ((format(printf, 3, 4)))
extern void affs_error(struct super_block *sb, const char *function, const char *fmt, ...);
+__attribute__ ((format(printf, 3, 4)))
extern void affs_warning(struct super_block *sb, const char *function, const char *fmt, ...);
extern int affs_check_name(const unsigned char *name, int len);
extern int affs_copy_name(unsigned char *bstr, struct dentry *dentry);
diff --git a/fs/affs/file.c b/fs/affs/file.c
index 0a90dcd..4c82c40 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -355,7 +355,8 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul

/* store new block */
if (bh_result->b_blocknr)
- affs_warning(sb, "get_block", "block already set (%x)", bh_result->b_blocknr);
+ affs_warning(sb, "get_block", "block already set (%lx)",
+ (unsigned long)bh_result->b_blocknr);
AFFS_BLOCK(sb, ext_bh, block) = cpu_to_be32(blocknr);
AFFS_HEAD(ext_bh)->block_count = cpu_to_be32(block + 1);
affs_adjust_checksum(ext_bh, blocknr - bh_result->b_blocknr + 1);
@@ -377,7 +378,8 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
return 0;

err_big:
- affs_error(inode->i_sb,"get_block","strange block request %d", block);
+ affs_error(inode->i_sb,"get_block","strange block request %d",
+ (int)block);
return -EIO;
err_ext:
// unlock cache
@@ -848,8 +850,8 @@ affs_truncate(struct inode *inode)
// lock cache
ext_bh = affs_get_extblock(inode, ext);
if (IS_ERR(ext_bh)) {
- affs_warning(sb, "truncate", "unexpected read error for ext block %u (%d)",
- ext, PTR_ERR(ext_bh));
+ affs_warning(sb, "truncate", "unexpected read error for ext block %u (%ld)",
+ (unsigned int)ext, PTR_ERR(ext_bh));
return;
}
if (AFFS_I(inode)->i_lc) {
@@ -895,8 +897,8 @@ affs_truncate(struct inode *inode)
struct buffer_head *bh = affs_bread_ino(inode, last_blk, 0);
u32 tmp;
if (IS_ERR(bh)) {
- affs_warning(sb, "truncate", "unexpected read error for last block %u (%d)",
- ext, PTR_ERR(bh));
+ affs_warning(sb, "truncate", "unexpected read error for last block %u (%ld)",
+ (unsigned int)ext, PTR_ERR(bh));
return;
}
tmp = be32_to_cpu(AFFS_DATA_HEAD(bh)->next);
--
1.7.3.1.g432b3.dirty

2010-11-10 23:46:30

by Joe Perches

[permalink] [raw]
Subject: [PATCH 6/6] fs/udf: Add printf format/argument verification

Add __attribute__((format... to udf_warning.

All arguments matched formats, no other changes necessary.

Signed-off-by: Joe Perches <[email protected]>
---
fs/udf/udfdecl.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 6995ab1..74d58c0 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -111,6 +111,8 @@ 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)
{
--
1.7.3.1.g432b3.dirty

2010-11-10 23:47:36

by Joe Perches

[permalink] [raw]
Subject: [PATCH 1/6] fs/adfs: Add printf format/argument verification and fix fallout

Add __attribute__ ((format(printf, 3, 4))) to __adfs_error prototype
Change format string to match argument in adfs_dir_lookup_byname

Signed-off-by: Joe Perches <[email protected]>
---
fs/adfs/adfs.h | 1 +
fs/adfs/dir.c | 2 +-
2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/adfs/adfs.h b/fs/adfs/adfs.h
index 2ff622f..fa078f1 100644
--- a/fs/adfs/adfs.h
+++ b/fs/adfs/adfs.h
@@ -129,6 +129,7 @@ extern int adfs_map_lookup(struct super_block *sb, unsigned int frag_id, unsigne
extern unsigned int adfs_map_free(struct super_block *sb);

/* Misc */
+__attribute__ ((format(printf, 3, 4)))
void __adfs_error(struct super_block *sb, const char *function,
const char *fmt, ...);
#define adfs_error(sb, fmt...) __adfs_error(sb, __func__, fmt)
diff --git a/fs/adfs/dir.c b/fs/adfs/dir.c
index f4287e4..6b74978 100644
--- a/fs/adfs/dir.c
+++ b/fs/adfs/dir.c
@@ -148,7 +148,7 @@ adfs_dir_lookup_byname(struct inode *inode, struct qstr *name, struct object_inf
goto out;

if (ADFS_I(inode)->parent_id != dir.parent_id) {
- adfs_error(sb, "parent directory changed under me! (%lx but got %lx)\n",
+ adfs_error(sb, "parent directory changed under me! (%lx but got %x)\n",
ADFS_I(inode)->parent_id, dir.parent_id);
ret = -EIO;
goto free_out;
--
1.7.3.1.g432b3.dirty

2010-11-11 09:20:37

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/6] fs/affs: Add printf format/argument verification and fix fallout

On Thu, Nov 11, 2010 at 00:46, Joe Perches <[email protected]> wrote:
> Add __attribute__((format... to affs_error and affs_warning.

Thanks, nice catch!

> Change formats to match types of arguments.

You didn't fix any formats, you just added casts to silence the warnings?

> --- a/fs/affs/file.c
> +++ b/fs/affs/file.c
> @@ -355,7 +355,8 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
>
>                /* store new block */
>                if (bh_result->b_blocknr)
> -                       affs_warning(sb, "get_block", "block already set (%x)", bh_result->b_blocknr);
> +                       affs_warning(sb, "get_block", "block already set (%lx)",
> +                                    (unsigned long)bh_result->b_blocknr);

struct buffer_head.b_blocknr is sector_t, which can be either u64 or
unsigned long.
So casting it to unsigned long may truncate it.
Please cast to unsigned long long instead, and use %llx.

>                AFFS_BLOCK(sb, ext_bh, block) = cpu_to_be32(blocknr);
>                AFFS_HEAD(ext_bh)->block_count = cpu_to_be32(block + 1);
>                affs_adjust_checksum(ext_bh, blocknr - bh_result->b_blocknr + 1);
> @@ -377,7 +378,8 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
>        return 0;
>
>  err_big:
> -       affs_error(inode->i_sb,"get_block","strange block request %d", block);
> +       affs_error(inode->i_sb,"get_block","strange block request %d",
> +                  (int)block);

Same here, block is sector_t.

>        return -EIO;
>  err_ext:
>        // unlock cache
> @@ -848,8 +850,8 @@ affs_truncate(struct inode *inode)
>        // lock cache
>        ext_bh = affs_get_extblock(inode, ext);
>        if (IS_ERR(ext_bh)) {
> -               affs_warning(sb, "truncate", "unexpected read error for ext block %u (%d)",
> -                            ext, PTR_ERR(ext_bh));
> +               affs_warning(sb, "truncate", "unexpected read error for ext block %u (%ld)",
> +                            (unsigned int)ext, PTR_ERR(ext_bh));

Do you really need the cast here? ext is u32, which is unsigned int.

>                return;
>        }
>        if (AFFS_I(inode)->i_lc) {
> @@ -895,8 +897,8 @@ affs_truncate(struct inode *inode)
>                        struct buffer_head *bh = affs_bread_ino(inode, last_blk, 0);
>                        u32 tmp;
>                        if (IS_ERR(bh)) {
> -                               affs_warning(sb, "truncate", "unexpected read error for last block %u (%d)",
> -                                            ext, PTR_ERR(bh));
> +                               affs_warning(sb, "truncate", "unexpected read error for last block %u (%ld)",
> +                                            (unsigned int)ext, PTR_ERR(bh));

ext is u32.

>                                return;
>                        }
>                        tmp = be32_to_cpu(AFFS_DATA_HEAD(bh)->next);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2010-11-11 09:55:55

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 6/6] fs/udf: Add printf format/argument verification

On Wed 10-11-10 15:46:18, Joe Perches wrote:
> Add __attribute__((format... to udf_warning.
>
> All arguments matched formats, no other changes necessary.
Thanks. Merged into my tree.

Honza
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> fs/udf/udfdecl.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> index 6995ab1..74d58c0 100644
> --- a/fs/udf/udfdecl.h
> +++ b/fs/udf/udfdecl.h
> @@ -111,6 +111,8 @@ 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)
> {
> --
> 1.7.3.1.g432b3.dirty
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2010-11-11 10:14:15

by Joe Perches

[permalink] [raw]
Subject: [PATCH] fs/affs/file.c: Use appropriate format types and casts

On Thu, 2010-11-11 at 10:20 +0100, Geert Uytterhoeven wrote:
> On Thu, Nov 11, 2010 at 00:46, Joe Perches <[email protected]> wrote:
> > Add __attribute__((format... to affs_error and affs_warning.
> You didn't fix any formats, you just added casts to silence the warnings?

I made them match the pr_debug uses.

Here's a patch to make the formats match the types
and use %llu and (unsigned long long) casts where
appropriate for the pr_debug as well as the affs_
uses.

Signed-off-by: Joe Perches <[email protected]>
---
fs/affs/file.c | 66 +++++++++++++++++++++++++++++++++++++------------------
1 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/fs/affs/file.c b/fs/affs/file.c
index 4c82c40..81f7805 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -46,7 +46,7 @@ const struct inode_operations affs_file_inode_operations = {
static int
affs_file_open(struct inode *inode, struct file *filp)
{
- pr_debug("AFFS: open(%lu,%d)\n",
+ pr_debug("AFFS: open(%lu, %d)\n",
inode->i_ino, atomic_read(&AFFS_I(inode)->i_opencnt));
atomic_inc(&AFFS_I(inode)->i_opencnt);
return 0;
@@ -166,7 +166,8 @@ affs_alloc_extblock(struct inode *inode, struct buffer_head *bh, u32 ext)

tmp = be32_to_cpu(AFFS_TAIL(sb, bh)->extension);
if (tmp)
- affs_warning(sb, "alloc_ext", "previous extension set (%x)", tmp);
+ affs_warning(sb, "alloc_ext", "previous extension set (%x)",
+ tmp);
AFFS_TAIL(sb, bh)->extension = cpu_to_be32(blocknr);
affs_adjust_checksum(bh, blocknr - tmp);
mark_buffer_dirty_inode(bh, inode);
@@ -325,7 +326,8 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
struct buffer_head *ext_bh;
u32 ext;

- pr_debug("AFFS: get_block(%u, %lu)\n", (u32)inode->i_ino, (unsigned long)block);
+ pr_debug("AFFS: get_block(%lu, %llu)\n",
+ inode->i_ino, (unsigned long long)block);

BUG_ON(block > (sector_t)0x7fffffffUL);

@@ -355,8 +357,9 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul

/* store new block */
if (bh_result->b_blocknr)
- affs_warning(sb, "get_block", "block already set (%lx)",
- (unsigned long)bh_result->b_blocknr);
+ affs_warning(sb, "get_block",
+ "block already set (%llx)",
+ (unsigned long long)bh_result->b_blocknr);
AFFS_BLOCK(sb, ext_bh, block) = cpu_to_be32(blocknr);
AFFS_HEAD(ext_bh)->block_count = cpu_to_be32(block + 1);
affs_adjust_checksum(ext_bh, blocknr - bh_result->b_blocknr + 1);
@@ -366,7 +369,9 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
/* insert first block into header block */
u32 tmp = be32_to_cpu(AFFS_HEAD(ext_bh)->first_data);
if (tmp)
- affs_warning(sb, "get_block", "first block already set (%d)", tmp);
+ affs_warning(sb, "get_block",
+ "first block already set (%u)",
+ tmp);
AFFS_HEAD(ext_bh)->first_data = cpu_to_be32(blocknr);
affs_adjust_checksum(ext_bh, blocknr - tmp);
}
@@ -378,8 +383,8 @@ affs_get_block(struct inode *inode, sector_t block, struct buffer_head *bh_resul
return 0;

err_big:
- affs_error(inode->i_sb,"get_block","strange block request %d",
- (int)block);
+ affs_error(inode->i_sb, "get_block", "strange block request %llu",
+ (unsigned long long)block);
return -EIO;
err_ext:
// unlock cache
@@ -504,7 +509,8 @@ affs_do_readpage_ofs(struct file *file, struct page *page, unsigned from, unsign
u32 bidx, boff, bsize;
u32 tmp;

- pr_debug("AFFS: read_page(%u, %ld, %d, %d)\n", (u32)inode->i_ino, page->index, from, to);
+ pr_debug("AFFS: read_page(%lu, %ld, %d, %d)\n",
+ inode->i_ino, page->index, from, to);
BUG_ON(from > to || to > PAGE_CACHE_SIZE);
kmap(page);
data = page_address(page);
@@ -539,7 +545,7 @@ affs_extent_file_ofs(struct inode *inode, u32 newsize)
u32 size, bsize;
u32 tmp;

- pr_debug("AFFS: extent_file(%u, %d)\n", (u32)inode->i_ino, newsize);
+ pr_debug("AFFS: extent_file(%lu, %d)\n", inode->i_ino, newsize);
bsize = AFFS_SB(sb)->s_data_blksize;
bh = NULL;
size = AFFS_I(inode)->mmu_private;
@@ -580,7 +586,9 @@ affs_extent_file_ofs(struct inode *inode, u32 newsize)
if (prev_bh) {
u32 tmp = be32_to_cpu(AFFS_DATA_HEAD(prev_bh)->next);
if (tmp)
- affs_warning(sb, "extent_file_ofs", "next block already set for %d (%d)", bidx, tmp);
+ affs_warning(sb, "extent_file_ofs",
+ "next block already set for %u (%u)",
+ bidx, tmp);
AFFS_DATA_HEAD(prev_bh)->next = cpu_to_be32(bh->b_blocknr);
affs_adjust_checksum(prev_bh, bh->b_blocknr - tmp);
mark_buffer_dirty_inode(prev_bh, inode);
@@ -605,7 +613,7 @@ affs_readpage_ofs(struct file *file, struct page *page)
u32 to;
int err;

- pr_debug("AFFS: read_page(%u, %ld)\n", (u32)inode->i_ino, page->index);
+ pr_debug("AFFS: read_page(%lu, %ld)\n", inode->i_ino, page->index);
to = PAGE_CACHE_SIZE;
if (((page->index + 1) << PAGE_CACHE_SHIFT) > inode->i_size) {
to = inode->i_size & ~PAGE_CACHE_MASK;
@@ -628,7 +636,10 @@ static int affs_write_begin_ofs(struct file *file, struct address_space *mapping
pgoff_t index;
int err = 0;

- pr_debug("AFFS: write_begin(%u, %llu, %llu)\n", (u32)inode->i_ino, (unsigned long long)pos, (unsigned long long)pos + len);
+ pr_debug("AFFS: write_begin(%lu, %llu, %llu)\n",
+ inode->i_ino,
+ (unsigned long long)pos,
+ (unsigned long long)(pos + len));
if (pos > AFFS_I(inode)->mmu_private) {
/* XXX: this probably leaves a too-big i_size in case of
* failure. Should really be updating i_size at write_end time
@@ -677,7 +688,10 @@ static int affs_write_end_ofs(struct file *file, struct address_space *mapping,
* due to write_begin.
*/

- pr_debug("AFFS: write_begin(%u, %llu, %llu)\n", (u32)inode->i_ino, (unsigned long long)pos, (unsigned long long)pos + len);
+ pr_debug("AFFS: write_begin(%lu, %llu, %llu)\n",
+ inode->i_ino,
+ (unsigned long long)pos,
+ (unsigned long long)(pos + len));
bsize = AFFS_SB(sb)->s_data_blksize;
data = page_address(page);

@@ -720,7 +734,9 @@ static int affs_write_end_ofs(struct file *file, struct address_space *mapping,
if (prev_bh) {
u32 tmp = be32_to_cpu(AFFS_DATA_HEAD(prev_bh)->next);
if (tmp)
- affs_warning(sb, "commit_write_ofs", "next block already set for %d (%d)", bidx, tmp);
+ affs_warning(sb, "commit_write_ofs",
+ "next block already set for %u (%u)",
+ bidx, tmp);
AFFS_DATA_HEAD(prev_bh)->next = cpu_to_be32(bh->b_blocknr);
affs_adjust_checksum(prev_bh, bh->b_blocknr - tmp);
mark_buffer_dirty_inode(prev_bh, inode);
@@ -751,7 +767,9 @@ static int affs_write_end_ofs(struct file *file, struct address_space *mapping,
if (prev_bh) {
u32 tmp = be32_to_cpu(AFFS_DATA_HEAD(prev_bh)->next);
if (tmp)
- affs_warning(sb, "commit_write_ofs", "next block already set for %d (%d)", bidx, tmp);
+ affs_warning(sb, "commit_write_ofs",
+ "next block already set for %u (%u)",
+ bidx, tmp);
AFFS_DATA_HEAD(prev_bh)->next = cpu_to_be32(bh->b_blocknr);
affs_adjust_checksum(prev_bh, bh->b_blocknr - tmp);
mark_buffer_dirty_inode(prev_bh, inode);
@@ -820,8 +838,10 @@ affs_truncate(struct inode *inode)
struct buffer_head *ext_bh;
int i;

- pr_debug("AFFS: truncate(inode=%d, oldsize=%u, newsize=%u)\n",
- (u32)inode->i_ino, (u32)AFFS_I(inode)->mmu_private, (u32)inode->i_size);
+ pr_debug("AFFS: truncate(inode=%lu, oldsize=%llu, newsize=%llu)\n",
+ inode->i_ino,
+ (unsigned long long)AFFS_I(inode)->mmu_private,
+ (unsigned long long)inode->i_size);

last_blk = 0;
ext = 0;
@@ -850,8 +870,9 @@ affs_truncate(struct inode *inode)
// lock cache
ext_bh = affs_get_extblock(inode, ext);
if (IS_ERR(ext_bh)) {
- affs_warning(sb, "truncate", "unexpected read error for ext block %u (%ld)",
- (unsigned int)ext, PTR_ERR(ext_bh));
+ affs_warning(sb, "truncate",
+ "unexpected read error for ext block %u (%ld)",
+ ext, PTR_ERR(ext_bh));
return;
}
if (AFFS_I(inode)->i_lc) {
@@ -897,8 +918,9 @@ affs_truncate(struct inode *inode)
struct buffer_head *bh = affs_bread_ino(inode, last_blk, 0);
u32 tmp;
if (IS_ERR(bh)) {
- affs_warning(sb, "truncate", "unexpected read error for last block %u (%ld)",
- (unsigned int)ext, PTR_ERR(bh));
+ affs_warning(sb, "truncate",
+ "unexpected read error for last block %u (%ld)",
+ ext, PTR_ERR(bh));
return;
}
tmp = be32_to_cpu(AFFS_DATA_HEAD(bh)->next);

2010-11-11 15:31:45

by Dave Kleikamp

[permalink] [raw]
Subject: Re: [Jfs-discussion] [PATCH 5/6] fs/jfs: Add printf format/argument verification

On Wed, 2010-11-10 at 15:46 -0800, Joe Perches wrote:
> Add __attribute__((format... to jfs_error.
>
> All arguments matched formats, no other changes necessary.
>
> Signed-off-by: Joe Perches <[email protected]>

Acked-by: Dave Kleikamp <[email protected]>

Would you like to push this patchset as a group, or would you like me to
push this patch through the jfs tree?

Shaggy

> ---
> fs/jfs/jfs_superblock.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/jfs/jfs_superblock.h b/fs/jfs/jfs_superblock.h
> index 884fc21..1da2538 100644
> --- a/fs/jfs/jfs_superblock.h
> +++ b/fs/jfs/jfs_superblock.h
> @@ -108,6 +108,8 @@ struct jfs_superblock {
>
> extern int readSuper(struct super_block *, struct buffer_head **);
> extern int updateSuper(struct super_block *, uint);
> +
> +__attribute__((format(printf, 2, 3)))
> extern void jfs_error(struct super_block *, const char *, ...);
> extern int jfs_mount(struct super_block *);
> extern int jfs_mount_rw(struct super_block *, int);

--
Dave Kleikamp
IBM Linux Technology Center

2010-11-12 08:53:50

by Joe Perches

[permalink] [raw]
Subject: Re: [Jfs-discussion] [PATCH 5/6] fs/jfs: Add printf format/argument verification

On Thu, 2010-11-11 at 09:31 -0600, Dave Kleikamp wrote:
> On Wed, 2010-11-10 at 15:46 -0800, Joe Perches wrote:
> > Add __attribute__((format... to jfs_error.
> > All arguments matched formats, no other changes necessary.
> > Signed-off-by: Joe Perches <[email protected]>
> Acked-by: Dave Kleikamp <[email protected]>
> Would you like to push this patchset as a group, or would you like me to
> push this patch through the jfs tree?

I think each patch should go through the
appropriate fs maintainers separately.

cheers, Joe

2010-11-16 03:28:48

by Tyler Hicks

[permalink] [raw]
Subject: Re: [PATCH 4/6] fs/ecryptfs: Add printf format/argument verification and fix fallout

On Wed Nov 10, 2010 at 03:46:16PM -0800, Joe Perches <[email protected]> wrote:
> Add __attribute__((format... to __ecryptfs_printk
> Make formats and arguments match.
> Add casts to (unsigned long long) for %llu.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---

Thanks Joe. I changed a couple minor things, which are noted below and
applied the fix to
git://git.kernel.org/pub/scm/linux/kernel/git/ecryptfs/ecryptfs-2.6.git#next

When compiling the eCryptfs module for an x86_64 kernel, this fix
uncovered several missing 'z' length modifiers for size_t variables. I
fixed those, as well:
http://git.kernel.org/?p=linux/kernel/git/ecryptfs/ecryptfs-2.6.git;a=commit;h=d1fbc23049e13980f22897b0e657345a9c7bcd50

Thanks again for the patch.

Tyler

> fs/ecryptfs/crypto.c | 20 ++++++++++++--------
> fs/ecryptfs/ecryptfs_kernel.h | 1 +
> fs/ecryptfs/file.c | 6 +++---
> fs/ecryptfs/keystore.c | 7 ++++---
> fs/ecryptfs/main.c | 7 ++++---
> fs/ecryptfs/mmap.c | 13 +++++++------
> 6 files changed, 31 insertions(+), 23 deletions(-)
>
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index cbadc1b..523bcb0 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -414,8 +414,9 @@ static int ecryptfs_encrypt_extent(struct page *enc_extent_page,
> (extent_base + extent_offset));
> if (rc) {
> ecryptfs_printk(KERN_ERR, "Error attempting to "
> - "derive IV for extent [0x%.16x]; "
> - "rc = [%d]\n", (extent_base + extent_offset),
> + "derive IV for extent [0x%.16llx]; "
> + "rc = [%d]\n",
> + (unsigned long long)(extent_base + extent_offset),
> rc);
> goto out;
> }
> @@ -443,8 +444,9 @@ static int ecryptfs_encrypt_extent(struct page *enc_extent_page,
> }
> rc = 0;
> if (unlikely(ecryptfs_verbosity > 0)) {
> - ecryptfs_printk(KERN_DEBUG, "Encrypt extent [0x%.16x]; "
> - "rc = [%d]\n", (extent_base + extent_offset),
> + ecryptfs_printk(KERN_DEBUG, "Encrypt extent [0x%.16llx]; "
> + "rc = [%d]\n",
> + (unsigned long long)(extent_base + extent_offset),

A handful of these lines reached over 80 columns, so I touched them up
before pushing the patch to my branch.

> rc);
> ecryptfs_printk(KERN_DEBUG, "First 8 bytes after "
> "encryption:\n");
> @@ -541,8 +543,9 @@ static int ecryptfs_decrypt_extent(struct page *page,
> (extent_base + extent_offset));
> if (rc) {
> ecryptfs_printk(KERN_ERR, "Error attempting to "
> - "derive IV for extent [0x%.16x]; "
> - "rc = [%d]\n", (extent_base + extent_offset),
> + "derive IV for extent [0x%.16llx]; "
> + "rc = [%d]\n",
> + (unsigned long long)(extent_base + extent_offset),
> rc);
> goto out;
> }
> @@ -571,8 +574,9 @@ static int ecryptfs_decrypt_extent(struct page *page,
> }
> rc = 0;
> if (unlikely(ecryptfs_verbosity > 0)) {
> - ecryptfs_printk(KERN_DEBUG, "Decrypt extent [0x%.16x]; "
> - "rc = [%d]\n", (extent_base + extent_offset),
> + ecryptfs_printk(KERN_DEBUG, "Decrypt extent [0x%.16llx]; "
> + "rc = [%d]\n",
> + (unsigned long long)(extent_base + extent_offset),
> rc);
> ecryptfs_printk(KERN_DEBUG, "First 8 bytes after "
> "decryption:\n");
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index 413a3c4..86c7101 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -584,6 +584,7 @@ ecryptfs_set_dentry_lower_mnt(struct dentry *dentry, struct vfsmount *lower_mnt)
>
> #define ecryptfs_printk(type, fmt, arg...) \
> __ecryptfs_printk(type "%s: " fmt, __func__, ## arg);
> +__attribute__ ((format(printf, 1, 2)))
> void __ecryptfs_printk(const char *fmt, ...);
>
> extern const struct file_operations ecryptfs_main_fops;
> diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
> index 91da029..d445672 100644
> --- a/fs/ecryptfs/file.c
> +++ b/fs/ecryptfs/file.c
> @@ -243,9 +243,9 @@ static int ecryptfs_open(struct inode *inode, struct file *file)
> }
> }
> mutex_unlock(&crypt_stat->cs_mutex);
> - ecryptfs_printk(KERN_DEBUG, "inode w/ addr = [0x%p], i_ino = [0x%.16x] "
> - "size: [0x%.16x]\n", inode, inode->i_ino,
> - i_size_read(inode));
> + ecryptfs_printk(KERN_DEBUG, "inode w/ addr = [0x%p], i_ino = [0x%.16lx] "
> + "size: [0x%.16llx]\n", inode, inode->i_ino,
> + (unsigned long long)i_size_read(inode));
> goto out;
> out_free:
> kmem_cache_free(ecryptfs_file_info_cache,
> diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
> index b1f6858..e73f24c 100644
> --- a/fs/ecryptfs/keystore.c
> +++ b/fs/ecryptfs/keystore.c
> @@ -59,7 +59,7 @@ static int process_request_key_err(long err_code)
> break;
> default:
> ecryptfs_printk(KERN_WARNING, "Unknown error code: "
> - "[0x%.16x]\n", err_code);
> + "[0x%.16lx]\n", err_code);
> rc = -EINVAL;
> }
> return rc;
> @@ -1864,8 +1864,9 @@ found_matching_auth_tok:
> "session key for authentication token with sig "
> "[%.*s]; rc = [%d]. Removing auth tok "
> "candidate from the list and searching for "
> - "the next match.\n", candidate_auth_tok_sig,
> - ECRYPTFS_SIG_SIZE_HEX, rc);
> + "the next myouatch.\n",

I also touched up this little typo.

> + ECRYPTFS_SIG_SIZE_HEX, candidate_auth_tok_sig,
> + rc);
> list_for_each_entry_safe(auth_tok_list_item,
> auth_tok_list_item_tmp,
> &auth_tok_list, list) {
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index a9dbd62..cfe636d 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -828,9 +828,10 @@ static int __init ecryptfs_init(void)
> ecryptfs_printk(KERN_ERR, "The eCryptfs extent size is "
> "larger than the host's page size, and so "
> "eCryptfs cannot run on this system. The "
> - "default eCryptfs extent size is [%d] bytes; "
> - "the page size is [%d] bytes.\n",
> - ECRYPTFS_DEFAULT_EXTENT_SIZE, PAGE_CACHE_SIZE);
> + "default eCryptfs extent size is [%u] bytes; "
> + "the page size is [%lu] bytes.\n",
> + ECRYPTFS_DEFAULT_EXTENT_SIZE,
> + (unsigned long)PAGE_CACHE_SIZE);
> goto out;
> }
> rc = ecryptfs_init_kmem_caches();
> diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c
> index b1d8275..667fc79 100644
> --- a/fs/ecryptfs/mmap.c
> +++ b/fs/ecryptfs/mmap.c
> @@ -65,7 +65,7 @@ static int ecryptfs_writepage(struct page *page, struct writeback_control *wbc)
> rc = ecryptfs_encrypt_page(page);
> if (rc) {
> ecryptfs_printk(KERN_WARNING, "Error encrypting "
> - "page (upper index [0x%.16x])\n", page->index);
> + "page (upper index [0x%.16lx])\n", page->index);
> ClearPageUptodate(page);
> goto out;
> }
> @@ -237,7 +237,7 @@ out:
> ClearPageUptodate(page);
> else
> SetPageUptodate(page);
> - ecryptfs_printk(KERN_DEBUG, "Unlocking page with index = [0x%.16x]\n",
> + ecryptfs_printk(KERN_DEBUG, "Unlocking page with index = [0x%.16lx]\n",
> page->index);
> unlock_page(page);
> return rc;
> @@ -488,7 +488,7 @@ static int ecryptfs_write_end(struct file *file,
> } else
> ecryptfs_printk(KERN_DEBUG, "Not a new file\n");
> ecryptfs_printk(KERN_DEBUG, "Calling fill_zeros_to_end_of_page"
> - "(page w/ index = [0x%.16x], to = [%d])\n", index, to);
> + "(page w/ index = [0x%.16lx], to = [%d])\n", index, to);
> if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
> rc = ecryptfs_write_lower_page_segment(ecryptfs_inode, page, 0,
> to);
> @@ -503,19 +503,20 @@ static int ecryptfs_write_end(struct file *file,
> rc = fill_zeros_to_end_of_page(page, to);
> if (rc) {
> ecryptfs_printk(KERN_WARNING, "Error attempting to fill "
> - "zeros in page with index = [0x%.16x]\n", index);
> + "zeros in page with index = [0x%.16lx]\n", index);
> goto out;
> }
> rc = ecryptfs_encrypt_page(page);
> if (rc) {
> ecryptfs_printk(KERN_WARNING, "Error encrypting page (upper "
> - "index [0x%.16x])\n", index);
> + "index [0x%.16lx])\n", index);
> goto out;
> }
> if (pos + copied > i_size_read(ecryptfs_inode)) {
> i_size_write(ecryptfs_inode, pos + copied);
> ecryptfs_printk(KERN_DEBUG, "Expanded file size to "
> - "[0x%.16x]\n", i_size_read(ecryptfs_inode));
> + "[0x%.16llx]\n",
> + (unsigned long long)i_size_read(ecryptfs_inode));
> }
> rc = ecryptfs_write_inode_size_to_metadata(ecryptfs_inode);
> if (rc)
> --
> 1.7.3.1.g432b3.dirty
>