2014-11-15 18:42:40

by SF Markus Elfring

Subject: [PATCH 1/1] ntfs: Deletion of unnecessary checks before the function call "iput"

From: Markus Elfring <[email protected]>
Date: Sat, 15 Nov 2014 19:35:05 +0100

The iput() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <[email protected]>
---
fs/ntfs/super.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 6c3296e..8f22a47 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -2204,17 +2204,12 @@ get_ctx_vol_failed:
return true;
#ifdef NTFS_RW
iput_usnjrnl_err_out:
- if (vol->usnjrnl_j_ino)
- iput(vol->usnjrnl_j_ino);
- if (vol->usnjrnl_max_ino)
- iput(vol->usnjrnl_max_ino);
- if (vol->usnjrnl_ino)
- iput(vol->usnjrnl_ino);
+ iput(vol->usnjrnl_j_ino);
+ iput(vol->usnjrnl_max_ino);
+ iput(vol->usnjrnl_ino);
iput_quota_err_out:
- if (vol->quota_q_ino)
- iput(vol->quota_q_ino);
- if (vol->quota_ino)
- iput(vol->quota_ino);
+ iput(vol->quota_q_ino);
+ iput(vol->quota_ino);
iput(vol->extend_ino);
#endif /* NTFS_RW */
iput_sec_err_out:
@@ -2223,8 +2218,7 @@ iput_root_err_out:
iput(vol->root_ino);
iput_logfile_err_out:
#ifdef NTFS_RW
- if (vol->logfile_ino)
- iput(vol->logfile_ino);
+ iput(vol->logfile_ino);
iput_vol_err_out:
#endif /* NTFS_RW */
iput(vol->vol_ino);
@@ -2254,8 +2248,7 @@ iput_mftbmp_err_out:
iput(vol->mftbmp_ino);
iput_mirr_err_out:
#ifdef NTFS_RW
- if (vol->mftmirr_ino)
- iput(vol->mftmirr_ino);
+ iput(vol->mftmirr_ino);
#endif /* NTFS_RW */
return false;
}
--
2.1.3

2014-11-15 19:54:26

by Julia Lawall

Subject: Re: [Cocci] [PATCH 1/1] ntfs: Deletion of unnecessary checks before the function call "iput"

On Sat, 15 Nov 2014, SF Markus Elfring wrote:

> From: Markus Elfring <[email protected]>
> Date: Sat, 15 Nov 2014 19:35:05 +0100
>
> The iput() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> fs/ntfs/super.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 6c3296e..8f22a47 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -2204,17 +2204,12 @@ get_ctx_vol_failed:
> return true;
> #ifdef NTFS_RW
> iput_usnjrnl_err_out:

I don't have time to look at the code now, but since there is an exit
label here, have you checked whether you could improve the gotos in these
cases?

julia

> - if (vol->usnjrnl_j_ino)
> - iput(vol->usnjrnl_j_ino);
> - if (vol->usnjrnl_max_ino)
> - iput(vol->usnjrnl_max_ino);
> - if (vol->usnjrnl_ino)
> - iput(vol->usnjrnl_ino);
> + iput(vol->usnjrnl_j_ino);
> + iput(vol->usnjrnl_max_ino);
> + iput(vol->usnjrnl_ino);
> iput_quota_err_out:
> - if (vol->quota_q_ino)
> - iput(vol->quota_q_ino);
> - if (vol->quota_ino)
> - iput(vol->quota_ino);
> + iput(vol->quota_q_ino);
> + iput(vol->quota_ino);
> iput(vol->extend_ino);
> #endif /* NTFS_RW */
> iput_sec_err_out:
> @@ -2223,8 +2218,7 @@ iput_root_err_out:
> iput(vol->root_ino);
> iput_logfile_err_out:
> #ifdef NTFS_RW
> - if (vol->logfile_ino)
> - iput(vol->logfile_ino);
> + iput(vol->logfile_ino);
> iput_vol_err_out:
> #endif /* NTFS_RW */
> iput(vol->vol_ino);
> @@ -2254,8 +2248,7 @@ iput_mftbmp_err_out:
> iput(vol->mftbmp_ino);
> iput_mirr_err_out:
> #ifdef NTFS_RW
> - if (vol->mftmirr_ino)
> - iput(vol->mftmirr_ino);
> + iput(vol->mftmirr_ino);
> #endif /* NTFS_RW */
> return false;
> }
> --
> 2.1.3
>
>
> _______________________________________________
> Cocci mailing list
> [email protected]
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

