diff mbox series

ext4: remove set but rewrite variables

Message ID 1621220165-11849-1-git-send-email-tiantao6@hisilicon.com
State Superseded
Headers show
Series ext4: remove set but rewrite variables | expand

Commit Message

tiantao \(H\) May 17, 2021, 2:56 a.m. UTC
In line 2500 of the ext4_dx_add_entry function, the at variable is
assigned but not used, and it is reassigned in line 2449, so delete
the assignment of the at variable in line 2500.

Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
---
 fs/ext4/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Artem Blagodarenko May 17, 2021, 8:05 a.m. UTC | #1
> On 17 May 2021, at 05:56, Tian Tao <tiantao6@hisilicon.com> wrote:
> 
> In line 2500 of the ext4_dx_add_entry function, the at variable is
> assigned but not used, and it is reassigned in line 2449, so delete
> the assignment of the at variable in line 2500.
> 
> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
> ---
> fs/ext4/namei.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index afb9d05..18bbf15 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2497,7 +2497,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
> 
> 			/* Which index block gets the new entry? */
> 			if (at - entries >= icount1) {
> -				frame->at = at = at - entries - icount1 + entries2;
> +				frame->at = at - entries - icount1 + entries2;
> 				frame->entries = entries = entries2;
> 				swap(frame->bh, bh2);
> 			}
> -- 
> 2.7.4
> 


Hello Tian,

Thank you for the patch. 

Please, clarify, do you think the logic not changed after you patch if "while (frame > frames) {“ loop is not executed or terminated by “break:” in " if (dx_get_count((frame - 1)->entries) < ...“ block?
Also I am not sure code lines numbers in the description are useful for future readers, because they can be changed.

Thank you.
tiantao (H) May 17, 2021, 8:44 a.m. UTC | #2
在 2021/5/17 16:05, Благодаренко Артём 写道:
>
>> On 17 May 2021, at 05:56, Tian Tao <tiantao6@hisilicon.com> wrote:
>>
>> In line 2500 of the ext4_dx_add_entry function, the at variable is
>> assigned but not used, and it is reassigned in line 2449, so delete
>> the assignment of the at variable in line 2500.
>>
>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>> ---
>> fs/ext4/namei.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
>> index afb9d05..18bbf15 100644
>> --- a/fs/ext4/namei.c
>> +++ b/fs/ext4/namei.c
>> @@ -2497,7 +2497,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
>>
>> 			/* Which index block gets the new entry? */
>> 			if (at - entries >= icount1) {
>> -				frame->at = at = at - entries - icount1 + entries2;
>> +				frame->at = at - entries - icount1 + entries2;
>> 				frame->entries = entries = entries2;
>> 				swap(frame->bh, bh2);
>> 			}
>> -- 
>> 2.7.4
>>
>
> Hello Tian,
>
> Thank you for the patch.
>
> Please, clarify, do you think the logic not changed after you patch if "while (frame > frames) {“ loop is not executed or terminated by “break:” in " if (dx_get_count((frame - 1)->entries) < ...“ block?

yes, it's reported by svace. and I check the code it's not change the 
logic. myabe I was wrong:-P

> Also I am not sure code lines numbers in the description are useful for future readers, because they can be changed.
I can update the commit message at v2.
>
> Thank you..
>
Artem Blagodarenko May 17, 2021, 9:18 a.m. UTC | #3
It looks like it is always reset just after “again:”  label. So, yes, your fix does not change the logic.

Feel free to add:
Reviewed-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>

Best regards,
Artem Blagodarenko
> On 17 May 2021, at 11:44, tiantao (H) <tiantao6@huawei.com> wrote:
> 
> 
> 在 2021/5/17 16:05, Благодаренко Артём 写道:
>> 
>>> On 17 May 2021, at 05:56, Tian Tao <tiantao6@hisilicon.com> wrote:
>>> 
>>> In line 2500 of the ext4_dx_add_entry function, the at variable is
>>> assigned but not used, and it is reassigned in line 2449, so delete
>>> the assignment of the at variable in line 2500.
>>> 
>>> Signed-off-by: Tian Tao <tiantao6@hisilicon.com>
>>> ---
>>> fs/ext4/namei.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
>>> index afb9d05..18bbf15 100644
>>> --- a/fs/ext4/namei.c
>>> +++ b/fs/ext4/namei.c
>>> @@ -2497,7 +2497,7 @@ static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
>>> 
>>> 			/* Which index block gets the new entry? */
>>> 			if (at - entries >= icount1) {
>>> -				frame->at = at = at - entries - icount1 + entries2;
>>> +				frame->at = at - entries - icount1 + entries2;
>>> 				frame->entries = entries = entries2;
>>> 				swap(frame->bh, bh2);
>>> 			}
>>> -- 
>>> 2.7.4
>>> 
>> 
>> Hello Tian,
>> 
>> Thank you for the patch.
>> 
>> Please, clarify, do you think the logic not changed after you patch if "while (frame > frames) {“ loop is not executed or terminated by “break:” in " if (dx_get_count((frame - 1)->entries) < ...“ block?
> 
> yes, it's reported by svace. and I check the code it's not change the logic. myabe I was wrong:-P
> 
>> Also I am not sure code lines numbers in the description are useful for future readers, because they can be changed.
> I can update the commit message at v2.
>> 
>> Thank you..
diff mbox series

Patch

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index afb9d05..18bbf15 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2497,7 +2497,7 @@  static int ext4_dx_add_entry(handle_t *handle, struct ext4_filename *fname,
 
 			/* Which index block gets the new entry? */
 			if (at - entries >= icount1) {
-				frame->at = at = at - entries - icount1 + entries2;
+				frame->at = at - entries - icount1 + entries2;
 				frame->entries = entries = entries2;
 				swap(frame->bh, bh2);
 			}