2015-08-02 11:03:59

by Shraddha Barke

[permalink] [raw]
Subject: [PATCH] Staging : lustre: Struct file_operations should be const

This patch fixes the following checkpatch.pl warning:

WARNING: struct file_operations should normally be const

Signed-off-by: Shraddha Barke <[email protected]>
---
drivers/staging/lustre/lustre/llite/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index dcd0c6d..369a722 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -3116,7 +3116,7 @@ int ll_inode_permission(struct inode *inode, int mask)
}

/* -o localflock - only provides locally consistent flock locks */
-struct file_operations ll_file_operations = {
+const struct file_operations ll_file_operations = {
.read_iter = ll_file_read_iter,
.write_iter = ll_file_write_iter,
.unlocked_ioctl = ll_file_ioctl,
--
2.1.0


2015-08-02 11:26:20

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH] Staging : lustre: Struct file_operations should be const



On Sun, 2 Aug 2015, Shraddha Barke wrote:

> This patch fixes the following checkpatch.pl warning:
>
> WARNING: struct file_operations should normally be const

This is not the good way to write the message. The information it conveys
is "checkpatch generated this warning, so I have made some change
(unspecified) that make checkpatch not complain".

A better approach would be to say:

Declare the file_operations structure ll_file_operations as const, as done
elsewhere in the kernel, as there are no modifications to its fields.

Problem found using checkpatch.

This way the reader knows what was done, and what issues were taken into
account when making the change. At the same time, you still credit
checkpatch for helping you find the problem

julia

>
> Signed-off-by: Shraddha Barke <[email protected]>
> ---
> drivers/staging/lustre/lustre/llite/file.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
> index dcd0c6d..369a722 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -3116,7 +3116,7 @@ int ll_inode_permission(struct inode *inode, int mask)
> }
>
> /* -o localflock - only provides locally consistent flock locks */
> -struct file_operations ll_file_operations = {
> +const struct file_operations ll_file_operations = {
> .read_iter = ll_file_read_iter,
> .write_iter = ll_file_write_iter,
> .unlocked_ioctl = ll_file_ioctl,
> --
> 2.1.0
>
>

2015-08-02 17:34:40

by Shraddha Barke

[permalink] [raw]
Subject: [PATCH v2] Revert "Staging : lustre: Struct file_operations should be const"

Declare the file_operations structure ll_file_operations as const, as done
elsewhere in the kernel, as there are no modifications to its fields.

Problem found using checkpatch.

Signed-off-by: Shraddha Barke <[email protected]>
---
Changes in v2:
- Make the commit message more clearer.

drivers/staging/lustre/lustre/llite/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
index 369a722..dcd0c6d 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -3116,7 +3116,7 @@ int ll_inode_permission(struct inode *inode, int mask)
}

/* -o localflock - only provides locally consistent flock locks */
-const struct file_operations ll_file_operations = {
+struct file_operations ll_file_operations = {
.read_iter = ll_file_read_iter,
.write_iter = ll_file_write_iter,
.unlocked_ioctl = ll_file_ioctl,
--
2.1.0

2015-08-03 05:29:22

by Sudip Mukherjee

[permalink] [raw]
Subject: Re: [PATCH v2] Revert "Staging : lustre: Struct file_operations should be const"

On Sun, Aug 02, 2015 at 11:04:22PM +0530, Shraddha Barke wrote:

Your subject says 'Revert "Staging : lustre: Struct file_operations
should be const"'. This patch reverts which commit?

> Declare the file_operations structure ll_file_operations as const, as done
> elsewhere in the kernel, as there are no modifications to its fields.
>
> Problem found using checkpatch.
>
> Signed-off-by: Shraddha Barke <[email protected]>
> ---
> Changes in v2:
> - Make the commit message more clearer.
>
> drivers/staging/lustre/lustre/llite/file.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c
> index 369a722..dcd0c6d 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -3116,7 +3116,7 @@ int ll_inode_permission(struct inode *inode, int mask)
> }
>
> /* -o localflock - only provides locally consistent flock locks */
> -const struct file_operations ll_file_operations = {
> +struct file_operations ll_file_operations = {
Your commit message says "Declare the file_operations structure
ll_file_operations as const" but your patch is removing the const.

regards
sudip