2015-07-05 08:30:11

by SF Markus Elfring

Subject: Re: [PATCH] ntfs: Deletion of unnecessary checks before the function call "iput"

> From: Markus Elfring <[email protected]>
> Date: Sat, 15 Nov 2014 19:35:05 +0100
>
> The iput() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> fs/ntfs/super.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
> index 6c3296e..8f22a47 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -2204,17 +2204,12 @@ get_ctx_vol_failed:
> return true;
> #ifdef NTFS_RW
> iput_usnjrnl_err_out:
> - if (vol->usnjrnl_j_ino)
> - iput(vol->usnjrnl_j_ino);
> - if (vol->usnjrnl_max_ino)
> - iput(vol->usnjrnl_max_ino);
> - if (vol->usnjrnl_ino)
> - iput(vol->usnjrnl_ino);
> + iput(vol->usnjrnl_j_ino);
> + iput(vol->usnjrnl_max_ino);
> + iput(vol->usnjrnl_ino);
> iput_quota_err_out:
> - if (vol->quota_q_ino)
> - iput(vol->quota_q_ino);
> - if (vol->quota_ino)
> - iput(vol->quota_ino);
> + iput(vol->quota_q_ino);
> + iput(vol->quota_ino);
> iput(vol->extend_ino);
> #endif /* NTFS_RW */
> iput_sec_err_out:
> @@ -2223,8 +2218,7 @@ iput_root_err_out:
> iput(vol->root_ino);
> iput_logfile_err_out:
> #ifdef NTFS_RW
> - if (vol->logfile_ino)
> - iput(vol->logfile_ino);
> + iput(vol->logfile_ino);
> iput_vol_err_out:
> #endif /* NTFS_RW */
> iput(vol->vol_ino);
> @@ -2254,8 +2248,7 @@ iput_mftbmp_err_out:
> iput(vol->mftbmp_ino);
> iput_mirr_err_out:
> #ifdef NTFS_RW
> - if (vol->mftmirr_ino)
> - iput(vol->mftmirr_ino);
> + iput(vol->mftmirr_ino);
> #endif /* NTFS_RW */
> return false;
> }
>

Would you like to integrate this update suggestion
into another source code repository?

Regards,
Markus

2015-07-05 08:30:01

by Anton Altaparmakov

Subject: Re: [PATCH] ntfs: Deletion of unnecessary checks before the function call "iput"

Hi Andrew,

Can you please take up this trivial patch and merge it upstream?

Reviewed-by: Anton Altaparmakov <[email protected]>

Thanks a lot in advance!

Best regards,

Anton

