Signed-off-by: Mathieu Segaud <[email protected]>
---
fs/ext3/acl.c | 194 ++++++++++++++++++++++++++++----------------------------
fs/ext3/acl.h | 6 +-
fs/ext4/acl.c | 190 ++++++++++++++++++++++++++++----------------------------
fs/ext4/acl.h | 6 +-
4 files changed, 198 insertions(+), 198 deletions(-)
diff --git a/fs/ext3/acl.c b/fs/ext3/acl.c
index d34e996..8de0050 100644
--- a/fs/ext3/acl.c
+++ b/fs/ext3/acl.c
@@ -40,34 +40,34 @@ ext3_acl_from_disk(const void *value, size_t size)
acl = posix_acl_alloc(count, GFP_KERNEL);
if (!acl)
return ERR_PTR(-ENOMEM);
- for (n=0; n < count; n++) {
+ for (n = 0; n < count; n++) {
ext3_acl_entry *entry =
(ext3_acl_entry *)value;
if ((char *)value + sizeof(ext3_acl_entry_short) > end)
goto fail;
acl->a_entries[n].e_tag = le16_to_cpu(entry->e_tag);
acl->a_entries[n].e_perm = le16_to_cpu(entry->e_perm);
- switch(acl->a_entries[n].e_tag) {
- case ACL_USER_OBJ:
- case ACL_GROUP_OBJ:
- case ACL_MASK:
- case ACL_OTHER:
- value = (char *)value +
- sizeof(ext3_acl_entry_short);
- acl->a_entries[n].e_id = ACL_UNDEFINED_ID;
- break;
-
- case ACL_USER:
- case ACL_GROUP:
- value = (char *)value + sizeof(ext3_acl_entry);
- if ((char *)value > end)
- goto fail;
- acl->a_entries[n].e_id =
- le32_to_cpu(entry->e_id);
- break;
-
- default:
+ switch (acl->a_entries[n].e_tag) {
+ case ACL_USER_OBJ:
+ case ACL_GROUP_OBJ:
+ case ACL_MASK:
+ case ACL_OTHER:
+ value = (char *)value +
+ sizeof(ext3_acl_entry_short);
+ acl->a_entries[n].e_id = ACL_UNDEFINED_ID;
+ break;
+
+ case ACL_USER:
+ case ACL_GROUP:
+ value = (char *)value + sizeof(ext3_acl_entry);
+ if ((char *)value > end)
goto fail;
+ acl->a_entries[n].e_id =
+ le32_to_cpu(entry->e_id);
+ break;
+
+ default:
+ goto fail;
}
}
if (value != end)
@@ -96,27 +96,27 @@ ext3_acl_to_disk(const struct posix_acl *acl, size_t *size)
return ERR_PTR(-ENOMEM);
ext_acl->a_version = cpu_to_le32(EXT3_ACL_VERSION);
e = (char *)ext_acl + sizeof(ext3_acl_header);
- for (n=0; n < acl->a_count; n++) {
+ for (n = 0; n < acl->a_count; n++) {
ext3_acl_entry *entry = (ext3_acl_entry *)e;
entry->e_tag = cpu_to_le16(acl->a_entries[n].e_tag);
entry->e_perm = cpu_to_le16(acl->a_entries[n].e_perm);
- switch(acl->a_entries[n].e_tag) {
- case ACL_USER:
- case ACL_GROUP:
- entry->e_id =
- cpu_to_le32(acl->a_entries[n].e_id);
- e += sizeof(ext3_acl_entry);
- break;
-
- case ACL_USER_OBJ:
- case ACL_GROUP_OBJ:
- case ACL_MASK:
- case ACL_OTHER:
- e += sizeof(ext3_acl_entry_short);
- break;
-
- default:
- goto fail;
+ switch (acl->a_entries[n].e_tag) {
+ case ACL_USER:
+ case ACL_GROUP:
+ entry->e_id =
+ cpu_to_le32(acl->a_entries[n].e_id);
+ e += sizeof(ext3_acl_entry);
+ break;
+
+ case ACL_USER_OBJ:
+ case ACL_GROUP_OBJ:
+ case ACL_MASK:
+ case ACL_OTHER:
+ e += sizeof(ext3_acl_entry_short);
+ break;
+
+ default:
+ goto fail;
}
}
return (char *)ext_acl;
@@ -141,7 +141,7 @@ ext3_iget_acl(struct inode *inode, struct posix_acl **i_acl)
static inline void
ext3_iset_acl(struct inode *inode, struct posix_acl **i_acl,
- struct posix_acl *acl)
+ struct posix_acl *acl)
{
spin_lock(&inode->i_lock);
if (*i_acl != EXT3_ACL_NOT_CACHED)
@@ -167,23 +167,23 @@ ext3_get_acl(struct inode *inode, int type)
if (!test_opt(inode->i_sb, POSIX_ACL))
return NULL;
- switch(type) {
- case ACL_TYPE_ACCESS:
- acl = ext3_iget_acl(inode, &ei->i_acl);
- if (acl != EXT3_ACL_NOT_CACHED)
- return acl;
- name_index = EXT3_XATTR_INDEX_POSIX_ACL_ACCESS;
- break;
-
- case ACL_TYPE_DEFAULT:
- acl = ext3_iget_acl(inode, &ei->i_default_acl);
- if (acl != EXT3_ACL_NOT_CACHED)
- return acl;
- name_index = EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT;
- break;
-
- default:
- return ERR_PTR(-EINVAL);
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ acl = ext3_iget_acl(inode, &ei->i_acl);
+ if (acl != EXT3_ACL_NOT_CACHED)
+ return acl;
+ name_index = EXT3_XATTR_INDEX_POSIX_ACL_ACCESS;
+ break;
+
+ case ACL_TYPE_DEFAULT:
+ acl = ext3_iget_acl(inode, &ei->i_default_acl);
+ if (acl != EXT3_ACL_NOT_CACHED)
+ return acl;
+ name_index = EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT;
+ break;
+
+ default:
+ return ERR_PTR(-EINVAL);
}
retval = ext3_xattr_get(inode, name_index, "", NULL, 0);
if (retval > 0) {
@@ -201,14 +201,14 @@ ext3_get_acl(struct inode *inode, int type)
kfree(value);
if (!IS_ERR(acl)) {
- switch(type) {
- case ACL_TYPE_ACCESS:
- ext3_iset_acl(inode, &ei->i_acl, acl);
- break;
-
- case ACL_TYPE_DEFAULT:
- ext3_iset_acl(inode, &ei->i_default_acl, acl);
- break;
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ ext3_iset_acl(inode, &ei->i_acl, acl);
+ break;
+
+ case ACL_TYPE_DEFAULT:
+ ext3_iset_acl(inode, &ei->i_default_acl, acl);
+ break;
}
}
return acl;
@@ -232,31 +232,31 @@ ext3_set_acl(handle_t *handle, struct inode *inode, int type,
if (S_ISLNK(inode->i_mode))
return -EOPNOTSUPP;
- switch(type) {
- case ACL_TYPE_ACCESS:
- name_index = EXT3_XATTR_INDEX_POSIX_ACL_ACCESS;
- if (acl) {
- mode_t mode = inode->i_mode;
- error = posix_acl_equiv_mode(acl, &mode);
- if (error < 0)
- return error;
- else {
- inode->i_mode = mode;
- ext3_mark_inode_dirty(handle, inode);
- if (error == 0)
- acl = NULL;
- }
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ name_index = EXT3_XATTR_INDEX_POSIX_ACL_ACCESS;
+ if (acl) {
+ mode_t mode = inode->i_mode;
+ error = posix_acl_equiv_mode(acl, &mode);
+ if (error < 0)
+ return error;
+ else {
+ inode->i_mode = mode;
+ ext3_mark_inode_dirty(handle, inode);
+ if (error == 0)
+ acl = NULL;
}
- break;
+ }
+ break;
- case ACL_TYPE_DEFAULT:
- name_index = EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT;
- if (!S_ISDIR(inode->i_mode))
- return acl ? -EACCES : 0;
- break;
+ case ACL_TYPE_DEFAULT:
+ name_index = EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT;
+ if (!S_ISDIR(inode->i_mode))
+ return acl ? -EACCES : 0;
+ break;
- default:
- return -EINVAL;
+ default:
+ return -EINVAL;
}
if (acl) {
value = ext3_acl_to_disk(acl, &size);
@@ -269,14 +269,14 @@ ext3_set_acl(handle_t *handle, struct inode *inode, int type,
kfree(value);
if (!error) {
- switch(type) {
- case ACL_TYPE_ACCESS:
- ext3_iset_acl(inode, &ei->i_acl, acl);
- break;
-
- case ACL_TYPE_DEFAULT:
- ext3_iset_acl(inode, &ei->i_default_acl, acl);
- break;
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ ext3_iset_acl(inode, &ei->i_acl, acl);
+ break;
+
+ case ACL_TYPE_DEFAULT:
+ ext3_iset_acl(inode, &ei->i_default_acl, acl);
+ break;
}
}
return error;
@@ -375,7 +375,7 @@ int
ext3_acl_chmod(struct inode *inode)
{
struct posix_acl *acl, *clone;
- int error;
+ int error;
if (S_ISLNK(inode->i_mode))
return -EOPNOTSUPP;
@@ -393,7 +393,7 @@ ext3_acl_chmod(struct inode *inode)
handle_t *handle;
int retries = 0;
- retry:
+retry:
handle = ext3_journal_start(inode,
EXT3_DATA_TRANS_BLOCKS(inode->i_sb));
if (IS_ERR(handle)) {
diff --git a/fs/ext3/acl.h b/fs/ext3/acl.h
index 0d1e627..ee85815 100644
--- a/fs/ext3/acl.h
+++ b/fs/ext3/acl.h
@@ -58,9 +58,9 @@ static inline int ext3_acl_count(size_t size)
#define EXT3_ACL_NOT_CACHED ((void *)-1)
/* acl.c */
-extern int ext3_permission (struct inode *, int, struct nameidata *);
-extern int ext3_acl_chmod (struct inode *);
-extern int ext3_init_acl (handle_t *, struct inode *, struct inode *);
+extern int ext3_permission(struct inode *, int, struct nameidata *);
+extern int ext3_acl_chmod(struct inode *);
+extern int ext3_init_acl(handle_t *, struct inode *, struct inode *);
#else /* CONFIG_EXT3_FS_POSIX_ACL */
#include <linux/sched.h>
diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index a8bae8c..f9f8d1e 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -40,34 +40,34 @@ ext4_acl_from_disk(const void *value, size_t size)
acl = posix_acl_alloc(count, GFP_KERNEL);
if (!acl)
return ERR_PTR(-ENOMEM);
- for (n=0; n < count; n++) {
+ for (n = 0; n < count; n++) {
ext4_acl_entry *entry =
(ext4_acl_entry *)value;
if ((char *)value + sizeof(ext4_acl_entry_short) > end)
goto fail;
acl->a_entries[n].e_tag = le16_to_cpu(entry->e_tag);
acl->a_entries[n].e_perm = le16_to_cpu(entry->e_perm);
- switch(acl->a_entries[n].e_tag) {
- case ACL_USER_OBJ:
- case ACL_GROUP_OBJ:
- case ACL_MASK:
- case ACL_OTHER:
- value = (char *)value +
- sizeof(ext4_acl_entry_short);
- acl->a_entries[n].e_id = ACL_UNDEFINED_ID;
- break;
-
- case ACL_USER:
- case ACL_GROUP:
- value = (char *)value + sizeof(ext4_acl_entry);
- if ((char *)value > end)
- goto fail;
- acl->a_entries[n].e_id =
- le32_to_cpu(entry->e_id);
- break;
-
- default:
+ switch (acl->a_entries[n].e_tag) {
+ case ACL_USER_OBJ:
+ case ACL_GROUP_OBJ:
+ case ACL_MASK:
+ case ACL_OTHER:
+ value = (char *)value +
+ sizeof(ext4_acl_entry_short);
+ acl->a_entries[n].e_id = ACL_UNDEFINED_ID;
+ break;
+
+ case ACL_USER:
+ case ACL_GROUP:
+ value = (char *)value + sizeof(ext4_acl_entry);
+ if ((char *)value > end)
goto fail;
+ acl->a_entries[n].e_id =
+ le32_to_cpu(entry->e_id);
+ break;
+
+ default:
+ goto fail;
}
}
if (value != end)
@@ -96,27 +96,27 @@ ext4_acl_to_disk(const struct posix_acl *acl, size_t *size)
return ERR_PTR(-ENOMEM);
ext_acl->a_version = cpu_to_le32(EXT4_ACL_VERSION);
e = (char *)ext_acl + sizeof(ext4_acl_header);
- for (n=0; n < acl->a_count; n++) {
+ for (n = 0; n < acl->a_count; n++) {
ext4_acl_entry *entry = (ext4_acl_entry *)e;
entry->e_tag = cpu_to_le16(acl->a_entries[n].e_tag);
entry->e_perm = cpu_to_le16(acl->a_entries[n].e_perm);
- switch(acl->a_entries[n].e_tag) {
- case ACL_USER:
- case ACL_GROUP:
- entry->e_id =
- cpu_to_le32(acl->a_entries[n].e_id);
- e += sizeof(ext4_acl_entry);
- break;
-
- case ACL_USER_OBJ:
- case ACL_GROUP_OBJ:
- case ACL_MASK:
- case ACL_OTHER:
- e += sizeof(ext4_acl_entry_short);
- break;
-
- default:
- goto fail;
+ switch (acl->a_entries[n].e_tag) {
+ case ACL_USER:
+ case ACL_GROUP:
+ entry->e_id =
+ cpu_to_le32(acl->a_entries[n].e_id);
+ e += sizeof(ext4_acl_entry);
+ break;
+
+ case ACL_USER_OBJ:
+ case ACL_GROUP_OBJ:
+ case ACL_MASK:
+ case ACL_OTHER:
+ e += sizeof(ext4_acl_entry_short);
+ break;
+
+ default:
+ goto fail;
}
}
return (char *)ext_acl;
@@ -167,23 +167,23 @@ ext4_get_acl(struct inode *inode, int type)
if (!test_opt(inode->i_sb, POSIX_ACL))
return NULL;
- switch(type) {
- case ACL_TYPE_ACCESS:
- acl = ext4_iget_acl(inode, &ei->i_acl);
- if (acl != EXT4_ACL_NOT_CACHED)
- return acl;
- name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS;
- break;
-
- case ACL_TYPE_DEFAULT:
- acl = ext4_iget_acl(inode, &ei->i_default_acl);
- if (acl != EXT4_ACL_NOT_CACHED)
- return acl;
- name_index = EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT;
- break;
-
- default:
- return ERR_PTR(-EINVAL);
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ acl = ext4_iget_acl(inode, &ei->i_acl);
+ if (acl != EXT4_ACL_NOT_CACHED)
+ return acl;
+ name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS;
+ break;
+
+ case ACL_TYPE_DEFAULT:
+ acl = ext4_iget_acl(inode, &ei->i_default_acl);
+ if (acl != EXT4_ACL_NOT_CACHED)
+ return acl;
+ name_index = EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT;
+ break;
+
+ default:
+ return ERR_PTR(-EINVAL);
}
retval = ext4_xattr_get(inode, name_index, "", NULL, 0);
if (retval > 0) {
@@ -201,14 +201,14 @@ ext4_get_acl(struct inode *inode, int type)
kfree(value);
if (!IS_ERR(acl)) {
- switch(type) {
- case ACL_TYPE_ACCESS:
- ext4_iset_acl(inode, &ei->i_acl, acl);
- break;
-
- case ACL_TYPE_DEFAULT:
- ext4_iset_acl(inode, &ei->i_default_acl, acl);
- break;
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ ext4_iset_acl(inode, &ei->i_acl, acl);
+ break;
+
+ case ACL_TYPE_DEFAULT:
+ ext4_iset_acl(inode, &ei->i_default_acl, acl);
+ break;
}
}
return acl;
@@ -232,31 +232,31 @@ ext4_set_acl(handle_t *handle, struct inode *inode, int type,
if (S_ISLNK(inode->i_mode))
return -EOPNOTSUPP;
- switch(type) {
- case ACL_TYPE_ACCESS:
- name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS;
- if (acl) {
- mode_t mode = inode->i_mode;
- error = posix_acl_equiv_mode(acl, &mode);
- if (error < 0)
- return error;
- else {
- inode->i_mode = mode;
- ext4_mark_inode_dirty(handle, inode);
- if (error == 0)
- acl = NULL;
- }
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ name_index = EXT4_XATTR_INDEX_POSIX_ACL_ACCESS;
+ if (acl) {
+ mode_t mode = inode->i_mode;
+ error = posix_acl_equiv_mode(acl, &mode);
+ if (error < 0)
+ return error;
+ else {
+ inode->i_mode = mode;
+ ext4_mark_inode_dirty(handle, inode);
+ if (error == 0)
+ acl = NULL;
}
- break;
+ }
+ break;
- case ACL_TYPE_DEFAULT:
- name_index = EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT;
- if (!S_ISDIR(inode->i_mode))
- return acl ? -EACCES : 0;
- break;
+ case ACL_TYPE_DEFAULT:
+ name_index = EXT4_XATTR_INDEX_POSIX_ACL_DEFAULT;
+ if (!S_ISDIR(inode->i_mode))
+ return acl ? -EACCES : 0;
+ break;
- default:
- return -EINVAL;
+ default:
+ return -EINVAL;
}
if (acl) {
value = ext4_acl_to_disk(acl, &size);
@@ -269,14 +269,14 @@ ext4_set_acl(handle_t *handle, struct inode *inode, int type,
kfree(value);
if (!error) {
- switch(type) {
- case ACL_TYPE_ACCESS:
- ext4_iset_acl(inode, &ei->i_acl, acl);
- break;
-
- case ACL_TYPE_DEFAULT:
- ext4_iset_acl(inode, &ei->i_default_acl, acl);
- break;
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ ext4_iset_acl(inode, &ei->i_acl, acl);
+ break;
+
+ case ACL_TYPE_DEFAULT:
+ ext4_iset_acl(inode, &ei->i_default_acl, acl);
+ break;
}
}
return error;
@@ -393,7 +393,7 @@ ext4_acl_chmod(struct inode *inode)
handle_t *handle;
int retries = 0;
- retry:
+retry:
handle = ext4_journal_start(inode,
EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
if (IS_ERR(handle)) {
diff --git a/fs/ext4/acl.h b/fs/ext4/acl.h
index 26a5c1a..7eac965 100644
--- a/fs/ext4/acl.h
+++ b/fs/ext4/acl.h
@@ -58,9 +58,9 @@ static inline int ext4_acl_count(size_t size)
#define EXT4_ACL_NOT_CACHED ((void *)-1)
/* acl.c */
-extern int ext4_permission (struct inode *, int, struct nameidata *);
-extern int ext4_acl_chmod (struct inode *);
-extern int ext4_init_acl (handle_t *, struct inode *, struct inode *);
+extern int ext4_permission(struct inode *, int, struct nameidata *);
+extern int ext4_acl_chmod(struct inode *);
+extern int ext4_init_acl(handle_t *, struct inode *, struct inode *);
#else /* CONFIG_EXT4DEV_FS_POSIX_ACL */
#include <linux/sched.h>
--
1.5.3.7
Misc style fixes from checkpatch.pl
Signed-off-by: Mathieu Segaud <[email protected]>
---
fs/ext3/ext3_jbd.c | 12 ++++++------
fs/ext4/ext4_jbd2.c | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/fs/ext3/ext3_jbd.c b/fs/ext3/ext3_jbd.c
index e1f91fd..6c96260 100644
--- a/fs/ext3/ext3_jbd.c
+++ b/fs/ext3/ext3_jbd.c
@@ -9,7 +9,7 @@ int __ext3_journal_get_undo_access(const char *where, handle_t *handle,
{
int err = journal_get_undo_access(handle, bh);
if (err)
- ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -18,7 +18,7 @@ int __ext3_journal_get_write_access(const char *where, handle_t *handle,
{
int err = journal_get_write_access(handle, bh);
if (err)
- ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -27,7 +27,7 @@ int __ext3_journal_forget(const char *where, handle_t *handle,
{
int err = journal_forget(handle, bh);
if (err)
- ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -36,7 +36,7 @@ int __ext3_journal_revoke(const char *where, handle_t *handle,
{
int err = journal_revoke(handle, blocknr, bh);
if (err)
- ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -45,7 +45,7 @@ int __ext3_journal_get_create_access(const char *where,
{
int err = journal_get_create_access(handle, bh);
if (err)
- ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -54,6 +54,6 @@ int __ext3_journal_dirty_metadata(const char *where,
{
int err = journal_dirty_metadata(handle, bh);
if (err)
- ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index d6afe4e..2256c43 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -9,7 +9,7 @@ int __ext4_journal_get_undo_access(const char *where, handle_t *handle,
{
int err = jbd2_journal_get_undo_access(handle, bh);
if (err)
- ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -18,7 +18,7 @@ int __ext4_journal_get_write_access(const char *where, handle_t *handle,
{
int err = jbd2_journal_get_write_access(handle, bh);
if (err)
- ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -27,7 +27,7 @@ int __ext4_journal_forget(const char *where, handle_t *handle,
{
int err = jbd2_journal_forget(handle, bh);
if (err)
- ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -36,7 +36,7 @@ int __ext4_journal_revoke(const char *where, handle_t *handle,
{
int err = jbd2_journal_revoke(handle, blocknr, bh);
if (err)
- ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -45,7 +45,7 @@ int __ext4_journal_get_create_access(const char *where,
{
int err = jbd2_journal_get_create_access(handle, bh);
if (err)
- ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
@@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where,
{
int err = jbd2_journal_dirty_metadata(handle, bh);
if (err)
- ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
+ ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
return err;
}
--
1.5.3.7
Misc style fixes from checkpatch.pl
Signed-off-by: Mathieu Segaud <[email protected]>
---
fs/ext3/dir.c | 55 ++++++++++++++++++++++++++++---------------------------
fs/ext4/dir.c | 50 +++++++++++++++++++++++++-------------------------
2 files changed, 53 insertions(+), 52 deletions(-)
diff --git a/fs/ext3/dir.c b/fs/ext3/dir.c
index 8ca3bfd..fc3621e 100644
--- a/fs/ext3/dir.c
+++ b/fs/ext3/dir.c
@@ -33,10 +33,10 @@ static unsigned char ext3_filetype_table[] = {
};
static int ext3_readdir(struct file *, void *, filldir_t);
-static int ext3_dx_readdir(struct file * filp,
- void * dirent, filldir_t filldir);
-static int ext3_release_dir (struct inode * inode,
- struct file * filp);
+static int ext3_dx_readdir(struct file *filp,
+ void *dirent, filldir_t filldir);
+static int ext3_release_dir(struct inode *inode,
+ struct file *filp);
const struct file_operations ext3_dir_operations = {
.llseek = generic_file_llseek,
@@ -61,12 +61,12 @@ static unsigned char get_dtype(struct super_block *sb, int filetype)
}
-int ext3_check_dir_entry (const char * function, struct inode * dir,
- struct ext3_dir_entry_2 * de,
- struct buffer_head * bh,
+int ext3_check_dir_entry(const char *function, struct inode *dir,
+ struct ext3_dir_entry_2 *de,
+ struct buffer_head *bh,
unsigned long offset)
{
- const char * error_msg = NULL;
+ const char *error_msg = NULL;
const int rlen = ext3_rec_len_from_disk(de->rec_len);
if (rlen < EXT3_DIR_REC_LEN(1))
@@ -82,7 +82,7 @@ int ext3_check_dir_entry (const char * function, struct inode * dir,
error_msg = "inode out of bounds";
if (error_msg != NULL)
- ext3_error (dir->i_sb, function,
+ ext3_error(dir->i_sb, function,
"bad entry in directory #%lu: %s - "
"offset=%lu, inode=%lu, rec_len=%d, name_len=%d",
dir->i_ino, error_msg, offset,
@@ -91,8 +91,8 @@ int ext3_check_dir_entry (const char * function, struct inode * dir,
return error_msg == NULL ? 1 : 0;
}
-static int ext3_readdir(struct file * filp,
- void * dirent, filldir_t filldir)
+static int ext3_readdir(struct file *filp,
+ void *dirent, filldir_t filldir)
{
int error = 0;
unsigned long offset;
@@ -148,7 +148,7 @@ static int ext3_readdir(struct file * filp,
* of recovering data when there's a bad sector
*/
if (!bh) {
- ext3_error (sb, "ext3_readdir",
+ ext3_error(sb, "ext3_readdir",
"directory #%lu contains a hole at offset %lu",
inode->i_ino, (unsigned long)filp->f_pos);
/* corrupt size? Maybe no more blocks to read */
@@ -187,13 +187,14 @@ revalidate:
while (!error && filp->f_pos < inode->i_size
&& offset < sb->s_blocksize) {
de = (struct ext3_dir_entry_2 *) (bh->b_data + offset);
- if (!ext3_check_dir_entry ("ext3_readdir", inode, de,
+ if (!ext3_check_dir_entry("ext3_readdir", inode, de,
bh, offset)) {
- /* On error, skip the f_pos to the
- next block. */
+ /*
+ * On error, skip the f_pos to the next block.
+ */
filp->f_pos = (filp->f_pos |
(sb->s_blocksize - 1)) + 1;
- brelse (bh);
+ brelse(bh);
ret = stored;
goto out;
}
@@ -217,12 +218,12 @@ revalidate:
break;
if (version != filp->f_version)
goto revalidate;
- stored ++;
+ stored++;
}
filp->f_pos += ext3_rec_len_from_disk(de->rec_len);
}
offset = 0;
- brelse (bh);
+ brelse(bh);
}
out:
return ret;
@@ -289,9 +290,9 @@ static void free_rb_tree_fname(struct rb_root *root)
parent = rb_parent(n);
fname = rb_entry(n, struct fname, rb_hash);
while (fname) {
- struct fname * old = fname;
+ struct fname *old = fname;
fname = fname->next;
- kfree (old);
+ kfree(old);
}
if (!parent)
root->rb_node = NULL;
@@ -336,7 +337,7 @@ int ext3_htree_store_dirent(struct file *dir_file, __u32 hash,
struct ext3_dir_entry_2 *dirent)
{
struct rb_node **p, *parent = NULL;
- struct fname * fname, *new_fn;
+ struct fname *fname, *new_fn;
struct dir_private_info *info;
int len;
@@ -393,19 +394,19 @@ int ext3_htree_store_dirent(struct file *dir_file, __u32 hash,
* for all entres on the fname linked list. (Normally there is only
* one entry on the linked list, unless there are 62 bit hash collisions.)
*/
-static int call_filldir(struct file * filp, void * dirent,
+static int call_filldir(struct file *filp, void *dirent,
filldir_t filldir, struct fname *fname)
{
struct dir_private_info *info = filp->private_data;
loff_t curr_pos;
struct inode *inode = filp->f_path.dentry->d_inode;
- struct super_block * sb;
+ struct super_block *sb;
int error;
sb = inode->i_sb;
if (!fname) {
- printk("call_filldir: called with null fname?!?\n");
+ printk(KERN_WARNING "call_filldir: called with null fname?!?\n");
return 0;
}
curr_pos = hash2pos(fname->hash, fname->minor_hash);
@@ -424,8 +425,8 @@ static int call_filldir(struct file * filp, void * dirent,
return 0;
}
-static int ext3_dx_readdir(struct file * filp,
- void * dirent, filldir_t filldir)
+static int ext3_dx_readdir(struct file filp,
+ void dirent, filldir_t filldir)
{
struct dir_private_info *info = filp->private_data;
struct inode *inode = filp->f_path.dentry->d_inode;
@@ -506,7 +507,7 @@ finished:
return 0;
}
-static int ext3_release_dir (struct inode * inode, struct file * filp)
+static int ext3_release_dir(struct inode *inode, struct file *filp)
{
if (filp->private_data)
ext3_htree_free_dir_info(filp->private_data);
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index f612bef..14385b6 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -33,10 +33,10 @@ static unsigned char ext4_filetype_table[] = {
};
static int ext4_readdir(struct file *, void *, filldir_t);
-static int ext4_dx_readdir(struct file * filp,
- void * dirent, filldir_t filldir);
-static int ext4_release_dir (struct inode * inode,
- struct file * filp);
+static int ext4_dx_readdir(struct file *filp,
+ void *dirent, filldir_t filldir);
+static int ext4_release_dir(struct inode *inode,
+ struct file *filp);
const struct file_operations ext4_dir_operations = {
.llseek = generic_file_llseek,
@@ -61,12 +61,12 @@ static unsigned char get_dtype(struct super_block *sb, int filetype)
}
-int ext4_check_dir_entry (const char * function, struct inode * dir,
- struct ext4_dir_entry_2 * de,
- struct buffer_head * bh,
+int ext4_check_dir_entry(const char *function, struct inode *dir,
+ struct ext4_dir_entry_2 *de,
+ struct buffer_head *bh,
unsigned long offset)
{
- const char * error_msg = NULL;
+ const char *error_msg = NULL;
const int rlen = le16_to_cpu(de->rec_len);
if (rlen < EXT4_DIR_REC_LEN(1))
@@ -82,7 +82,7 @@ int ext4_check_dir_entry (const char * function, struct inode * dir,
error_msg = "inode out of bounds";
if (error_msg != NULL)
- ext4_error (dir->i_sb, function,
+ ext4_error(dir->i_sb, function,
"bad entry in directory #%lu: %s - "
"offset=%lu, inode=%lu, rec_len=%d, name_len=%d",
dir->i_ino, error_msg, offset,
@@ -91,8 +91,8 @@ int ext4_check_dir_entry (const char * function, struct inode * dir,
return error_msg == NULL ? 1 : 0;
}
-static int ext4_readdir(struct file * filp,
- void * dirent, filldir_t filldir)
+static int ext4_readdir(struct file *filp,
+ void *dirent, filldir_t filldir)
{
int error = 0;
unsigned long offset;
@@ -147,7 +147,7 @@ static int ext4_readdir(struct file * filp,
* of recovering data when there's a bad sector
*/
if (!bh) {
- ext4_error (sb, "ext4_readdir",
+ ext4_error(sb, "ext4_readdir",
"directory #%lu contains a hole at offset %lu",
inode->i_ino, (unsigned long)filp->f_pos);
/* corrupt size? Maybe no more blocks to read */
@@ -186,14 +186,14 @@ revalidate:
while (!error && filp->f_pos < inode->i_size
&& offset < sb->s_blocksize) {
de = (struct ext4_dir_entry_2 *) (bh->b_data + offset);
- if (!ext4_check_dir_entry ("ext4_readdir", inode, de,
+ if (!ext4_check_dir_entry("ext4_readdir", inode, de,
bh, offset)) {
/*
* On error, skip the f_pos to the next block
*/
filp->f_pos = (filp->f_pos |
(sb->s_blocksize - 1)) + 1;
- brelse (bh);
+ brelse(bh);
ret = stored;
goto out;
}
@@ -217,12 +217,12 @@ revalidate:
break;
if (version != filp->f_version)
goto revalidate;
- stored ++;
+ stored++;
}
filp->f_pos += le16_to_cpu(de->rec_len);
}
offset = 0;
- brelse (bh);
+ brelse(bh);
}
out:
return ret;
@@ -289,9 +289,9 @@ static void free_rb_tree_fname(struct rb_root *root)
parent = rb_parent(n);
fname = rb_entry(n, struct fname, rb_hash);
while (fname) {
- struct fname * old = fname;
+ struct fname *old = fname;
fname = fname->next;
- kfree (old);
+ kfree(old);
}
if (!parent)
root->rb_node = NULL;
@@ -336,7 +336,7 @@ int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
struct ext4_dir_entry_2 *dirent)
{
struct rb_node **p, *parent = NULL;
- struct fname * fname, *new_fn;
+ struct fname *fname, *new_fn;
struct dir_private_info *info;
int len;
@@ -393,19 +393,19 @@ int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
* for all entres on the fname linked list. (Normally there is only
* one entry on the linked list, unless there are 62 bit hash collisions.)
*/
-static int call_filldir(struct file * filp, void * dirent,
+static int call_filldir(struct file *filp, void *dirent,
filldir_t filldir, struct fname *fname)
{
struct dir_private_info *info = filp->private_data;
loff_t curr_pos;
struct inode *inode = filp->f_path.dentry->d_inode;
- struct super_block * sb;
+ struct super_block *sb;
int error;
sb = inode->i_sb;
if (!fname) {
- printk("call_filldir: called with null fname?!?\n");
+ printk(KERN_WARNING "call_filldir: called with null fname?!?\n");
return 0;
}
curr_pos = hash2pos(fname->hash, fname->minor_hash);
@@ -424,8 +424,8 @@ static int call_filldir(struct file * filp, void * dirent,
return 0;
}
-static int ext4_dx_readdir(struct file * filp,
- void * dirent, filldir_t filldir)
+static int ext4_dx_readdir(struct file *filp,
+ void *dirent, filldir_t filldir)
{
struct dir_private_info *info = filp->private_data;
struct inode *inode = filp->f_path.dentry->d_inode;
@@ -506,7 +506,7 @@ finished:
return 0;
}
-static int ext4_release_dir (struct inode * inode, struct file * filp)
+static int ext4_release_dir(struct inode *inode, struct file *filp)
{
if (filp->private_data)
ext4_htree_free_dir_info(filp->private_data);
--
1.5.3.7
Signed-off-by: Mathieu Segaud <[email protected]>
---
fs/ext3/bitmap.c | 2 +-
fs/ext4/bitmap.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ext3/bitmap.c b/fs/ext3/bitmap.c
index 6afc39d..c402bc4 100644
--- a/fs/ext3/bitmap.c
+++ b/fs/ext3/bitmap.c
@@ -15,7 +15,7 @@
static const int nibblemap[] = {4, 3, 3, 2, 3, 2, 2, 1, 3, 2, 2, 1, 2, 1, 1, 0};
-unsigned long ext3_count_free (struct buffer_head * map, unsigned int numchars)
+unsigned long ext3_count_free(struct buffer_head *map, unsigned int numchars)
{
unsigned int i;
unsigned long sum = 0;
diff --git a/fs/ext4/bitmap.c b/fs/ext4/bitmap.c
index 420554f..a473206 100644
--- a/fs/ext4/bitmap.c
+++ b/fs/ext4/bitmap.c
@@ -15,7 +15,7 @@
static const int nibblemap[] = {4, 3, 3, 2, 3, 2, 2, 1, 3, 2, 2, 1, 2, 1, 1, 0};
-unsigned long ext4_count_free (struct buffer_head * map, unsigned int numchars)
+unsigned long ext4_count_free(struct buffer_head *map, unsigned int numchars)
{
unsigned int i;
unsigned long sum = 0;
--
1.5.3.7
Signed-off-by: Mathieu Segaud <[email protected]>
---
fs/ext3/balloc.c | 101 ++++++++++++++++++++++++++++--------------------------
fs/ext4/balloc.c | 93 +++++++++++++++++++++++++------------------------
2 files changed, 100 insertions(+), 94 deletions(-)
diff --git a/fs/ext3/balloc.c b/fs/ext3/balloc.c
index a8ba7e8..feb00fb 100644
--- a/fs/ext3/balloc.c
+++ b/fs/ext3/balloc.c
@@ -45,17 +45,17 @@
* @bh: pointer to the buffer head to store the block
* group descriptor
*/
-struct ext3_group_desc * ext3_get_group_desc(struct super_block * sb,
+struct ext3_group_desc *ext3_get_group_desc(struct super_block *sb,
unsigned int block_group,
- struct buffer_head ** bh)
+ struct buffer_head **bh)
{
unsigned long group_desc;
unsigned long offset;
- struct ext3_group_desc * desc;
+ struct ext3_group_desc *desc;
struct ext3_sb_info *sbi = EXT3_SB(sb);
if (block_group >= sbi->s_groups_count) {
- ext3_error (sb, "ext3_get_group_desc",
+ ext3_error(sb, "ext3_get_group_desc",
"block_group >= groups_count - "
"block_group = %d, groups_count = %lu",
block_group, sbi->s_groups_count);
@@ -67,7 +67,7 @@ struct ext3_group_desc * ext3_get_group_desc(struct super_block * sb,
group_desc = block_group >> EXT3_DESC_PER_BLOCK_BITS(sb);
offset = block_group & (EXT3_DESC_PER_BLOCK(sb) - 1);
if (!sbi->s_group_desc[group_desc]) {
- ext3_error (sb, "ext3_get_group_desc",
+ ext3_error(sb, "ext3_get_group_desc",
"Group descriptor not loaded - "
"block_group = %d, group_desc = %lu, desc = %lu",
block_group, group_desc, offset);
@@ -93,15 +93,15 @@ struct ext3_group_desc * ext3_get_group_desc(struct super_block * sb,
static struct buffer_head *
read_block_bitmap(struct super_block *sb, unsigned int block_group)
{
- struct ext3_group_desc * desc;
- struct buffer_head * bh = NULL;
+ struct ext3_group_desc *desc;
+ struct buffer_head *bh = NULL;
- desc = ext3_get_group_desc (sb, block_group, NULL);
+ desc = ext3_get_group_desc(sb, block_group, NULL);
if (!desc)
goto error_out;
bh = sb_bread(sb, le32_to_cpu(desc->bg_block_bitmap));
if (!bh)
- ext3_error (sb, "read_block_bitmap",
+ ext3_error(sb, "read_block_bitmap",
"Cannot read block bitmap - "
"block_group = %d, block_bitmap = %u",
block_group, le32_to_cpu(desc->bg_block_bitmap));
@@ -142,26 +142,26 @@ restart:
bad = 0;
prev = NULL;
- printk("Block Allocation Reservation Windows Map (%s):\n", fn);
+ printk(KERN_INFO "Block Allocation Reservation Windows Map (%s):\n", fn);
while (n) {
rsv = rb_entry(n, struct ext3_reserve_window_node, rsv_node);
if (verbose)
- printk("reservation window 0x%p "
+ printk(KERN_INFO "reservation window 0x%p "
"start: %lu, end: %lu\n",
rsv, rsv->rsv_start, rsv->rsv_end);
if (rsv->rsv_start && rsv->rsv_start >= rsv->rsv_end) {
- printk("Bad reservation %p (start >= end)\n",
- rsv);
+ printk(KERN_WARNING "Bad reservation %p (start >= end)\n", rsv);
bad = 1;
}
if (prev && prev->rsv_end >= rsv->rsv_start) {
- printk("Bad reservation %p (prev->end >= start)\n",
- rsv);
+ printk(KERN_WARNING
+ "Bad reservation %p (prev->end >= start)\n", rsv);
bad = 1;
}
if (bad) {
if (!verbose) {
- printk("Restarting reservation walk in verbose mode\n");
+ printk(KERN_WARNING
+ "Restarting reservation walk in verbose mode\n");
verbose = 1;
goto restart;
}
@@ -169,7 +169,7 @@ restart:
n = rb_next(n);
prev = rsv;
}
- printk("Window map complete.\n");
+ printk(KERN_INFO "Window map complete.\n");
if (bad)
BUG();
}
@@ -197,7 +197,7 @@ restart:
*/
static int
goal_in_my_reservation(struct ext3_reserve_window *rsv, ext3_grpblk_t grp_goal,
- unsigned int group, struct super_block * sb)
+ unsigned int group, struct super_block *sb)
{
ext3_fsblk_t group_first_block, group_last_block;
@@ -268,12 +268,11 @@ void ext3_rsv_window_add(struct super_block *sb,
struct rb_node *node = &rsv->rsv_node;
ext3_fsblk_t start = rsv->rsv_start;
- struct rb_node ** p = &root->rb_node;
- struct rb_node * parent = NULL;
+ struct rb_node **p = &root->rb_node;
+ struct rb_node *parent = NULL;
struct ext3_reserve_window_node *this;
- while (*p)
- {
+ while (*p) {
parent = *p;
this = rb_entry(parent, struct ext3_reserve_window_node, rsv_node);
@@ -421,8 +420,8 @@ void ext3_free_blocks_sb(handle_t *handle, struct super_block *sb,
ext3_grpblk_t bit;
unsigned long i;
unsigned long overflow;
- struct ext3_group_desc * desc;
- struct ext3_super_block * es;
+ struct ext3_group_desc *desc;
+ struct ext3_super_block *es;
struct ext3_sb_info *sbi;
int err = 0, ret;
ext3_grpblk_t group_freed;
@@ -433,13 +432,13 @@ void ext3_free_blocks_sb(handle_t *handle, struct super_block *sb,
if (block < le32_to_cpu(es->s_first_data_block) ||
block + count < block ||
block + count > le32_to_cpu(es->s_blocks_count)) {
- ext3_error (sb, "ext3_free_blocks",
+ ext3_error(sb, "ext3_free_blocks",
"Freeing blocks not in datazone - "
"block = "E3FSBLK", count = %lu", block, count);
goto error_return;
}
- ext3_debug ("freeing block(s) %lu-%lu\n", block, block + count - 1);
+ ext3_debug("freeing block(s) %lu-%lu\n", block, block + count - 1);
do_more:
overflow = 0;
@@ -459,17 +458,17 @@ do_more:
bitmap_bh = read_block_bitmap(sb, block_group);
if (!bitmap_bh)
goto error_return;
- desc = ext3_get_group_desc (sb, block_group, &gd_bh);
+ desc = ext3_get_group_desc(sb, block_group, &gd_bh);
if (!desc)
goto error_return;
- if (in_range (le32_to_cpu(desc->bg_block_bitmap), block, count) ||
- in_range (le32_to_cpu(desc->bg_inode_bitmap), block, count) ||
- in_range (block, le32_to_cpu(desc->bg_inode_table),
+ if (in_range(le32_to_cpu(desc->bg_block_bitmap), block, count) ||
+ in_range(le32_to_cpu(desc->bg_inode_bitmap), block, count) ||
+ in_range(block, le32_to_cpu(desc->bg_inode_table),
sbi->s_itb_per_group) ||
- in_range (block + count - 1, le32_to_cpu(desc->bg_inode_table),
+ in_range(block + count - 1, le32_to_cpu(desc->bg_inode_table),
sbi->s_itb_per_group))
- ext3_error (sb, "ext3_free_blocks",
+ ext3_error(sb, "ext3_free_blocks",
"Freeing blocks in system zones - "
"Block = "E3FSBLK", count = %lu",
block, count);
@@ -579,7 +578,8 @@ do_more:
/* And the group descriptor block */
BUFFER_TRACE(gd_bh, "dirtied group descriptor block");
ret = ext3_journal_dirty_metadata(handle, gd_bh);
- if (!err) err = ret;
+ if (!err)
+ err = ret;
*pdquot_freed_blocks += group_freed;
if (overflow && !err) {
@@ -604,12 +604,12 @@ error_return:
void ext3_free_blocks(handle_t *handle, struct inode *inode,
ext3_fsblk_t block, unsigned long count)
{
- struct super_block * sb;
+ struct super_block *sb;
unsigned long dquot_freed_blocks;
sb = inode->i_sb;
if (!sb) {
- printk ("ext3_free_blocks: nonexistent device");
+ printk(KERN_WARNING "ext3_free_blocks: nonexistent device");
return;
}
ext3_free_blocks_sb(handle, sb, block, count, &dquot_freed_blocks);
@@ -765,7 +765,7 @@ claim_block(spinlock_t *lock, ext3_grpblk_t block, struct buffer_head *bh)
if (ext3_set_bit_atomic(lock, block, bh->b_data))
return 0;
jbd_lock_bh_state(bh);
- if (jh->b_committed_data && ext3_test_bit(block,jh->b_committed_data)) {
+ if (jh->b_committed_data && ext3_test_bit(block, jh->b_committed_data)) {
ext3_clear_bit_atomic(lock, block, bh->b_data);
ret = 0;
} else {
@@ -915,7 +915,7 @@ fail_access:
static int find_next_reservable_window(
struct ext3_reserve_window_node *search_head,
struct ext3_reserve_window_node *my_rsv,
- struct super_block * sb,
+ struct super_block *sb,
ext3_fsblk_t start_block,
ext3_fsblk_t last_block)
{
@@ -949,7 +949,7 @@ static int find_next_reservable_window(
prev = rsv;
next = rb_next(&rsv->rsv_node);
- rsv = rb_entry(next,struct ext3_reserve_window_node,rsv_node);
+ rsv = rb_entry(next, struct ext3_reserve_window_node, rsv_node);
/*
* Reached the last reservation, we can just append to the
@@ -1087,7 +1087,7 @@ static int alloc_new_reservation(struct ext3_reserve_window_node *my_rsv,
size = size * 2;
if (size > EXT3_MAX_RESERVE_BLOCKS)
size = EXT3_MAX_RESERVE_BLOCKS;
- my_rsv->rsv_goal_size= size;
+ my_rsv->rsv_goal_size = size;
}
}
@@ -1236,7 +1236,7 @@ static ext3_grpblk_t
ext3_try_to_allocate_with_rsv(struct super_block *sb, handle_t *handle,
unsigned int group, struct buffer_head *bitmap_bh,
ext3_grpblk_t grp_goal,
- struct ext3_reserve_window_node * my_rsv,
+ struct ext3_reserve_window_node *my_rsv,
unsigned long *count, int *errp)
{
ext3_fsblk_t group_first_block, group_last_block;
@@ -1264,7 +1264,7 @@ ext3_try_to_allocate_with_rsv(struct super_block *sb, handle_t *handle,
* or the file is not a regular file
* or last attempt to allocate a block with reservation turned on failed
*/
- if (my_rsv == NULL ) {
+ if (my_rsv == NULL) {
ret = ext3_try_to_allocate(sb, handle, group, bitmap_bh,
grp_goal, count, NULL);
goto out;
@@ -1361,9 +1361,9 @@ static int ext3_has_free_blocks(struct ext3_sb_info *sbi)
root_blocks = le32_to_cpu(sbi->s_es->s_r_blocks_count);
if (free_blocks < root_blocks + 1 && !capable(CAP_SYS_RESOURCE) &&
sbi->s_resuid != current->fsuid &&
- (sbi->s_resgid == 0 || !in_group_p (sbi->s_resgid))) {
+ (sbi->s_resgid == 0 || !in_group_p(sbi->s_resgid)))
return 0;
- }
+
return 1;
}
@@ -1457,8 +1457,11 @@ ext3_fsblk_t ext3_new_blocks(handle_t *handle, struct inode *inode,
* reservation on that particular file)
*/
block_i = EXT3_I(inode)->i_block_alloc_info;
- if (block_i && ((windowsz = block_i->rsv_window_node.rsv_goal_size) > 0))
- my_rsv = &block_i->rsv_window_node;
+ if (block_i) {
+ windowsz = block_i->rsv_window_node.rsv_goal_size
+ if (windowsz > 0)
+ my_rsv = &block_i->rsv_window_node;
+ }
if (!ext3_has_free_blocks(sbi)) {
*errp = -ENOSPC;
@@ -1714,7 +1717,7 @@ ext3_fsblk_t ext3_count_free_blocks(struct super_block *sb)
bitmap_count += x;
}
brelse(bitmap_bh);
- printk("ext3_count_free_blocks: stored = "E3FSBLK
+ printk(KERN_INFO "ext3_count_free_blocks: stored = "E3FSBLK
", computed = "E3FSBLK", "E3FSBLK"\n",
le32_to_cpu(es->s_free_blocks_count),
desc_count, bitmap_count);
@@ -1804,10 +1807,10 @@ unsigned long ext3_bg_num_gdb(struct super_block *sb, int group)
le32_to_cpu(EXT3_SB(sb)->s_es->s_first_meta_bg);
unsigned long metagroup = group / EXT3_DESC_PER_BLOCK(sb);
- if (!EXT3_HAS_INCOMPAT_FEATURE(sb,EXT3_FEATURE_INCOMPAT_META_BG) ||
+ if (!EXT3_HAS_INCOMPAT_FEATURE(sb, EXT3_FEATURE_INCOMPAT_META_BG) ||
metagroup < first_meta_bg)
- return ext3_bg_num_gdb_nometa(sb,group);
+ return ext3_bg_num_gdb_nometa(sb, group);
- return ext3_bg_num_gdb_meta(sb,group);
+ return ext3_bg_num_gdb_meta(sb, group);
}
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 71ee95e..2ce8db7 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -98,7 +98,7 @@ unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
*/
group_blocks = ext4_blocks_count(sbi->s_es) -
le32_to_cpu(sbi->s_es->s_first_data_block) -
- (EXT4_BLOCKS_PER_GROUP(sb) * (sbi->s_groups_count -1));
+ (EXT4_BLOCKS_PER_GROUP(sb) * (sbi->s_groups_count - 1));
} else {
group_blocks = EXT4_BLOCKS_PER_GROUP(sb);
}
@@ -152,17 +152,17 @@ unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
* @bh: pointer to the buffer head to store the block
* group descriptor
*/
-struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb,
+struct ext4_group_desc *ext4_get_group_desc(struct super_block *sb,
unsigned int block_group,
- struct buffer_head ** bh)
+ struct buffer_head **bh)
{
unsigned long group_desc;
unsigned long offset;
- struct ext4_group_desc * desc;
+ struct ext4_group_desc *desc;
struct ext4_sb_info *sbi = EXT4_SB(sb);
if (block_group >= sbi->s_groups_count) {
- ext4_error (sb, "ext4_get_group_desc",
+ ext4_error(sb, "ext4_get_group_desc",
"block_group >= groups_count - "
"block_group = %d, groups_count = %lu",
block_group, sbi->s_groups_count);
@@ -174,7 +174,7 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb,
group_desc = block_group >> EXT4_DESC_PER_BLOCK_BITS(sb);
offset = block_group & (EXT4_DESC_PER_BLOCK(sb) - 1);
if (!sbi->s_group_desc[group_desc]) {
- ext4_error (sb, "ext4_get_group_desc",
+ ext4_error(sb, "ext4_get_group_desc",
"Group descriptor not loaded - "
"block_group = %d, group_desc = %lu, desc = %lu",
block_group, group_desc, offset);
@@ -202,8 +202,8 @@ struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb,
struct buffer_head *
read_block_bitmap(struct super_block *sb, unsigned int block_group)
{
- struct ext4_group_desc * desc;
- struct buffer_head * bh = NULL;
+ struct ext4_group_desc *desc;
+ struct buffer_head *bh = NULL;
ext4_fsblk_t bitmap_blk;
desc = ext4_get_group_desc(sb, block_group, NULL);
@@ -225,7 +225,7 @@ read_block_bitmap(struct super_block *sb, unsigned int block_group)
bh = sb_bread(sb, bitmap_blk);
}
if (!bh)
- ext4_error (sb, __FUNCTION__,
+ ext4_error(sb, __FUNCTION__,
"Cannot read block bitmap - "
"block_group = %d, block_bitmap = %llu",
block_group, bitmap_blk);
@@ -265,26 +265,26 @@ restart:
bad = 0;
prev = NULL;
- printk("Block Allocation Reservation Windows Map (%s):\n", fn);
+ printk(KERN_INFO "Block Allocation Reservation Windows Map (%s):\n", fn);
while (n) {
rsv = rb_entry(n, struct ext4_reserve_window_node, rsv_node);
if (verbose)
- printk("reservation window 0x%p "
+ printk(KERN_INFO "reservation window 0x%p "
"start: %llu, end: %llu\n",
rsv, rsv->rsv_start, rsv->rsv_end);
if (rsv->rsv_start && rsv->rsv_start >= rsv->rsv_end) {
- printk("Bad reservation %p (start >= end)\n",
- rsv);
+ printk(KERN_WARNING "Bad reservation %p (start >= end)\n", rsv);
bad = 1;
}
if (prev && prev->rsv_end >= rsv->rsv_start) {
- printk("Bad reservation %p (prev->end >= start)\n",
- rsv);
+ printk(KERN_WARNING
+ "Bad reservation %p (prev->end >= start)\n", rsv);
bad = 1;
}
if (bad) {
if (!verbose) {
- printk("Restarting reservation walk in verbose mode\n");
+ printk(KERN_WARNING
+ "Restarting reservation walk in verbose mode\n");
verbose = 1;
goto restart;
}
@@ -292,7 +292,7 @@ restart:
n = rb_next(n);
prev = rsv;
}
- printk("Window map complete.\n");
+ printk(KERN_INFO "Window map complete.\n");
if (bad)
BUG();
}
@@ -320,7 +320,7 @@ restart:
*/
static int
goal_in_my_reservation(struct ext4_reserve_window *rsv, ext4_grpblk_t grp_goal,
- unsigned int group, struct super_block * sb)
+ unsigned int group, struct super_block *sb)
{
ext4_fsblk_t group_first_block, group_last_block;
@@ -391,12 +391,11 @@ void ext4_rsv_window_add(struct super_block *sb,
struct rb_node *node = &rsv->rsv_node;
ext4_fsblk_t start = rsv->rsv_start;
- struct rb_node ** p = &root->rb_node;
- struct rb_node * parent = NULL;
+ struct rb_node **p = &root->rb_node;
+ struct rb_node *parent = NULL;
struct ext4_reserve_window_node *this;
- while (*p)
- {
+ while (*p) {
parent = *p;
this = rb_entry(parent, struct ext4_reserve_window_node, rsv_node);
@@ -544,8 +543,8 @@ void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
ext4_grpblk_t bit;
unsigned long i;
unsigned long overflow;
- struct ext4_group_desc * desc;
- struct ext4_super_block * es;
+ struct ext4_group_desc *desc;
+ struct ext4_super_block *es;
struct ext4_sb_info *sbi;
int err = 0, ret;
ext4_grpblk_t group_freed;
@@ -556,13 +555,13 @@ void ext4_free_blocks_sb(handle_t *handle, struct super_block *sb,
if (block < le32_to_cpu(es->s_first_data_block) ||
block + count < block ||
block + count > ext4_blocks_count(es)) {
- ext4_error (sb, "ext4_free_blocks",
+ ext4_error(sb, "ext4_free_blocks",
"Freeing blocks not in datazone - "
"block = %llu, count = %lu", block, count);
goto error_return;
}
- ext4_debug ("freeing block(s) %llu-%llu\n", block, block + count - 1);
+ ext4_debug("freeing block(s) %llu-%llu\n", block, block + count - 1);
do_more:
overflow = 0;
@@ -579,7 +578,7 @@ do_more:
bitmap_bh = read_block_bitmap(sb, block_group);
if (!bitmap_bh)
goto error_return;
- desc = ext4_get_group_desc (sb, block_group, &gd_bh);
+ desc = ext4_get_group_desc(sb, block_group, &gd_bh);
if (!desc)
goto error_return;
@@ -588,7 +587,7 @@ do_more:
in_range(block, ext4_inode_table(sb, desc), sbi->s_itb_per_group) ||
in_range(block + count - 1, ext4_inode_table(sb, desc),
sbi->s_itb_per_group))
- ext4_error (sb, "ext4_free_blocks",
+ ext4_error(sb, "ext4_free_blocks",
"Freeing blocks in system zones - "
"Block = %llu, count = %lu",
block, count);
@@ -699,7 +698,8 @@ do_more:
/* And the group descriptor block */
BUFFER_TRACE(gd_bh, "dirtied group descriptor block");
ret = ext4_journal_dirty_metadata(handle, gd_bh);
- if (!err) err = ret;
+ if (!err)
+ err = ret;
*pdquot_freed_blocks += group_freed;
if (overflow && !err) {
@@ -724,12 +724,12 @@ error_return:
void ext4_free_blocks(handle_t *handle, struct inode *inode,
ext4_fsblk_t block, unsigned long count)
{
- struct super_block * sb;
+ struct super_block *sb;
unsigned long dquot_freed_blocks;
sb = inode->i_sb;
if (!sb) {
- printk ("ext4_free_blocks: nonexistent device");
+ printk(KERN_WARNING "ext4_free_blocks: nonexistent device");
return;
}
ext4_free_blocks_sb(handle, sb, block, count, &dquot_freed_blocks);
@@ -885,7 +885,7 @@ claim_block(spinlock_t *lock, ext4_grpblk_t block, struct buffer_head *bh)
if (ext4_set_bit_atomic(lock, block, bh->b_data))
return 0;
jbd_lock_bh_state(bh);
- if (jh->b_committed_data && ext4_test_bit(block,jh->b_committed_data)) {
+ if (jh->b_committed_data && ext4_test_bit(block, jh->b_committed_data)) {
ext4_clear_bit_atomic(lock, block, bh->b_data);
ret = 0;
} else {
@@ -1035,7 +1035,7 @@ fail_access:
static int find_next_reservable_window(
struct ext4_reserve_window_node *search_head,
struct ext4_reserve_window_node *my_rsv,
- struct super_block * sb,
+ struct super_block *sb,
ext4_fsblk_t start_block,
ext4_fsblk_t last_block)
{
@@ -1069,7 +1069,7 @@ static int find_next_reservable_window(
prev = rsv;
next = rb_next(&rsv->rsv_node);
- rsv = rb_entry(next,struct ext4_reserve_window_node,rsv_node);
+ rsv = rb_entry(next, struct ext4_reserve_window_node, rsv_node);
/*
* Reached the last reservation, we can just append to the
@@ -1207,7 +1207,7 @@ static int alloc_new_reservation(struct ext4_reserve_window_node *my_rsv,
size = size * 2;
if (size > EXT4_MAX_RESERVE_BLOCKS)
size = EXT4_MAX_RESERVE_BLOCKS;
- my_rsv->rsv_goal_size= size;
+ my_rsv->rsv_goal_size = size;
}
}
@@ -1356,7 +1356,7 @@ static ext4_grpblk_t
ext4_try_to_allocate_with_rsv(struct super_block *sb, handle_t *handle,
unsigned int group, struct buffer_head *bitmap_bh,
ext4_grpblk_t grp_goal,
- struct ext4_reserve_window_node * my_rsv,
+ struct ext4_reserve_window_node *my_rsv,
unsigned long *count, int *errp)
{
ext4_fsblk_t group_first_block, group_last_block;
@@ -1384,7 +1384,7 @@ ext4_try_to_allocate_with_rsv(struct super_block *sb, handle_t *handle,
* or the file is not a regular file
* or last attempt to allocate a block with reservation turned on failed
*/
- if (my_rsv == NULL ) {
+ if (my_rsv == NULL) {
ret = ext4_try_to_allocate(sb, handle, group, bitmap_bh,
grp_goal, count, NULL);
goto out;
@@ -1481,9 +1481,9 @@ static int ext4_has_free_blocks(struct ext4_sb_info *sbi)
root_blocks = ext4_r_blocks_count(sbi->s_es);
if (free_blocks < root_blocks + 1 && !capable(CAP_SYS_RESOURCE) &&
sbi->s_resuid != current->fsuid &&
- (sbi->s_resgid == 0 || !in_group_p (sbi->s_resgid))) {
+ (sbi->s_resgid == 0 || !in_group_p(sbi->s_resgid)))
return 0;
- }
+
return 1;
}
@@ -1577,8 +1577,11 @@ ext4_fsblk_t ext4_new_blocks(handle_t *handle, struct inode *inode,
* reservation on that particular file)
*/
block_i = EXT4_I(inode)->i_block_alloc_info;
- if (block_i && ((windowsz = block_i->rsv_window_node.rsv_goal_size) > 0))
- my_rsv = &block_i->rsv_window_node;
+ if (block_i) {
+ windowsz = block_i->rsv_window_node.rsv_goal_size;
+ if (windowsz > 0)
+ my_rsv = &block_i->rsv_window_node;
+ }
if (!ext4_has_free_blocks(sbi)) {
*errp = -ENOSPC;
@@ -1834,7 +1837,7 @@ ext4_fsblk_t ext4_count_free_blocks(struct super_block *sb)
bitmap_count += x;
}
brelse(bitmap_bh);
- printk("ext4_count_free_blocks: stored = %llu"
+ printk(KERN_INFO "ext4_count_free_blocks: stored = %llu"
", computed = %llu, %llu\n",
EXT4_FREE_BLOCKS_COUNT(es),
desc_count, bitmap_count);
@@ -1924,10 +1927,10 @@ unsigned long ext4_bg_num_gdb(struct super_block *sb, int group)
le32_to_cpu(EXT4_SB(sb)->s_es->s_first_meta_bg);
unsigned long metagroup = group / EXT4_DESC_PER_BLOCK(sb);
- if (!EXT4_HAS_INCOMPAT_FEATURE(sb,EXT4_FEATURE_INCOMPAT_META_BG) ||
+ if (!EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_META_BG) ||
metagroup < first_meta_bg)
- return ext4_bg_num_gdb_nometa(sb,group);
+ return ext4_bg_num_gdb_nometa(sb, group);
- return ext4_bg_num_gdb_meta(sb,group);
+ return ext4_bg_num_gdb_meta(sb, group);
}
--
1.5.3.7
Mathieu Segaud wrote:
> Misc style fixes from checkpatch.pl
>
> Signed-off-by: Mathieu Segaud <[email protected]>
> ---
> fs/ext3/ext3_jbd.c | 12 ++++++------
> fs/ext4/ext4_jbd2.c | 12 ++++++------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ext3/ext3_jbd.c b/fs/ext3/ext3_jbd.c
> index e1f91fd..6c96260 100644
> --- a/fs/ext3/ext3_jbd.c
> +++ b/fs/ext3/ext3_jbd.c
> @@ -9,7 +9,7 @@ int __ext3_journal_get_undo_access(const char *where, handle_t *handle,
> {
> int err = journal_get_undo_access(handle, bh);
> if (err)
> - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -18,7 +18,7 @@ int __ext3_journal_get_write_access(const char *where, handle_t *handle,
> {
> int err = journal_get_write_access(handle, bh);
> if (err)
> - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -27,7 +27,7 @@ int __ext3_journal_forget(const char *where, handle_t *handle,
> {
> int err = journal_forget(handle, bh);
> if (err)
> - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -36,7 +36,7 @@ int __ext3_journal_revoke(const char *where, handle_t *handle,
> {
> int err = journal_revoke(handle, blocknr, bh);
> if (err)
> - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -45,7 +45,7 @@ int __ext3_journal_get_create_access(const char *where,
> {
> int err = journal_get_create_access(handle, bh);
> if (err)
> - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -54,6 +54,6 @@ int __ext3_journal_dirty_metadata(const char *where,
> {
> int err = journal_dirty_metadata(handle, bh);
> if (err)
> - ext3_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext3_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index d6afe4e..2256c43 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -9,7 +9,7 @@ int __ext4_journal_get_undo_access(const char *where, handle_t *handle,
> {
> int err = jbd2_journal_get_undo_access(handle, bh);
> if (err)
> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -18,7 +18,7 @@ int __ext4_journal_get_write_access(const char *where, handle_t *handle,
> {
> int err = jbd2_journal_get_write_access(handle, bh);
> if (err)
> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -27,7 +27,7 @@ int __ext4_journal_forget(const char *where, handle_t *handle,
> {
> int err = jbd2_journal_forget(handle, bh);
> if (err)
> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -36,7 +36,7 @@ int __ext4_journal_revoke(const char *where, handle_t *handle,
> {
> int err = jbd2_journal_revoke(handle, blocknr, bh);
> if (err)
> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -45,7 +45,7 @@ int __ext4_journal_get_create_access(const char *where,
> {
> int err = jbd2_journal_get_create_access(handle, bh);
> if (err)
> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
> @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where,
> {
> int err = jbd2_journal_dirty_metadata(handle, bh);
> if (err)
> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
> return err;
> }
>
Hi Mathieu
What about changing the __FUNCTION__ to __func__, while you are at it?
Richard Knutsson
Coding-style only changes tends to screw up our ability to merge
pending patches, but I'll take care of it, thanks.
- Ted
Vous m'avez dit r?cemment :
> Coding-style only changes tends to screw up our ability to merge
> pending patches, but I'll take care of it, thanks
yep, I noticed that...
would you rather me wait till 2.6.24 is out ?
--
Mathieu
Vous m'avez dit r?cemment :
[snip]
> Hi Mathieu
>
> What about changing the __FUNCTION__ to __func__, while you are at it?
well, won't do any harm, even though other compilers than GCC are not
officialy supported :)
thanks a lot
And I think I will revamp, including fixes into fs/ext2
--
Mathieu
On Fri, Jan 04, 2008 at 02:49:07PM +0100, Mathieu SEGAUD wrote:
> Vous m'avez dit r?cemment :
>
> > Coding-style only changes tends to screw up our ability to merge
> > pending patches, but I'll take care of it, thanks
>
> yep, I noticed that...
> would you rather me wait till 2.6.24 is out ?
I'll take your patches and merge all or most of it as I can into the
patch queue destined for 2.6.24-rc1. Check back with me after -rc1;
if there are some left over we'll do them after that.
- Ted
Theodore Tso <[email protected]> writes:
> Coding-style only changes tends to screw up our ability to merge
> pending patches, but I'll take care of it, thanks.
Exactly. And looking at the patch the old code was already perfectly
readable anyways. Benefit about zero.
I also don't see how you can take care of patch conflicts caused by
this for patches not yet in your tree but still in development somewhere
else.
IMHO any coding style cleanup should only done on code that changes
anyways, but not on other code.
The recent flurry of cleanup code patches on l-k causes far more
problems than it solves. I'm not even sure why people do this? Just
because it is en vogue recently?
If they want to contribute in simple ways to the Linux kernel I'm sure
actually really useful things can be found that they can change.
-Andi
On Fri, Jan 04, 2008 at 05:30:00PM +0100, Andi Kleen wrote:
>
> Exactly. And looking at the patch the old code was already perfectly
> readable anyways. Benefit about zero.
File this under the "checkpatch.pl" considered harmful category....
The problem is not with the tool, but that at least *some* people seem
to think that making checkpatch.pl be completely silent is somehow
this holy grail that will make kernel code bug-free(tm). (And of
course, people who want to encode nazi-like coding conventions and to
force all of the kernel to use a single coding convention as if that
somehow would improve the kernel's quality. Some of that is OK, but
as long as the code is readable, do we really care about whether or
not the code is using exactly the same coding conventions everyplace?
Or, pressuring maintainers not to ignore cleanup patches lest they be
viewed as "bad" maintainers?)
Personally I find it annoying, but I'm willing to live with the
cleanup patches. I don't think they add anything, though. Maybe I
should be more cranky about such patches....
> The recent flurry of cleanup code patches on l-k causes far more
> problems than it solves. I'm not even sure why people do this? Just
> because it is en vogue recently?
I don't know, because people want to be able to say that they've
contributed fixes to the Linux kernel?
I will say that in past kernel summit program commitees, the
perception that someone _only_ submitted trivial patches (i.e.,
whitespace-only, spelling fixes in comments, etc.) has sometimes been
perceived a negative factor towards whether the program committee
might consider that person to be useful contributor to discussions at
the kernel summit.... So people should be warned (I would have hoped
that it would be obvious), that submitting vast number of trivial
cleanup patches without contributing anything else will very likely
not work, and possibly backfire if that is your goal.
- Ted
> Personally I find it annoying, but I'm willing to live with the
> cleanup patches. I don't think they add anything, though. Maybe I
The problem I see is that if someone has a more involved outstanding
patch series against the code that is being cleaned up (and more complicated
features tend to require some time to stabilize so "just merge early"
is not always the solution) then it is a serious mess to readapt
a patch series to the cleanups. Yes it can be all done but it wastes
time that would be more constructively used e.g. for better testing.
Now if some area is changed anyways then they're usually ok because
all outstanding patches will need to be adapted anyways.
So I guess a useful rule for cleanup patches would be "only if that
code changed recently"
> > The recent flurry of cleanup code patches on l-k causes far more
> > problems than it solves. I'm not even sure why people do this? Just
> > because it is en vogue recently?
>
> I don't know, because people want to be able to say that they've
> contributed fixes to the Linux kernel?
My pet theory is that it is similar to the "unsubscribe me"
cascade effect you sometimes see on mailing lists. One person
sends a "unsubscribe me" to everybody and then suddenly a lot of
people think that is the right way to unsubscribe and reply
with lots of "unsubscribe me too".
So one person sends a cleanup and it gets accepted and suddenly
other people realize it is very easy to do these cleanups
(not realizing the hidden costs they have) and then they go on...
I thought we had the janitor project to steer these people into
more useful directions, but apparently that is not well known
enough anymore. Perhaps it just needs to be more regularly announced?
Although I must admit I am not 100% happy with kernel-janitors
either -- e.g. a few times I sent suggestions about easy things
someone could do to that list, but never heard anything back.
Anyways there are lots of ways to do trivial cleanups in a useful
way and if people want to do this perhaps they should just
ask on linux-kernel and people suggest something?
My hope here is of course that these trivial changes are primarily
used as a way to get "the feet wet" to understand the procedures
for contribuing larger not quite as trivial changes
-Andi
P.S.: Mathieu, this is not against you personally; you just happened
to be a convenient example of a larger problem in this case. Sorry.
[Andi Kleen - Fri, Jan 04, 2008 at 08:41:29PM +0100]
[...snip...]
| My hope here is of course that these trivial changes are primarily
| used as a way to get "the feet wet" to understand the procedures
| for contribuing larger not quite as trivial changes
|
| -Andi
|
| P.S.: Mathieu, this is not against you personally; you just happened
| to be a convenient example of a larger problem in this case. Sorry.
|
Oh, Andi, you're so right! I don't like to admit that, but you're
right (in my case at least). I counted four clean-up patches I sent
since last 'really bug fix' I made patch. Actually I thought that
really helps the kernel but it seems it doesn't and causes more
problem with patch applience (unfortunately).
- Cyrill -
On Jan 4, 2008 8:41 PM, Andi Kleen <[email protected]> wrote:
[...]
> > I don't know, because people want to be able to say that they've
> > contributed fixes to the Linux kernel?
>
> My pet theory is that it is similar to the "unsubscribe me"
> cascade effect you sometimes see on mailing lists. One person
> sends a "unsubscribe me" to everybody and then suddenly a lot of
> people think that is the right way to unsubscribe and reply
> with lots of "unsubscribe me too".
>
> So one person sends a cleanup and it gets accepted and suddenly
> other people realize it is very easy to do these cleanups
> (not realizing the hidden costs they have) and then they go on...
Since I'm one of those people that sent "Codying style fixes" patches
I give my contribution to this discussion as well.
I think that _one_ of the reasons that made a few people sent this kind of
patches to the list is because checkpatch.pl is far better then any other
kerneljanitor scripts/easy task and _seems_ to be an easy way to start
understanding the code, creation of patches and process in general.
> I thought we had the janitor project to steer these people into
> more useful directions, but apparently that is not well known
> enough anymore. Perhaps it just needs to be more regularly announced?
>
> Although I must admit I am not 100% happy with kernel-janitors
> either -- e.g. a few times I sent suggestions about easy things
> someone could do to that list, but never heard anything back.
>
> Anyways there are lots of ways to do trivial cleanups in a useful
> way and if people want to do this perhaps they should just
> ask on linux-kernel and people suggest something?
Yes please do that.
Even if fixing errors reported by checkpatch.pl still sounds like a
useful way to spent a couple of hours.
Maybe our mistake was to send the patches to lkml instead of to
[email protected]
or to kerneljanitors?
I mean, I now understand the rationales behind your complaints but I
don't think it's
good idea to discourage people willing to perform easy task.
They just need guidance in order to be useful.
> My hope here is of course that these trivial changes are primarily
> used as a way to get "the feet wet" to understand the procedures
> for contribuing larger not quite as trivial changes
Agreed.
ciao,
--
Paolo
http://paolo.ciarrocchi.googlepages.com/
On Fri, Jan 04, 2008 at 09:03:53PM +0100, Paolo Ciarrocchi wrote:
> On Jan 4, 2008 8:41 PM, Andi Kleen <[email protected]> wrote:
> [...]
> > > I don't know, because people want to be able to say that they've
> > > contributed fixes to the Linux kernel?
> >
> > My pet theory is that it is similar to the "unsubscribe me"
> > cascade effect you sometimes see on mailing lists. One person
> > sends a "unsubscribe me" to everybody and then suddenly a lot of
> > people think that is the right way to unsubscribe and reply
> > with lots of "unsubscribe me too".
> >
> > So one person sends a cleanup and it gets accepted and suddenly
> > other people realize it is very easy to do these cleanups
> > (not realizing the hidden costs they have) and then they go on...
>
> Since I'm one of those people that sent "Codying style fixes" patches
> I give my contribution to this discussion as well.
>
> I think that _one_ of the reasons that made a few people sent this kind of
> patches to the list is because checkpatch.pl is far better then any other
> kerneljanitor scripts/easy task and _seems_ to be an easy way to start
> understanding the code, creation of patches and process in general.
The problem is that it has large hidden costs as pointed out. So while
it might be easy for you it's not a cheap operation for the whole development
process.
>
> > I thought we had the janitor project to steer these people into
> > more useful directions, but apparently that is not well known
> > enough anymore. Perhaps it just needs to be more regularly announced?
> >
> > Although I must admit I am not 100% happy with kernel-janitors
> > either -- e.g. a few times I sent suggestions about easy things
> > someone could do to that list, but never heard anything back.
> >
> > Anyways there are lots of ways to do trivial cleanups in a useful
> > way and if people want to do this perhaps they should just
> > ask on linux-kernel and people suggest something?
>
> Yes please do that.
> Even if fixing errors reported by checkpatch.pl still sounds like a
> useful way to spent a couple of hours.
> Maybe our mistake was to send the patches to lkml instead of to
> [email protected]
> or to kerneljanitors?
No, they would have ended in tree too eventually and caused the same problem.
How about if you're looking for simple work for a few hours you just
send an email to l-k and ask if someone has an idea for something?
I'm sure you'll get suggestions. Probably more than you can take.
e.g. from the top of my hat what would be useful:
- Go through Documentation/* files and check if the options etc. described
in there are still in the code
That will actually require you to find code in the source tree and understand
it at least a little bit which are both very useful skills in general.
- Or check for kerneldoc comments that do not appear in the kerneldoc output
(because the files are missing in the DocBook templates)
- Or build the kernel and check for any "deprecated" warnings and fix them
[perhaps not 100% trivial, but should be doable by studying other code
a bit -- i expect that people who attempt to write such patches have at least
some knowledge of programming and C so that should be possible]
> I mean, I now understand the rationales behind your complaints but I
> don't think it's
> good idea to discourage people willing to perform easy task.
> They just need guidance in order to be useful.
Yes, the best way to get guidance is to ask.
-Andi
On Jan 4, 2008 11:33 PM, Andi Kleen <[email protected]> wrote:
[...]
> > I think that _one_ of the reasons that made a few people sent this kind of
> > patches to the list is because checkpatch.pl is far better then any other
> > kerneljanitor scripts/easy task and _seems_ to be an easy way to start
> > understanding the code, creation of patches and process in general.
>
> The problem is that it has large hidden costs as pointed out. So while
> it might be easy for you it's not a cheap operation for the whole development
> process.
Isn't it a timing problem?
I mean, I guess that codying style fixes are OK if there is a good coordination
with the maintainer and patches are sent with the right timing in
order to not cause
problems in the process.
Do you agree?
May be, similar as you suggested, next time people should ask on the list
and fixing the codying style issues on the files suggested by the relevant
maintainers?
[...]
>
> How about if you're looking for simple work for a few hours you just
> send an email to l-k and ask if someone has an idea for something?
> I'm sure you'll get suggestions. Probably more than you can take.
>
> e.g. from the top of my hat what would be useful:
>
> - Go through Documentation/* files and check if the options etc. described
> in there are still in the code
>
> That will actually require you to find code in the source tree and understand
> it at least a little bit which are both very useful skills in general.
>
> - Or check for kerneldoc comments that do not appear in the kerneldoc output
> (because the files are missing in the DocBook templates)
>
> - Or build the kernel and check for any "deprecated" warnings and fix them
> [perhaps not 100% trivial, but should be doable by studying other code
> a bit -- i expect that people who attempt to write such patches have at least
> some knowledge of programming and C so that should be possible]
OK, thanks for the hints!
> > I mean, I now understand the rationales behind your complaints but I
> > don't think it's
> > good idea to discourage people willing to perform easy task.
> > They just need guidance in order to be useful.
>
> Yes, the best way to get guidance is to ask.
Ciao,
--
Paolo
http://paolo.ciarrocchi.googlepages.com/
On Sat, Jan 05, 2008 at 01:12:44AM +0100, Paolo Ciarrocchi wrote:
> Isn't it a timing problem?
> I mean, I guess that codying style fixes are OK if there is a good coordination
> with the maintainer and patches are sent with the right timing in
> order to not cause
> problems in the process.
But because running some kind of mechanical script and fixing up the
problems is relatively mindless, it doesn't *add* anything. Only the
maintainer knows when it is a reasonably convenient time to fix all of
the problems, or when to fix portions of the code that is being
reworked anyway --- and the maintainer can just run the script by
himself, for himself. The patch doesn't actually save him much work,
and in some cases, is actually more work than simply regenerating the
fixes *after* the other patches waiting in his patch queue have been
applied (but of course, it screws up everyone else's patches that
haven't been submitted to the maintainer yet).
In some cases, it's worth it. In other (most) cases, it really isn't.
- Ted
On Jan 04, 2008 14:41 +0100, Richard Knutsson wrote:
>> @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where,
>> {
>> int err = jbd2_journal_dirty_metadata(handle, bh);
>> if (err)
>> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
>> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
>> return err;
>> }
>
> What about changing the __FUNCTION__ to __func__, while you are at it?
What's wrong with __FUNCTION__? I thought that was ANSI C?
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
Andreas Dilger пишет:
> On Jan 04, 2008 14:41 +0100, Richard Knutsson wrote:
>>> @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where,
>>> {
>>> int err = jbd2_journal_dirty_metadata(handle, bh);
>>> if (err)
>>> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
>>> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
>>> return err;
>>> }
>> What about changing the __FUNCTION__ to __func__, while you are at it?
>
> What's wrong with __FUNCTION__? I thought that was ANSI C?
No, it was not. The ANSI C 1990 Standard defines the following so-called
"predefined macros": __LINE__, __FILE__, __DATE__, __TIME__, and __STDC__.
The ISO/IEC 9899 Standard commonly referred to as the C99, defines a few
additional predefined macros, as well as an additional predefined
identifier __func__. For more information please refer to the ISO/IEC 9899
document itself, which is freely available for download at the time of
me writing this.
Although seemingly "natural", the __FUNCTION__ macro has never been part
of the C Standard.
Dmitri
>
> Cheers, Andreas
> --
> Andreas Dilger
> Sr. Staff Engineer, Lustre Group
> Sun Microsystems of Canada, Inc.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Andreas Dilger пишет:
> On Jan 04, 2008 14:41 +0100, Richard Knutsson wrote:
>>> @@ -54,6 +54,6 @@ int __ext4_journal_dirty_metadata(const char *where,
>>> {
>>> int err = jbd2_journal_dirty_metadata(handle, bh);
>>> if (err)
>>> - ext4_journal_abort_handle(where, __FUNCTION__, bh, handle,err);
>>> + ext4_journal_abort_handle(where, __FUNCTION__, bh, handle, err);
>>> return err;
>>> }
>> What about changing the __FUNCTION__ to __func__, while you are at it?
>
> What's wrong with __FUNCTION__? I thought that was ANSI C?
No, it was not. The ANSI C 1990 Standard defines the following so-called
"predefined macros": __LINE__, __FILE__, __DATE__, __TIME__, and __STDC__.
The ISO/IEC 9899 Standard commonly referred to as the C99, defines a few
additional predefined macros, as well as an additional predefined
identifier __func__. For more information please refer to the ISO/IEC 9899
document itself, which is freely available for download at the time of
me writing this.
Although seemingly "natural", the __FUNCTION__ macro has never been part
of the C Standard.
Dmitri
>
> Cheers, Andreas
> --
> Andreas Dilger
> Sr. Staff Engineer, Lustre Group
> Sun Microsystems of Canada, Inc.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
On Fri, Jan 04, 2008 at 09:12:28PM -0700, Andreas Dilger wrote:
> What's wrong with __FUNCTION__? I thought that was ANSI C?
__FUNCTION__ is a gccism of dubious taste - it pretends to be a macro
when it's something far out of scope of preprocessor. Think for a minute
and you'll see why - you can't expand it until you are done with parsing.
__func__ is C99, but it's not what __FUNCTION__ used to be - it's not a
string literal. 6.4.2.2(1):
The identifier __func__ shall be implicitly declared by the translator
as if, immediately following the opening brace of each function definition,
the declaration
static const char __func__[] = "function-name";
appeared, where function-name is the name of the lexically-enclosing function.
IOW, it's a phase 7 (parsing and translation of translation units) and not
phase 4 (preprocessor). Practical implications are:
* _way_ fewer kludges
* it happens after phase 6 (string literal concatenation)
So __FUNCION__ " is called" within body of foo() would result in
"foo is called" while __func__ "is called" is a syntax error.
These days old gcc __FUNCTION__ is gone; it's a macro expanding to __func__,
so behaviour does *not* match the original (see above).
The thing is, it's not something like __FILE__ or __LINE__ and never had been;
it tried to pretend that it had been like those but that required far too
nasty kludges and these days it is firmly in the "nothing to do with
preprocessor" land. Old name is #defined to __func__ to approximate the
old behaviour, but it doesn't approximate it all that well and it's really
not fit for anything but backwards compatibility in legacy code.
BTW, we used to have code that broke when gcc abandoned the old kludge -
several years ago there'd been a pile of patches fixing the resulting
turds.
On Jan 4 2008 19:39, Theodore Tso wrote:
>On Sat, Jan 05, 2008 at 01:12:44AM +0100, Paolo Ciarrocchi wrote:
>
>But because running some kind of mechanical script and fixing up the
>problems is relatively mindless, it doesn't *add* anything. Only the
>maintainer knows when it is a reasonably convenient time to fix all of
>the problems, or when to fix portions of the code that is being
>reworked anyway --- and the maintainer can just run the script by
>himself, for himself.
I have to agree. Best use of checkpatch is right before submitting, uh,
a patch actually. The option that makes it work on non-patch files
should be, hm, hidden or removed.
Al Viro wrote:
> __func__ is C99, but it's not what __FUNCTION__ used to be - it's not a
> string literal. 6.4.2.2(1):
>
> The identifier __func__ shall be implicitly declared by the translator
> as if, immediately following the opening brace of each function definition,
> the declaration
> static const char __func__[] = "function-name";
> appeared, where function-name is the name of the lexically-enclosing function.
>
> IOW, it's a phase 7 (parsing and translation of translation units) and not
> phase 4 (preprocessor). Practical implications are:
> * _way_ fewer kludges
> * it happens after phase 6 (string literal concatenation)
> So __FUNCION__ " is called" within body of foo() would result in
> "foo is called" while __func__ "is called" is a syntax error.
>
> These days old gcc __FUNCTION__ is gone; it's a macro expanding to __func__,
> so behaviour does *not* match the original (see above).
so if I understand correctly, this requires a fix:
--
__FUNCTION__ is a macro expanding to __func__ and translated as if previously
in that function was declared:
static const char __func__[] = "function-name";
As is DEBUG() - when active - will produce a syntax error.
Signed-off-by: Roel Kluin <[email protected]>
---
diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c
index ce9d5c4..855e1d6 100644
--- a/drivers/pcmcia/au1000_xxs1500.c
+++ b/drivers/pcmcia/au1000_xxs1500.c
@@ -56,7 +56,7 @@
#define PCMCIA_IRQ AU1000_GPIO_4
#if 0
-#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
+#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args)
#else
#define DEBUG(x,args...)
#endif
On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote:
> -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
> +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args)
Can this really be expected to work when x contains conversions?
How about:
#define DEBUG(x, args...) printk("%s: " x, __func__, ##args)
//Peter
On Fri, Jan 11, 2008 at 12:42:40PM +0900, Paul Mundt wrote:
> How about throwing out hand-rolled debug printk wrappers for the
> brain-damage they are and using the ones the kernel provides
> instead?
Sounds great!
//Peter
On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote:
> On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote:
> > -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
> > +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args)
>
> Can this really be expected to work when x contains conversions?
>
> How about:
>
> #define DEBUG(x, args...) printk("%s: " x, __func__, ##args)
>
How about throwing out hand-rolled debug printk wrappers for the
brain-damage they are and using the ones the kernel provides instead?
dev_dbg() and pr_debug() both manage to get these semantics right, and
you can even bury the #define DEBUG underneath some Kconfig silliness if
you're the kind of person that leans that way.
Maybe we can just amend checkpatch to delete a patch out of protest if it
introduces printk() wrappers..
Paul Mundt wrote:
> On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote:
>> On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote:
>>> -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
>>> +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args)
>> Can this really be expected to work when x contains conversions?
>>
>> How about:
>>
>> #define DEBUG(x, args...) printk("%s: " x, __func__, ##args)
>>
> How about throwing out hand-rolled debug printk wrappers for the
> brain-damage they are and using the ones the kernel provides instead?
>
Should it be done like this?
--
Replace printk wrapper - with a syntax error - by pr_debug
Signed-off-by: Roel Kluin <[email protected]>
---
diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c
index ce9d5c4..4c32389 100644
--- a/drivers/pcmcia/au1000_xxs1500.c
+++ b/drivers/pcmcia/au1000_xxs1500.c
@@ -25,6 +25,10 @@
*
*
*/
+#ifdef CONFIG_MIPS_XXS1500_DEBUG
+#define DEBUG 1
+#endif
+
#include <linux/module.h>
#include <linux/init.h>
#include <linux/delay.h>
@@ -55,11 +59,6 @@
#define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1)
#define PCMCIA_IRQ AU1000_GPIO_4
-#if 0
-#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
-#else
-#define DEBUG(x,args...)
-#endif
static int xxs1500_pcmcia_init(struct pcmcia_init *init)
{
@@ -143,13 +142,13 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
if(configure->sock > PCMCIA_MAX_SOCK) return -1;
- DEBUG("Vcc %dV Vpp %dV, reset %d\n",
+ pr_debug("Vcc %dV Vpp %dV, reset %d\n",
configure->vcc, configure->vpp, configure->reset);
switch(configure->vcc){
case 33: /* Vcc 3.3V */
/* turn on power */
- DEBUG("turn on power\n");
+ pr_debug("turn on power\n");
au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<14))|(1<<30),
GPIO2_OUTPUT);
au_sync_delay(100);
@@ -166,7 +165,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
}
if (!configure->reset) {
- DEBUG("deassert reset\n");
+ pr_debug("deassert reset\n");
au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<4))|(1<<20),
GPIO2_OUTPUT);
au_sync_delay(100);
@@ -174,7 +173,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
GPIO2_OUTPUT);
}
else {
- DEBUG("assert reset\n");
+ pr_debug("assert reset\n");
au_writel(au_readl(GPIO2_PINSTATE) | (1<<4)|(1<<20),
GPIO2_OUTPUT);
}
On Fri, Jan 11, 2008 at 10:45:30AM +0100, Roel Kluin wrote:
> Paul Mundt wrote:
> > On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote:
> >> On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote:
> >>> -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
> >>> +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args)
> >> Can this really be expected to work when x contains conversions?
> >>
> >> How about:
> >>
> >> #define DEBUG(x, args...) printk("%s: " x, __func__, ##args)
> >>
> > How about throwing out hand-rolled debug printk wrappers for the
> > brain-damage they are and using the ones the kernel provides instead?
> >
> Should it be done like this?
> --
> Replace printk wrapper - with a syntax error - by pr_debug
>
> Signed-off-by: Roel Kluin <[email protected]>
Close. But in this case #define DEBUG is already instrumented by the
subsystem-wide debug option if it's enabled, so it's preferable to use
that and just drop the special debug Kconfig option completely.
Take a look at how CONFIG_PCMCIA_DEBUG is handled.
With DEBUG()->pr_debug() conversion here you've silently dropped the
__func__ prefixing. Note that dev_dbg() is usually preferred when you can
get a hold of a struct device pointer, as it takes care of prettifying
the output with the driver name and so on, rather than the convention of
adding a prefix. If you can't get at the struct device pointer, you'll
probably just want to insert the __func__ prefixing manually at the
callsites.
Paul Mundt wrote:
> On Fri, Jan 11, 2008 at 10:45:30AM +0100, Roel Kluin wrote:
>> Paul Mundt wrote:
>>> On Fri, Jan 11, 2008 at 04:09:45AM +0100, Peter Stuge wrote:
>>>> On Thu, Jan 10, 2008 at 10:03:58PM +0100, Roel Kluin wrote:
>>>>> -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
>>>>> +#define DEBUG(x, args...) printk("%s: ", __func__, x, ##args)
>>>> Can this really be expected to work when x contains conversions?
>>>>
>>>> How about:
>>>>
>>>> #define DEBUG(x, args...) printk("%s: " x, __func__, ##args)
>>>>
>>> How about throwing out hand-rolled debug printk wrappers for the
>>> brain-damage they are and using the ones the kernel provides instead?
>>>
>> Should it be done like this?
>> --
>> Replace printk wrapper - with a syntax error - by pr_debug
>>
>> Signed-off-by: Roel Kluin <[email protected]>
>
> Close. But in this case #define DEBUG is already instrumented by the
> subsystem-wide debug option if it's enabled, so it's preferable to use
> that and just drop the special debug Kconfig option completely.
>
> Take a look at how CONFIG_PCMCIA_DEBUG is handled.
In drivers/pcmcia/Makefile, when CONFIG_PCMCIA_DEBUG=y, it gives
EXTRA_CFLAGS += -DDEBUG
which causes the definition of DEBUG as a macro, with definition 1.
> With DEBUG()->pr_debug() conversion here you've silently dropped the
> __func__ prefixing. Note that dev_dbg() is usually preferred when you can
> get a hold of a struct device pointer, as it takes care of prettifying
> the output with the driver name and so on, rather than the convention of
> adding a prefix. If you can't get at the struct device pointer, you'll
> probably just want to insert the __func__ prefixing manually at the
> callsites.
Ah, ok, then this should be right:
--
Replace printk wrapper - with a syntax error - by pr_debug.
DEBUG is defined 1 when CONFIG_PCMCIA_DEBUG is set.
Signed-off-by: Roel Kluin <[email protected]>
---
diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c
index ce9d5c4..8e6426b 100644
--- a/drivers/pcmcia/au1000_xxs1500.c
+++ b/drivers/pcmcia/au1000_xxs1500.c
@@ -55,12 +55,6 @@
#define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1)
#define PCMCIA_IRQ AU1000_GPIO_4
-#if 0
-#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
-#else
-#define DEBUG(x,args...)
-#endif
-
static int xxs1500_pcmcia_init(struct pcmcia_init *init)
{
return PCMCIA_NUM_SOCKS;
@@ -143,13 +137,13 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
if(configure->sock > PCMCIA_MAX_SOCK) return -1;
- DEBUG("Vcc %dV Vpp %dV, reset %d\n",
+ pr_debug("Vcc %dV Vpp %dV, reset %d\n",
configure->vcc, configure->vpp, configure->reset);
switch(configure->vcc){
case 33: /* Vcc 3.3V */
/* turn on power */
- DEBUG("turn on power\n");
+ pr_debug("turn on power\n");
au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<14))|(1<<30),
GPIO2_OUTPUT);
au_sync_delay(100);
@@ -166,7 +160,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
}
if (!configure->reset) {
- DEBUG("deassert reset\n");
+ pr_debug("deassert reset\n");
au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<4))|(1<<20),
GPIO2_OUTPUT);
au_sync_delay(100);
@@ -174,7 +168,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
GPIO2_OUTPUT);
}
else {
- DEBUG("assert reset\n");
+ pr_debug("assert reset\n");
au_writel(au_readl(GPIO2_PINSTATE) | (1<<4)|(1<<20),
GPIO2_OUTPUT);
}
On Fri, Jan 11, 2008 at 12:04:14PM +0100, Roel Kluin wrote:
> Paul Mundt wrote:
> > Take a look at how CONFIG_PCMCIA_DEBUG is handled.
>
> In drivers/pcmcia/Makefile, when CONFIG_PCMCIA_DEBUG=y, it gives
> EXTRA_CFLAGS += -DDEBUG
> which causes the definition of DEBUG as a macro, with definition 1.
>
> > With DEBUG()->pr_debug() conversion here you've silently dropped the
> > __func__ prefixing. Note that dev_dbg() is usually preferred when you can
> > get a hold of a struct device pointer, as it takes care of prettifying
> > the output with the driver name and so on, rather than the convention of
> > adding a prefix. If you can't get at the struct device pointer, you'll
> > probably just want to insert the __func__ prefixing manually at the
> > callsites.
>
> Ah, ok, then this should be right:
> --
> Replace printk wrapper - with a syntax error - by pr_debug.
> DEBUG is defined 1 when CONFIG_PCMCIA_DEBUG is set.
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c
> index ce9d5c4..8e6426b 100644
> --- a/drivers/pcmcia/au1000_xxs1500.c
> +++ b/drivers/pcmcia/au1000_xxs1500.c
> @@ -55,12 +55,6 @@
> #define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1)
> #define PCMCIA_IRQ AU1000_GPIO_4
>
> -#if 0
> -#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
> -#else
> -#define DEBUG(x,args...)
> -#endif
> -
> static int xxs1500_pcmcia_init(struct pcmcia_init *init)
> {
> return PCMCIA_NUM_SOCKS;
> @@ -143,13 +137,13 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
>
> if(configure->sock > PCMCIA_MAX_SOCK) return -1;
>
> - DEBUG("Vcc %dV Vpp %dV, reset %d\n",
> + pr_debug("Vcc %dV Vpp %dV, reset %d\n",
> configure->vcc, configure->vpp, configure->reset);
>
You're still changing the semantics here. The DEBUG() does __FUNCTION__
prefixing, while pr_debug() does not. This should be something like
pr_debug("%s: ....", __func__, ...); instead, if you want to maintain
consistency. Beyond that, this looks fine, yes.
Paul Mundt wrote:
> On Fri, Jan 11, 2008 at 12:04:14PM +0100, Roel Kluin wrote:
>> Paul Mundt wrote:
>>> Take a look at how CONFIG_PCMCIA_DEBUG is handled.
>> In drivers/pcmcia/Makefile, when CONFIG_PCMCIA_DEBUG=y, it gives
>> EXTRA_CFLAGS += -DDEBUG
>> which causes the definition of DEBUG as a macro, with definition 1.
>>
>>> With DEBUG()->pr_debug() conversion here you've silently dropped the
>>> __func__ prefixing. Note that dev_dbg() is usually preferred when you can
>>> get a hold of a struct device pointer, as it takes care of prettifying
>>> the output with the driver name and so on, rather than the convention of
>>> adding a prefix. If you can't get at the struct device pointer, you'll
>>> probably just want to insert the __func__ prefixing manually at the
>>> callsites.
> You're still changing the semantics here. The DEBUG() does __FUNCTION__
> prefixing, while pr_debug() does not. This should be something like
> pr_debug("%s: ....", __func__, ...); instead, if you want to maintain
> consistency. Beyond that, this looks fine, yes.
Somehow I overlooked your last point. Well, the original code had no
semantics - it wasn't working - but I get your point.
Below a patch to print the __func__'s. Note that all pr_debugs are in the
same function. Maybe the function name should be printed in the first
pr_debug only? If desired, I'll send another patch.
--
Replace printk wrapper - with a syntax error - by pr_debug; keep printing
the __func__'s. (DEBUG is defined 1 when CONFIG_PCMCIA_DEBUG is set)
Signed-off-by: Roel Kluin <[email protected]>
---
diff --git a/drivers/pcmcia/au1000_xxs1500.c b/drivers/pcmcia/au1000_xxs1500.c
index ce9d5c4..b4bad6e 100644
--- a/drivers/pcmcia/au1000_xxs1500.c
+++ b/drivers/pcmcia/au1000_xxs1500.c
@@ -55,12 +55,6 @@
#define PCMCIA_NUM_SOCKS (PCMCIA_MAX_SOCK + 1)
#define PCMCIA_IRQ AU1000_GPIO_4
-#if 0
-#define DEBUG(x,args...) printk(__FUNCTION__ ": " x,##args)
-#else
-#define DEBUG(x,args...)
-#endif
-
static int xxs1500_pcmcia_init(struct pcmcia_init *init)
{
return PCMCIA_NUM_SOCKS;
@@ -143,13 +137,13 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
if(configure->sock > PCMCIA_MAX_SOCK) return -1;
- DEBUG("Vcc %dV Vpp %dV, reset %d\n",
+ pr_debug("%s: Vcc %dV Vpp %dV, reset %d\n", __func__,
configure->vcc, configure->vpp, configure->reset);
switch(configure->vcc){
case 33: /* Vcc 3.3V */
/* turn on power */
- DEBUG("turn on power\n");
+ pr_debug("%s: turn on power\n", __func__);
au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<14))|(1<<30),
GPIO2_OUTPUT);
au_sync_delay(100);
@@ -166,7 +160,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
}
if (!configure->reset) {
- DEBUG("deassert reset\n");
+ pr_debug("%s: deassert reset\n", __func__);
au_writel((au_readl(GPIO2_PINSTATE) & ~(1<<4))|(1<<20),
GPIO2_OUTPUT);
au_sync_delay(100);
@@ -174,7 +168,7 @@ xxs1500_pcmcia_configure_socket(const struct pcmcia_configure *configure)
GPIO2_OUTPUT);
}
else {
- DEBUG("assert reset\n");
+ pr_debug("%s: assert reset\n", __func__);
au_writel(au_readl(GPIO2_PINSTATE) | (1<<4)|(1<<20),
GPIO2_OUTPUT);
}