[PATCH] hugetlbfs:inode: initialize 'error' variable at definition to reduce code redundancy

[PATCH] hugetlbfs:inode: initialize 'error' variable at definition to reduce code redundancy

[Hu Song]

Initialize the error variable to -ENOMEM at definition in
init_hugetlbfs_fs().This removes the need for a separate
initialization later and makes the code slightly more concise,
while still preserving the original logic.
No functional change intended.

Signed-off-by: Hu Song <husong@kylinos.cn>
---
 fs/hugetlbfs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index e4de5425838d..390cddd5872c 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1600,7 +1600,7 @@ static int __init init_hugetlbfs_fs(void)
 {
 	struct vfsmount *mnt;
 	struct hstate *h;
-	int error;
+	int error = -ENOMEM;
 	int i;
 
 	if (!hugepages_supported()) {
@@ -1608,7 +1608,6 @@ static int __init init_hugetlbfs_fs(void)
 		return -ENOTSUPP;
 	}
 
-	error = -ENOMEM;
 	hugetlbfs_inode_cachep = kmem_cache_create("hugetlbfs_inode_cache",
 					sizeof(struct hugetlbfs_inode_info),
 					0, SLAB_ACCOUNT, init_once);
-- 
2.25.1

Re: [PATCH] hugetlbfs:inode: initialize 'error' variable at definition to reduce code redundancy

[Oscar Salvador]

On Mon, Jun 09, 2025 at 02:56:15PM +0800, Hu Song wrote:
> Initialize the error variable to -ENOMEM at definition in
> init_hugetlbfs_fs().This removes the need for a separate
> initialization later and makes the code slightly more concise,
> while still preserving the original logic.
> No functional change intended.
> 
> Signed-off-by: Hu Song <husong@kylinos.cn>
> ---
>  fs/hugetlbfs/inode.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index e4de5425838d..390cddd5872c 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -1600,7 +1600,7 @@ static int __init init_hugetlbfs_fs(void)
>  {
>  	struct vfsmount *mnt;
>  	struct hstate *h;
> -	int error;
> +	int error = -ENOMEM;
>  	int i;
>  
>  	if (!hugepages_supported()) {
> @@ -1608,7 +1608,6 @@ static int __init init_hugetlbfs_fs(void)
>  		return -ENOTSUPP;
>  	}
>  
> -	error = -ENOMEM;
>  	hugetlbfs_inode_cachep = kmem_cache_create("hugetlbfs_inode_cache",
>  					sizeof(struct hugetlbfs_inode_info),
>  					0, SLAB_ACCOUNT, init_once);

Uhmf, I do not know. 

Often, we tend to use those declarations in order to mark what error
code we will return if we fail the next operationg. E.g:

  error = -ENOMEM
  if (try_to_allocate)
    goto out;
 
  error = -EINVAL
  if (check_params)
    goto out;
 
 out:
  return error

No really strong opinion here, but I'd vote for leave it as is?


-- 
Oscar Salvador
SUSE Labs

Re: [PATCH] hugetlbfs:inode: initialize 'error' variable at definition to reduce code redundancy

[Ye Liu]

On 2025/6/11 02:51, Oscar Salvador wrote:
> On Mon, Jun 09, 2025 at 02:56:15PM +0800, Hu Song wrote:
>> Initialize the error variable to -ENOMEM at definition in
>> init_hugetlbfs_fs().This removes the need for a separate
>> initialization later and makes the code slightly more concise,
>> while still preserving the original logic.
>> No functional change intended.
>>
>> Signed-off-by: Hu Song <husong@kylinos.cn>
>> ---
>>  fs/hugetlbfs/inode.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
>> index e4de5425838d..390cddd5872c 100644
>> --- a/fs/hugetlbfs/inode.c
>> +++ b/fs/hugetlbfs/inode.c
>> @@ -1600,7 +1600,7 @@ static int __init init_hugetlbfs_fs(void)
>>  {
>>  	struct vfsmount *mnt;
>>  	struct hstate *h;
>> -	int error;
>> +	int error = -ENOMEM;
>>  	int i;
>>  
>>  	if (!hugepages_supported()) {
>> @@ -1608,7 +1608,6 @@ static int __init init_hugetlbfs_fs(void)
>>  		return -ENOTSUPP;
>>  	}
>>  
>> -	error = -ENOMEM;
>>  	hugetlbfs_inode_cachep = kmem_cache_create("hugetlbfs_inode_cache",
>>  					sizeof(struct hugetlbfs_inode_info),
>>  					0, SLAB_ACCOUNT, init_once);
> 
> Uhmf, I do not know. 
> 
> Often, we tend to use those declarations in order to mark what error
> code we will return if we fail the next operationg. E.g:
> 
>   error = -ENOMEM
>   if (try_to_allocate)
>     goto out;
>  
>   error = -EINVAL
>   if (check_params)
>     goto out;
>  
>  out:
>   return error
> 

I prefer the following format, as it avoids unnecessary assignments:

        if (try_to_allocate) {
                error = -ENOMEM
                goto out;
        }
        
        if (check_params) {
                error = -EINVAL
                goto out;
        }
out:
	return error                   

> No really strong opinion here, but I'd vote for leave it as is?

Currently init_hugetlbfs_fs() pre-initializes the error variable
with -ENOMEM before the allocation that might fail. This creates
an unnecessary assignment in the success path.                  

Therefore, this change might be preferable:

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index e4de5425838d..3fa7892ac865 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1608,12 +1608,13 @@ static int __init init_hugetlbfs_fs(void)
                return -ENOTSUPP;
        }
 
-       error = -ENOMEM;
        hugetlbfs_inode_cachep = kmem_cache_create("hugetlbfs_inode_cache",
                                        sizeof(struct hugetlbfs_inode_info),
                                        0, SLAB_ACCOUNT, init_once);
-       if (hugetlbfs_inode_cachep == NULL)
+       if (hugetlbfs_inode_cachep == NULL) {
+               error = -ENOMEM;
                goto out;
+       }
 
        error = register_filesystem(&hugetlbfs_fs_type);
        if (error)

Thanks,
Ye