> On 4 Jul 2015, at 11:32, SF Markus Elfring <[email protected]> wrote:
>
>> From: Markus Elfring <[email protected]>
>> Date: Sat, 15 Nov 2014 19:35:05 +0100
>>
>> The iput() function tests whether its argument is NULL and then
>> returns immediately. Thus the test around the call is not needed.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Signed-off-by: Markus Elfring <[email protected]>
>> ---
>> fs/ntfs/super.c | 21 +++++++--------------
>> 1 file changed, 7 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
>> index 6c3296e..8f22a47 100644
>> --- a/fs/ntfs/super.c
>> +++ b/fs/ntfs/super.c
>> @@ -2204,17 +2204,12 @@ get_ctx_vol_failed:
>> return true;
>> #ifdef NTFS_RW
>> iput_usnjrnl_err_out:
>> - if (vol->usnjrnl_j_ino)
>> - iput(vol->usnjrnl_j_ino);
>> - if (vol->usnjrnl_max_ino)
>> - iput(vol->usnjrnl_max_ino);
>> - if (vol->usnjrnl_ino)
>> - iput(vol->usnjrnl_ino);
>> + iput(vol->usnjrnl_j_ino);
>> + iput(vol->usnjrnl_max_ino);
>> + iput(vol->usnjrnl_ino);
>> iput_quota_err_out:
>> - if (vol->quota_q_ino)
>> - iput(vol->quota_q_ino);
>> - if (vol->quota_ino)
>> - iput(vol->quota_ino);
>> + iput(vol->quota_q_ino);
>> + iput(vol->quota_ino);
>> iput(vol->extend_ino);
>> #endif /* NTFS_RW */
>> iput_sec_err_out:
>> @@ -2223,8 +2218,7 @@ iput_root_err_out:
>> iput(vol->root_ino);
>> iput_logfile_err_out:
>> #ifdef NTFS_RW
>> - if (vol->logfile_ino)
>> - iput(vol->logfile_ino);
>> + iput(vol->logfile_ino);
>> iput_vol_err_out:
>> #endif /* NTFS_RW */
>> iput(vol->vol_ino);
>> @@ -2254,8 +2248,7 @@ iput_mftbmp_err_out:
>> iput(vol->mftbmp_ino);
>> iput_mirr_err_out:
>> #ifdef NTFS_RW
>> - if (vol->mftmirr_ino)
>> - iput(vol->mftmirr_ino);
>> + iput(vol->mftmirr_ino);
>> #endif /* NTFS_RW */
>> return false;
>> }
>>
>
> Would you like to integrate this update suggestion
> into another source code repository?
>
> Regards,
> Markus

--
Anton Altaparmakov <anton at tuxera.com> (replace at with @)
Lead in File System Development, Tuxera Inc., http://www.tuxera.com/
Linux NTFS maintainer

2015-07-06 21:50:18

by Andrew Morton

Subject: Re: [PATCH] ntfs: Deletion of unnecessary checks before the function call "iput"

On Sat, 4 Jul 2015 10:36:57 +0000 Anton Altaparmakov <[email protected]> wrote:

> Hi Andrew,
>
> Can you please take up this trivial patch and merge it upstream?
>
> Reviewed-by: Anton Altaparmakov <[email protected]>
>

Nobody responded to Julia's review comment: "I don't have time to look
at the code now, but since there is an exit label here, have you
checked whether you could improve the gotos in these cases?"

She has a good point. We can either change those gotos to go to the
correct place (current code is rather bizarre) or we can simply do

--- a/fs/ntfs/super.c~a
+++ a/fs/ntfs/super.c
@@ -2202,36 +2202,26 @@ get_ctx_vol_failed:
}
#endif /* NTFS_RW */
return true;
+out:
#ifdef NTFS_RW
-iput_usnjrnl_err_out:
iput(vol->usnjrnl_j_ino);
iput(vol->usnjrnl_max_ino);
iput(vol->usnjrnl_ino);
-iput_quota_err_out:
iput(vol->quota_q_ino);
iput(vol->quota_ino);
iput(vol->extend_ino);
#endif /* NTFS_RW */
-iput_sec_err_out:
iput(vol->secure_ino);
-iput_root_err_out:
iput(vol->root_ino);
-iput_logfile_err_out:
#ifdef NTFS_RW
iput(vol->logfile_ino);
-iput_vol_err_out:
#endif /* NTFS_RW */
iput(vol->vol_ino);
-iput_lcnbmp_err_out:
iput(vol->lcnbmp_ino);
-iput_attrdef_err_out:
vol->attrdef_size = 0;
- if (vol->attrdef) {
- ntfs_free(vol->attrdef);
- vol->attrdef = NULL;
- }
+ ntfs_free(vol->attrdef);
+ vol->attrdef = NULL;
#ifdef NTFS_RW
-iput_upcase_err_out:
#endif /* NTFS_RW */
vol->upcase_len = 0;
mutex_lock(&ntfs_lock);
@@ -2246,7 +2236,6 @@ iput_upcase_err_out:
}
iput_mftbmp_err_out:
iput(vol->mftbmp_ino);
-iput_mirr_err_out:
#ifdef NTFS_RW
iput(vol->mftmirr_ino);
#endif /* NTFS_RW */

(note that ntfs_free(NULL) is OK)

then change all the appropriate gotos to good old "goto out;".

Or we can not bother ;)