2022-07-04 09:31:31

by Lukas Bulwahn

[permalink] [raw]
Subject: [PATCH] secretmem: drop unneeded initialization in secretmem_file_create()

Drop the unneeded initialization of the local variable file in function
secretmem_file_create().

No functional change and no change in the resulting object code.

This unneeded initialization was already introduced with the introduction
of secretmem in commit 1507f51255c9 ("mm: introduce memfd_secret system
call to create "secret" memory areas"). This minor code-stylistic issue
was discovered as a dead store with clang-analyzer.

Signed-off-by: Lukas Bulwahn <[email protected]>
---
Mike, please ack.

Andrew, please pick this minor stylistic clean-up patch. Thanks.

mm/secretmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/secretmem.c b/mm/secretmem.c
index 7584aab54d20..46b431d62761 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -171,7 +171,7 @@ static struct vfsmount *secretmem_mnt;

static struct file *secretmem_file_create(unsigned long flags)
{
- struct file *file = ERR_PTR(-ENOMEM);
+ struct file *file;
struct inode *inode;
const char *anon_name = "[secretmem]";
const struct qstr qname = QSTR_INIT(anon_name, strlen(anon_name));
--
2.17.1


2022-07-04 15:10:02

by Tom Rix

[permalink] [raw]
Subject: Re: [PATCH] secretmem: drop unneeded initialization in secretmem_file_create()


On 7/4/22 2:26 AM, Lukas Bulwahn wrote:
> Drop the unneeded initialization of the local variable file in function
> secretmem_file_create().
>
> No functional change and no change in the resulting object code.
>
> This unneeded initialization was already introduced with the introduction
> of secretmem in commit 1507f51255c9 ("mm: introduce memfd_secret system
> call to create "secret" memory areas").

You may need to be more specific about this commit, the whole function,
including this initialization was introduced at this time.

Otherwise, change looks fine.

Reviewed-by: Tom Rix <[email protected]>

> This minor code-stylistic issue
> was discovered as a dead store with clang-analyzer.
>
> Signed-off-by: Lukas Bulwahn <[email protected]>
> ---
> Mike, please ack.
>
> Andrew, please pick this minor stylistic clean-up patch. Thanks.
>
> mm/secretmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/secretmem.c b/mm/secretmem.c
> index 7584aab54d20..46b431d62761 100644
> --- a/mm/secretmem.c
> +++ b/mm/secretmem.c
> @@ -171,7 +171,7 @@ static struct vfsmount *secretmem_mnt;
>
> static struct file *secretmem_file_create(unsigned long flags)
> {
> - struct file *file = ERR_PTR(-ENOMEM);
> + struct file *file;
> struct inode *inode;
> const char *anon_name = "[secretmem]";
> const struct qstr qname = QSTR_INIT(anon_name, strlen(anon_name));

2022-07-04 16:38:50

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH] secretmem: drop unneeded initialization in secretmem_file_create()

Hi Lukas,

On Mon, Jul 04, 2022 at 11:26:05AM +0200, Lukas Bulwahn wrote:
> Drop the unneeded initialization of the local variable file in function
> secretmem_file_create().
>
> No functional change and no change in the resulting object code.
>
> This unneeded initialization was already introduced with the introduction
> of secretmem in commit 1507f51255c9 ("mm: introduce memfd_secret system
> call to create "secret" memory areas"). This minor code-stylistic issue
> was discovered as a dead store with clang-analyzer.

You are right and in the current code the initialization is unneeded, but
there's a pending patch that relies of this initialization:

https://lore.kernel.org/all/[email protected]/

Let's wait and see how that fix develops.

> Signed-off-by: Lukas Bulwahn <[email protected]>
> ---
> Mike, please ack.
>
> Andrew, please pick this minor stylistic clean-up patch. Thanks.
>
> mm/secretmem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/secretmem.c b/mm/secretmem.c
> index 7584aab54d20..46b431d62761 100644
> --- a/mm/secretmem.c
> +++ b/mm/secretmem.c
> @@ -171,7 +171,7 @@ static struct vfsmount *secretmem_mnt;
>
> static struct file *secretmem_file_create(unsigned long flags)
> {
> - struct file *file = ERR_PTR(-ENOMEM);
> + struct file *file;
> struct inode *inode;
> const char *anon_name = "[secretmem]";
> const struct qstr qname = QSTR_INIT(anon_name, strlen(anon_name));
> --
> 2.17.1
>

--
Sincerely yours,
Mike.

2022-07-05 06:22:07

by Lukas Bulwahn

[permalink] [raw]
Subject: Re: [PATCH] secretmem: drop unneeded initialization in secretmem_file_create()

On Mon, Jul 4, 2022 at 6:30 PM Mike Rapoport <[email protected]> wrote:
>
> Hi Lukas,
>
> On Mon, Jul 04, 2022 at 11:26:05AM +0200, Lukas Bulwahn wrote:
> > Drop the unneeded initialization of the local variable file in function
> > secretmem_file_create().
> >
> > No functional change and no change in the resulting object code.
> >
> > This unneeded initialization was already introduced with the introduction
> > of secretmem in commit 1507f51255c9 ("mm: introduce memfd_secret system
> > call to create "secret" memory areas"). This minor code-stylistic issue
> > was discovered as a dead store with clang-analyzer.
>
> You are right and in the current code the initialization is unneeded, but
> there's a pending patch that relies of this initialization:
>
> https://lore.kernel.org/all/[email protected]/
>
> Let's wait and see how that fix develops.
>

Agree. I will continue tracking the evolution of this function and
resend my patch if this patch above does not land in the next six
months. For now, this clean-up patch here can be ignored.

Lukas