[PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Daeseok Youn]

The use of 'status' in __ocfs2_add_entry() can return wrong
status when some functions are failed.

If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
that status is saved to 'status' but return variable is 'retval'
which is saved 'success' status. In case of this,  __ocfs2_add_entry()
is failed but can be returned as 'success'.

So replace 'status' with 'retval'.

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
V2 : update changelog

 fs/ocfs2/dir.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index b08050b..1478a50 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
 	struct ocfs2_dir_entry *de, *de1;
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
 	struct super_block *sb = dir->i_sb;
-	int retval, status;
+	int retval;
 	unsigned int size = sb->s_blocksize;
 	struct buffer_head *insert_bh = lookup->dl_leaf_bh;
 	char *data_start = insert_bh->b_data;
@@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
 			}
 
 			if (insert_bh == parent_fe_bh)
-				status = ocfs2_journal_access_di(handle,
+				retval = ocfs2_journal_access_di(handle,
 								 INODE_CACHE(dir),
 								 insert_bh,
 								 OCFS2_JOURNAL_ACCESS_WRITE);
 			else {
-				status = ocfs2_journal_access_db(handle,
+				retval = ocfs2_journal_access_db(handle,
 								 INODE_CACHE(dir),
 								 insert_bh,
 					      OCFS2_JOURNAL_ACCESS_WRITE);
 
 				if (ocfs2_dir_indexed(dir)) {
-					status = ocfs2_dx_dir_insert(dir,
+					retval = ocfs2_dx_dir_insert(dir,
 								handle,
 								lookup);
-					if (status) {
-						mlog_errno(status);
+					if (retval) {
+						mlog_errno(retval);
 						goto bail;
 					}
 				}
-- 
1.7.1

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Daeseok Youn]

The use of 'status' in __ocfs2_add_entry() can return wrong
status when some functions are failed.

If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
that status is saved to 'status' but return variable is 'retval'
which is saved 'success' status. In case of this,  __ocfs2_add_entry()
is failed but can be returned as 'success'.

So replace 'status' with 'retval'.

Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
---
V2 : update changelog

 fs/ocfs2/dir.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index b08050b..1478a50 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
 	struct ocfs2_dir_entry *de, *de1;
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
 	struct super_block *sb = dir->i_sb;
-	int retval, status;
+	int retval;
 	unsigned int size = sb->s_blocksize;
 	struct buffer_head *insert_bh = lookup->dl_leaf_bh;
 	char *data_start = insert_bh->b_data;
@@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
 			}
 
 			if (insert_bh == parent_fe_bh)
-				status = ocfs2_journal_access_di(handle,
+				retval = ocfs2_journal_access_di(handle,
 								 INODE_CACHE(dir),
 								 insert_bh,
 								 OCFS2_JOURNAL_ACCESS_WRITE);
 			else {
-				status = ocfs2_journal_access_db(handle,
+				retval = ocfs2_journal_access_db(handle,
 								 INODE_CACHE(dir),
 								 insert_bh,
 					      OCFS2_JOURNAL_ACCESS_WRITE);
 
 				if (ocfs2_dir_indexed(dir)) {
-					status = ocfs2_dx_dir_insert(dir,
+					retval = ocfs2_dx_dir_insert(dir,
 								handle,
 								lookup);
-					if (status) {
-						mlog_errno(status);
+					if (retval) {
+						mlog_errno(retval);
 						goto bail;
 					}
 				}
-- 
1.7.1

Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Richard Weinberger]

Am 28.02.2015 um 00:48 schrieb Daeseok Youn:
> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
> 
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
> 
> So replace 'status' with 'retval'.

As this patch is untested and the issue is theoretical I'm nervous.
But the final decision is up to ocfs2 maintainers.

Thanks,
//richard

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Richard Weinberger]

Am 28.02.2015 um 00:48 schrieb Daeseok Youn:
> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
> 
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
> 
> So replace 'status' with 'retval'.

As this patch is untested and the issue is theoretical I'm nervous.
But the final decision is up to ocfs2 maintainers.

Thanks,
//richard

Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi, all

please, review this patch.

thanks.

regards,
Daeseok Youn.

2015-03-02 18:04 GMT+09:00 Richard Weinberger <richard@nod.at>:
> Am 28.02.2015 um 00:48 schrieb Daeseok Youn:
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>
> As this patch is untested and the issue is theoretical I'm nervous.
> But the final decision is up to ocfs2 maintainers.
>
> Thanks,
> //richard

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi, all

please, review this patch.

thanks.

regards,
Daeseok Youn.

2015-03-02 18:04 GMT+09:00 Richard Weinberger <richard@nod.at>:
> Am 28.02.2015 um 00:48 schrieb Daeseok Youn:
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>
> As this patch is untested and the issue is theoretical I'm nervous.
> But the final decision is up to ocfs2 maintainers.
>
> Thanks,
> //richard

Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi,

How is this patch going on?

please check for me.

Thanks.
Daeseok Youn.

2015-03-03 10:38 GMT+09:00 DaeSeok Youn <daeseok.youn@gmail.com>:
> Hi, all
>
> please, review this patch.
>
> thanks.
>
> regards,
> Daeseok Youn.
>
> 2015-03-02 18:04 GMT+09:00 Richard Weinberger <richard@nod.at>:
>> Am 28.02.2015 um 00:48 schrieb Daeseok Youn:
>>> The use of 'status' in __ocfs2_add_entry() can return wrong
>>> status when some functions are failed.
>>>
>>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>>> that status is saved to 'status' but return variable is 'retval'
>>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>>> is failed but can be returned as 'success'.
>>>
>>> So replace 'status' with 'retval'.
>>
>> As this patch is untested and the issue is theoretical I'm nervous.
>> But the final decision is up to ocfs2 maintainers.
>>
>> Thanks,
>> //richard

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi,

How is this patch going on?

please check for me.

Thanks.
Daeseok Youn.

2015-03-03 10:38 GMT+09:00 DaeSeok Youn <daeseok.youn@gmail.com>:
> Hi, all
>
> please, review this patch.
>
> thanks.
>
> regards,
> Daeseok Youn.
>
> 2015-03-02 18:04 GMT+09:00 Richard Weinberger <richard@nod.at>:
>> Am 28.02.2015 um 00:48 schrieb Daeseok Youn:
>>> The use of 'status' in __ocfs2_add_entry() can return wrong
>>> status when some functions are failed.
>>>
>>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>>> that status is saved to 'status' but return variable is 'retval'
>>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>>> is failed but can be returned as 'success'.
>>>
>>> So replace 'status' with 'retval'.
>>
>> As this patch is untested and the issue is theoretical I'm nervous.
>> But the final decision is up to ocfs2 maintainers.
>>
>> Thanks,
>> //richard

Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi,

This patch have been pending for 2 weeks.
Do I need to check other things?

please, check for me.

Thanks!

regards,
Daeseok Youn

2015-02-28 8:48 GMT+09:00 Daeseok Youn <daeseok.youn@gmail.com>:
> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
>
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
>
> So replace 'status' with 'retval'.
>
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> V2 : update changelog
>
>  fs/ocfs2/dir.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index b08050b..1478a50 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>         struct ocfs2_dir_entry *de, *de1;
>         struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>         struct super_block *sb = dir->i_sb;
> -       int retval, status;
> +       int retval;
>         unsigned int size = sb->s_blocksize;
>         struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>         char *data_start = insert_bh->b_data;
> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>                         }
>
>                         if (insert_bh == parent_fe_bh)
> -                               status = ocfs2_journal_access_di(handle,
> +                               retval = ocfs2_journal_access_di(handle,
>                                                                  INODE_CACHE(dir),
>                                                                  insert_bh,
>                                                                  OCFS2_JOURNAL_ACCESS_WRITE);
>                         else {
> -                               status = ocfs2_journal_access_db(handle,
> +                               retval = ocfs2_journal_access_db(handle,
>                                                                  INODE_CACHE(dir),
>                                                                  insert_bh,
>                                               OCFS2_JOURNAL_ACCESS_WRITE);
>
>                                 if (ocfs2_dir_indexed(dir)) {
> -                                       status = ocfs2_dx_dir_insert(dir,
> +                                       retval = ocfs2_dx_dir_insert(dir,
>                                                                 handle,
>                                                                 lookup);
> -                                       if (status) {
> -                                               mlog_errno(status);
> +                                       if (retval) {
> +                                               mlog_errno(retval);
>                                                 goto bail;
>                                         }
>                                 }
> --
> 1.7.1
>

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi,

This patch have been pending for 2 weeks.
Do I need to check other things?

please, check for me.

Thanks!

regards,
Daeseok Youn

2015-02-28 8:48 GMT+09:00 Daeseok Youn <daeseok.youn@gmail.com>:
> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
>
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
>
> So replace 'status' with 'retval'.
>
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> V2 : update changelog
>
>  fs/ocfs2/dir.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index b08050b..1478a50 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>         struct ocfs2_dir_entry *de, *de1;
>         struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>         struct super_block *sb = dir->i_sb;
> -       int retval, status;
> +       int retval;
>         unsigned int size = sb->s_blocksize;
>         struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>         char *data_start = insert_bh->b_data;
> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>                         }
>
>                         if (insert_bh == parent_fe_bh)
> -                               status = ocfs2_journal_access_di(handle,
> +                               retval = ocfs2_journal_access_di(handle,
>                                                                  INODE_CACHE(dir),
>                                                                  insert_bh,
>                                                                  OCFS2_JOURNAL_ACCESS_WRITE);
>                         else {
> -                               status = ocfs2_journal_access_db(handle,
> +                               retval = ocfs2_journal_access_db(handle,
>                                                                  INODE_CACHE(dir),
>                                                                  insert_bh,
>                                               OCFS2_JOURNAL_ACCESS_WRITE);
>
>                                 if (ocfs2_dir_indexed(dir)) {
> -                                       status = ocfs2_dx_dir_insert(dir,
> +                                       retval = ocfs2_dx_dir_insert(dir,
>                                                                 handle,
>                                                                 lookup);
> -                                       if (status) {
> -                                               mlog_errno(status);
> +                                       if (retval) {
> +                                               mlog_errno(retval);
>                                                 goto bail;
>                                         }
>                                 }
> --
> 1.7.1
>

Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Richard Weinberger]

Am 13.03.2015 um 04:59 schrieb DaeSeok Youn:
> Hi,
> 
> This patch have been pending for 2 weeks.
> Do I need to check other things?
> 
> please, check for me.

I cannot speak for OCFS2 folks. But maybe you can give them a
better feeling if you proof that your patch is tested.

Thanks,
//richard

> Thanks!
> 
> regards,
> Daeseok Youn
> 
> 2015-02-28 8:48 GMT+09:00 Daeseok Youn <daeseok.youn@gmail.com>:
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>> ---
>> V2 : update changelog
>>
>>  fs/ocfs2/dir.c |   12 ++++++------
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
>> index b08050b..1478a50 100644
>> --- a/fs/ocfs2/dir.c
>> +++ b/fs/ocfs2/dir.c
>> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>>         struct ocfs2_dir_entry *de, *de1;
>>         struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>>         struct super_block *sb = dir->i_sb;
>> -       int retval, status;
>> +       int retval;
>>         unsigned int size = sb->s_blocksize;
>>         struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>>         char *data_start = insert_bh->b_data;
>> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>>                         }
>>
>>                         if (insert_bh == parent_fe_bh)
>> -                               status = ocfs2_journal_access_di(handle,
>> +                               retval = ocfs2_journal_access_di(handle,
>>                                                                  INODE_CACHE(dir),
>>                                                                  insert_bh,
>>                                                                  OCFS2_JOURNAL_ACCESS_WRITE);
>>                         else {
>> -                               status = ocfs2_journal_access_db(handle,
>> +                               retval = ocfs2_journal_access_db(handle,
>>                                                                  INODE_CACHE(dir),
>>                                                                  insert_bh,
>>                                               OCFS2_JOURNAL_ACCESS_WRITE);
>>
>>                                 if (ocfs2_dir_indexed(dir)) {
>> -                                       status = ocfs2_dx_dir_insert(dir,
>> +                                       retval = ocfs2_dx_dir_insert(dir,
>>                                                                 handle,
>>                                                                 lookup);
>> -                                       if (status) {
>> -                                               mlog_errno(status);
>> +                                       if (retval) {
>> +                                               mlog_errno(retval);
>>                                                 goto bail;
>>                                         }
>>                                 }
>> --
>> 1.7.1
>>

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Richard Weinberger]

Am 13.03.2015 um 04:59 schrieb DaeSeok Youn:
> Hi,
> 
> This patch have been pending for 2 weeks.
> Do I need to check other things?
> 
> please, check for me.

I cannot speak for OCFS2 folks. But maybe you can give them a
better feeling if you proof that your patch is tested.

Thanks,
//richard

> Thanks!
> 
> regards,
> Daeseok Youn
> 
> 2015-02-28 8:48 GMT+09:00 Daeseok Youn <daeseok.youn@gmail.com>:
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>> ---
>> V2 : update changelog
>>
>>  fs/ocfs2/dir.c |   12 ++++++------
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
>> index b08050b..1478a50 100644
>> --- a/fs/ocfs2/dir.c
>> +++ b/fs/ocfs2/dir.c
>> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>>         struct ocfs2_dir_entry *de, *de1;
>>         struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>>         struct super_block *sb = dir->i_sb;
>> -       int retval, status;
>> +       int retval;
>>         unsigned int size = sb->s_blocksize;
>>         struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>>         char *data_start = insert_bh->b_data;
>> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>>                         }
>>
>>                         if (insert_bh == parent_fe_bh)
>> -                               status = ocfs2_journal_access_di(handle,
>> +                               retval = ocfs2_journal_access_di(handle,
>>                                                                  INODE_CACHE(dir),
>>                                                                  insert_bh,
>>                                                                  OCFS2_JOURNAL_ACCESS_WRITE);
>>                         else {
>> -                               status = ocfs2_journal_access_db(handle,
>> +                               retval = ocfs2_journal_access_db(handle,
>>                                                                  INODE_CACHE(dir),
>>                                                                  insert_bh,
>>                                               OCFS2_JOURNAL_ACCESS_WRITE);
>>
>>                                 if (ocfs2_dir_indexed(dir)) {
>> -                                       status = ocfs2_dx_dir_insert(dir,
>> +                                       retval = ocfs2_dx_dir_insert(dir,
>>                                                                 handle,
>>                                                                 lookup);
>> -                                       if (status) {
>> -                                               mlog_errno(status);
>> +                                       if (retval) {
>> +                                               mlog_errno(retval);
>>                                                 goto bail;
>>                                         }
>>                                 }
>> --
>> 1.7.1
>>

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi, Richard.

Thanks for reply.
Ok. I will keep trying to set the environment for verifying my paches.
But, before I said, it takes some time... Setting this filesystem in my
machine with qemu is too hard for me. :(

Regards,
Daeseok Youn.

2015. 3. 13. ?? 5:16? "Richard Weinberger" <richard@nod.at>?? ??:
>
> Am 13.03.2015 um 04:59 schrieb DaeSeok Youn:
> > Hi,
> >
> > This patch have been pending for 2 weeks.
> > Do I need to check other things?
> >
> > please, check for me.
>
> I cannot speak for OCFS2 folks. But maybe you can give them a
> better feeling if you proof that your patch is tested.
>
> Thanks,
> //richard
>
> > Thanks!
> >
> > regards,
> > Daeseok Youn
> >
> > 2015-02-28 8:48 GMT+09:00 Daeseok Youn <daeseok.youn@gmail.com>:
> >> The use of 'status' in __ocfs2_add_entry() can return wrong
> >> status when some functions are failed.
> >>
> >> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> >> that status is saved to 'status' but return variable is 'retval'
> >> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> >> is failed but can be returned as 'success'.
> >>
> >> So replace 'status' with 'retval'.
> >>
> >> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> >> ---
> >> V2 : update changelog
> >>
> >>  fs/ocfs2/dir.c |   12 ++++++------
> >>  1 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> >> index b08050b..1478a50 100644
> >> --- a/fs/ocfs2/dir.c
> >> +++ b/fs/ocfs2/dir.c
> >> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
> >>         struct ocfs2_dir_entry *de, *de1;
> >>         struct ocfs2_dinode *di = (struct ocfs2_dinode
*)parent_fe_bh->b_data;
> >>         struct super_block *sb = dir->i_sb;
> >> -       int retval, status;
> >> +       int retval;
> >>         unsigned int size = sb->s_blocksize;
> >>         struct buffer_head *insert_bh = lookup->dl_leaf_bh;
> >>         char *data_start = insert_bh->b_data;
> >> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
> >>                         }
> >>
> >>                         if (insert_bh == parent_fe_bh)
> >> -                               status =
ocfs2_journal_access_di(handle,
> >> +                               retval =
ocfs2_journal_access_di(handle,
> >>
INODE_CACHE(dir),
> >>
insert_bh,
> >>
OCFS2_JOURNAL_ACCESS_WRITE);
> >>                         else {
> >> -                               status =
ocfs2_journal_access_db(handle,
> >> +                               retval =
ocfs2_journal_access_db(handle,
> >>
INODE_CACHE(dir),
> >>
insert_bh,
> >>
 OCFS2_JOURNAL_ACCESS_WRITE);
> >>
> >>                                 if (ocfs2_dir_indexed(dir)) {
> >> -                                       status =
ocfs2_dx_dir_insert(dir,
> >> +                                       retval =
ocfs2_dx_dir_insert(dir,
> >>                                                                 handle,
> >>
 lookup);
> >> -                                       if (status) {
> >> -                                               mlog_errno(status);
> >> +                                       if (retval) {
> >> +                                               mlog_errno(retval);
> >>                                                 goto bail;
> >>                                         }
> >>                                 }
> >> --
> >> 1.7.1
> >>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20150313/7c2e5ba8/attachment.html 

Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Joseph Qi]

Looks good to me.

On 2015/2/28 7:48, Daeseok Youn wrote:
> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
> 
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
> 
> So replace 'status' with 'retval'.
> 
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>

Reviewed-by: Joseph Qi <joseph.qi@huawei.com>
> ---
> V2 : update changelog
> 
>  fs/ocfs2/dir.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index b08050b..1478a50 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>  	struct ocfs2_dir_entry *de, *de1;
>  	struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>  	struct super_block *sb = dir->i_sb;
> -	int retval, status;
> +	int retval;
>  	unsigned int size = sb->s_blocksize;
>  	struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>  	char *data_start = insert_bh->b_data;
> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>  			}
>  
>  			if (insert_bh == parent_fe_bh)
> -				status = ocfs2_journal_access_di(handle,
> +				retval = ocfs2_journal_access_di(handle,
>  								 INODE_CACHE(dir),
>  								 insert_bh,
>  								 OCFS2_JOURNAL_ACCESS_WRITE);
>  			else {
> -				status = ocfs2_journal_access_db(handle,
> +				retval = ocfs2_journal_access_db(handle,
>  								 INODE_CACHE(dir),
>  								 insert_bh,
>  					      OCFS2_JOURNAL_ACCESS_WRITE);
>  
>  				if (ocfs2_dir_indexed(dir)) {
> -					status = ocfs2_dx_dir_insert(dir,
> +					retval = ocfs2_dx_dir_insert(dir,
>  								handle,
>  								lookup);
> -					if (status) {
> -						mlog_errno(status);
> +					if (retval) {
> +						mlog_errno(retval);
>  						goto bail;
>  					}
>  				}
> 

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Joseph Qi]

Looks good to me.

On 2015/2/28 7:48, Daeseok Youn wrote:
> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
> 
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
> 
> So replace 'status' with 'retval'.
> 
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>

Reviewed-by: Joseph Qi <joseph.qi@huawei.com>
> ---
> V2 : update changelog
> 
>  fs/ocfs2/dir.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
> index b08050b..1478a50 100644
> --- a/fs/ocfs2/dir.c
> +++ b/fs/ocfs2/dir.c
> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>  	struct ocfs2_dir_entry *de, *de1;
>  	struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>  	struct super_block *sb = dir->i_sb;
> -	int retval, status;
> +	int retval;
>  	unsigned int size = sb->s_blocksize;
>  	struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>  	char *data_start = insert_bh->b_data;
> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>  			}
>  
>  			if (insert_bh == parent_fe_bh)
> -				status = ocfs2_journal_access_di(handle,
> +				retval = ocfs2_journal_access_di(handle,
>  								 INODE_CACHE(dir),
>  								 insert_bh,
>  								 OCFS2_JOURNAL_ACCESS_WRITE);
>  			else {
> -				status = ocfs2_journal_access_db(handle,
> +				retval = ocfs2_journal_access_db(handle,
>  								 INODE_CACHE(dir),
>  								 insert_bh,
>  					      OCFS2_JOURNAL_ACCESS_WRITE);
>  
>  				if (ocfs2_dir_indexed(dir)) {
> -					status = ocfs2_dx_dir_insert(dir,
> +					retval = ocfs2_dx_dir_insert(dir,
>  								handle,
>  								lookup);
> -					if (status) {
> -						mlog_errno(status);
> +					if (retval) {
> +						mlog_errno(retval);
>  						goto bail;
>  					}
>  				}
> 

Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Andrew Morton]

On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn@gmail.com> wrote:

> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
> 
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
> 
> So replace 'status' with 'retval'.
> 
> -						mlog_errno(status);
> +					if (retval) {
> +						mlog_errno(retval);
>  						goto bail;

and

bail:
	if (retval)
		mlog_errno(retval);

	return retval;
}

so we'll clearly log the same error twice.

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Andrew Morton]

On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn@gmail.com> wrote:

> The use of 'status' in __ocfs2_add_entry() can return wrong
> status when some functions are failed.
> 
> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
> that status is saved to 'status' but return variable is 'retval'
> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
> is failed but can be returned as 'success'.
> 
> So replace 'status' with 'retval'.
> 
> -						mlog_errno(status);
> +					if (retval) {
> +						mlog_errno(retval);
>  						goto bail;

and

bail:
	if (retval)
		mlog_errno(retval);

	return retval;
}

so we'll clearly log the same error twice.

Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Joseph Qi]

Hi Andrew,

On 2015/3/20 6:23, Andrew Morton wrote:
> On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn@gmail.com> wrote:
> 
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>>
>> -						mlog_errno(status);
>> +					if (retval) {
>> +						mlog_errno(retval);
>>  						goto bail;
> 
> and
> 
> bail:
> 	if (retval)
> 		mlog_errno(retval);
> 
> 	return retval;
> }
> 
> so we'll clearly log the same error twice.
> 
IMO, if we only depends on the bail error log, we still don't know where
the error occurs.
So if we want to do the cleanup, the bail error log should be cleaned.

> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
> 
> 

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[Joseph Qi]

Hi Andrew,

On 2015/3/20 6:23, Andrew Morton wrote:
> On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn@gmail.com> wrote:
> 
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>>
>> -						mlog_errno(status);
>> +					if (retval) {
>> +						mlog_errno(retval);
>>  						goto bail;
> 
> and
> 
> bail:
> 	if (retval)
> 		mlog_errno(retval);
> 
> 	return retval;
> }
> 
> so we'll clearly log the same error twice.
> 
IMO, if we only depends on the bail error log, we still don't know where
the error occurs.
So if we want to do the cleanup, the bail error log should be cleaned.

> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
> 
> 

Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

2015-03-19 15:24 GMT+09:00 Joseph Qi <joseph.qi@huawei.com>:
> Looks good to me.
>
> On 2015/2/28 7:48, Daeseok Youn wrote:
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>
> Reviewed-by: Joseph Qi <joseph.qi@huawei.com>
Hi, Joseph.

Thanks for review this patch. I have actually tried to set this file
system on my machine with qemu but I didn't success, yet.

Regards,
Daeseok Youn.

>> ---
>> V2 : update changelog
>>
>>  fs/ocfs2/dir.c |   12 ++++++------
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
>> index b08050b..1478a50 100644
>> --- a/fs/ocfs2/dir.c
>> +++ b/fs/ocfs2/dir.c
>> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>>       struct ocfs2_dir_entry *de, *de1;
>>       struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>>       struct super_block *sb = dir->i_sb;
>> -     int retval, status;
>> +     int retval;
>>       unsigned int size = sb->s_blocksize;
>>       struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>>       char *data_start = insert_bh->b_data;
>> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>>                       }
>>
>>                       if (insert_bh == parent_fe_bh)
>> -                             status = ocfs2_journal_access_di(handle,
>> +                             retval = ocfs2_journal_access_di(handle,
>>                                                                INODE_CACHE(dir),
>>                                                                insert_bh,
>>                                                                OCFS2_JOURNAL_ACCESS_WRITE);
>>                       else {
>> -                             status = ocfs2_journal_access_db(handle,
>> +                             retval = ocfs2_journal_access_db(handle,
>>                                                                INODE_CACHE(dir),
>>                                                                insert_bh,
>>                                             OCFS2_JOURNAL_ACCESS_WRITE);
>>
>>                               if (ocfs2_dir_indexed(dir)) {
>> -                                     status = ocfs2_dx_dir_insert(dir,
>> +                                     retval = ocfs2_dx_dir_insert(dir,
>>                                                               handle,
>>                                                               lookup);
>> -                                     if (status) {
>> -                                             mlog_errno(status);
>> +                                     if (retval) {
>> +                                             mlog_errno(retval);
>>                                               goto bail;
>>                                       }
>>                               }
>>
>
>

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

2015-03-19 15:24 GMT+09:00 Joseph Qi <joseph.qi@huawei.com>:
> Looks good to me.
>
> On 2015/2/28 7:48, Daeseok Youn wrote:
>> The use of 'status' in __ocfs2_add_entry() can return wrong
>> status when some functions are failed.
>>
>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>> that status is saved to 'status' but return variable is 'retval'
>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>> is failed but can be returned as 'success'.
>>
>> So replace 'status' with 'retval'.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
>
> Reviewed-by: Joseph Qi <joseph.qi@huawei.com>
Hi, Joseph.

Thanks for review this patch. I have actually tried to set this file
system on my machine with qemu but I didn't success, yet.

Regards,
Daeseok Youn.

>> ---
>> V2 : update changelog
>>
>>  fs/ocfs2/dir.c |   12 ++++++------
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
>> index b08050b..1478a50 100644
>> --- a/fs/ocfs2/dir.c
>> +++ b/fs/ocfs2/dir.c
>> @@ -1617,7 +1617,7 @@ int __ocfs2_add_entry(handle_t *handle,
>>       struct ocfs2_dir_entry *de, *de1;
>>       struct ocfs2_dinode *di = (struct ocfs2_dinode *)parent_fe_bh->b_data;
>>       struct super_block *sb = dir->i_sb;
>> -     int retval, status;
>> +     int retval;
>>       unsigned int size = sb->s_blocksize;
>>       struct buffer_head *insert_bh = lookup->dl_leaf_bh;
>>       char *data_start = insert_bh->b_data;
>> @@ -1695,22 +1695,22 @@ int __ocfs2_add_entry(handle_t *handle,
>>                       }
>>
>>                       if (insert_bh == parent_fe_bh)
>> -                             status = ocfs2_journal_access_di(handle,
>> +                             retval = ocfs2_journal_access_di(handle,
>>                                                                INODE_CACHE(dir),
>>                                                                insert_bh,
>>                                                                OCFS2_JOURNAL_ACCESS_WRITE);
>>                       else {
>> -                             status = ocfs2_journal_access_db(handle,
>> +                             retval = ocfs2_journal_access_db(handle,
>>                                                                INODE_CACHE(dir),
>>                                                                insert_bh,
>>                                             OCFS2_JOURNAL_ACCESS_WRITE);
>>
>>                               if (ocfs2_dir_indexed(dir)) {
>> -                                     status = ocfs2_dx_dir_insert(dir,
>> +                                     retval = ocfs2_dx_dir_insert(dir,
>>                                                               handle,
>>                                                               lookup);
>> -                                     if (status) {
>> -                                             mlog_errno(status);
>> +                                     if (retval) {
>> +                                             mlog_errno(retval);
>>                                               goto bail;
>>                                       }
>>                               }
>>
>
>

Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi, Andrew.

2015-03-20 10:17 GMT+09:00 Joseph Qi <joseph.qi@huawei.com>:
> Hi Andrew,
>
> On 2015/3/20 6:23, Andrew Morton wrote:
>> On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn@gmail.com> wrote:
>>
>>> The use of 'status' in __ocfs2_add_entry() can return wrong
>>> status when some functions are failed.
>>>
>>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>>> that status is saved to 'status' but return variable is 'retval'
>>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>>> is failed but can be returned as 'success'.
>>>
>>> So replace 'status' with 'retval'.
>>>
>>> -                                            mlog_errno(status);
>>> +                                    if (retval) {
>>> +                                            mlog_errno(retval);
>>>                                              goto bail;
>>
>> and
>>
>> bail:
>>       if (retval)
>>               mlog_errno(retval);
>>
>>       return retval;
>> }
>>
>> so we'll clearly log the same error twice.

As joseph comment, it doesn't need to cleanup mlog_errno() call in this patch.
And he reviewed this patch.

Check, please.

Thanks.
Daeseok Youn
>>
> IMO, if we only depends on the bail error log, we still don't know where
> the error occurs.
> So if we want to do the cleanup, the bail error log should be cleaned.
>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel@oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>
>>
>
>

[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error

[DaeSeok Youn]

Hi, Andrew.

2015-03-20 10:17 GMT+09:00 Joseph Qi <joseph.qi@huawei.com>:
> Hi Andrew,
>
> On 2015/3/20 6:23, Andrew Morton wrote:
>> On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn@gmail.com> wrote:
>>
>>> The use of 'status' in __ocfs2_add_entry() can return wrong
>>> status when some functions are failed.
>>>
>>> If ocfs2_journal_access_db() in __ocfs2_add_entry() is failed,
>>> that status is saved to 'status' but return variable is 'retval'
>>> which is saved 'success' status. In case of this,  __ocfs2_add_entry()
>>> is failed but can be returned as 'success'.
>>>
>>> So replace 'status' with 'retval'.
>>>
>>> -                                            mlog_errno(status);
>>> +                                    if (retval) {
>>> +                                            mlog_errno(retval);
>>>                                              goto bail;
>>
>> and
>>
>> bail:
>>       if (retval)
>>               mlog_errno(retval);
>>
>>       return retval;
>> }
>>
>> so we'll clearly log the same error twice.

As joseph comment, it doesn't need to cleanup mlog_errno() call in this patch.
And he reviewed this patch.

Check, please.

Thanks.
Daeseok Youn
>>
> IMO, if we only depends on the bail error log, we still don't know where
> the error occurs.
> So if we want to do the cleanup, the bail error log should be cleaned.
>
>> _______________________________________________
>> Ocfs2-devel mailing list
>> Ocfs2-devel at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>
>>
>
>