LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
@ 2015-02-27 23:48 Daeseok Youn
  2015-03-02  9:04 ` Richard Weinberger
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Daeseok Youn @ 2015-02-27 23:48 UTC (permalink / raw)
  To: mfasheh, akpm; +Cc: jlbec, ocfs2-devel, linux-kernel, richard.weinberger

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-02-27 23:48 [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error Daeseok Youn
@ 2015-03-02  9:04 ` Richard Weinberger
  2015-03-03  1:38   ` DaeSeok Youn
  2015-03-13  3:59 ` DaeSeok Youn
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Richard Weinberger @ 2015-03-02  9:04 UTC (permalink / raw)
  To: Daeseok Youn, mfasheh, akpm; +Cc: jlbec, ocfs2-devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-03-02  9:04 ` Richard Weinberger
@ 2015-03-03  1:38   ` DaeSeok Youn
  2015-03-05  9:36     ` DaeSeok Youn
  0 siblings, 1 reply; 11+ messages in thread
From: DaeSeok Youn @ 2015-03-03  1:38 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: mfasheh, Andrew Morton, Joel Becker, ocfs2-devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-03-03  1:38   ` DaeSeok Youn
@ 2015-03-05  9:36     ` DaeSeok Youn
  0 siblings, 0 replies; 11+ messages in thread
From: DaeSeok Youn @ 2015-03-05  9:36 UTC (permalink / raw)
  To: Andrew Morton
  Cc: mfasheh, Richard Weinberger, Joel Becker, ocfs2-devel, linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-02-27 23:48 [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error Daeseok Youn
  2015-03-02  9:04 ` Richard Weinberger
@ 2015-03-13  3:59 ` DaeSeok Youn
  2015-03-13  8:15   ` Richard Weinberger
  2015-03-19  6:24 ` [Ocfs2-devel] " Joseph Qi
  2015-03-19 22:23 ` Andrew Morton
  3 siblings, 1 reply; 11+ messages in thread
From: DaeSeok Youn @ 2015-03-13  3:59 UTC (permalink / raw)
  To: mfasheh, Andrew Morton
  Cc: Joel Becker, ocfs2-devel, linux-kernel, Richard Weinberger

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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-03-13  3:59 ` DaeSeok Youn
@ 2015-03-13  8:15   ` Richard Weinberger
  0 siblings, 0 replies; 11+ messages in thread
From: Richard Weinberger @ 2015-03-13  8:15 UTC (permalink / raw)
  To: DaeSeok Youn, mfasheh, Andrew Morton
  Cc: Joel Becker, ocfs2-devel, linux-kernel

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
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-02-27 23:48 [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error Daeseok Youn
  2015-03-02  9:04 ` Richard Weinberger
  2015-03-13  3:59 ` DaeSeok Youn
@ 2015-03-19  6:24 ` Joseph Qi
  2015-03-20  2:22   ` DaeSeok Youn
  2015-03-19 22:23 ` Andrew Morton
  3 siblings, 1 reply; 11+ messages in thread
From: Joseph Qi @ 2015-03-19  6:24 UTC (permalink / raw)
  To: Daeseok Youn; +Cc: mfasheh, akpm, richard.weinberger, ocfs2-devel, linux-kernel

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;
>  					}
>  				}
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-02-27 23:48 [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error Daeseok Youn
                   ` (2 preceding siblings ...)
  2015-03-19  6:24 ` [Ocfs2-devel] " Joseph Qi
@ 2015-03-19 22:23 ` Andrew Morton
  2015-03-20  1:17   ` [Ocfs2-devel] " Joseph Qi
  3 siblings, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2015-03-19 22:23 UTC (permalink / raw)
  To: Daeseok Youn
  Cc: mfasheh, jlbec, ocfs2-devel, linux-kernel, richard.weinberger

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-03-19 22:23 ` Andrew Morton
@ 2015-03-20  1:17   ` Joseph Qi
  2015-03-24  6:17     ` DaeSeok Youn
  0 siblings, 1 reply; 11+ messages in thread
From: Joseph Qi @ 2015-03-20  1:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Daeseok Youn, richard.weinberger, mfasheh, ocfs2-devel, linux-kernel

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
> 
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-03-19  6:24 ` [Ocfs2-devel] " Joseph Qi
@ 2015-03-20  2:22   ` DaeSeok Youn
  0 siblings, 0 replies; 11+ messages in thread
From: DaeSeok Youn @ 2015-03-20  2:22 UTC (permalink / raw)
  To: Joseph Qi
  Cc: mfasheh, Andrew Morton, Richard Weinberger, ocfs2-devel, linux-kernel

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;
>>                                       }
>>                               }
>>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
  2015-03-20  1:17   ` [Ocfs2-devel] " Joseph Qi
@ 2015-03-24  6:17     ` DaeSeok Youn
  0 siblings, 0 replies; 11+ messages in thread
From: DaeSeok Youn @ 2015-03-24  6:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Richard Weinberger, mfasheh, ocfs2-devel, linux-kernel, Joseph Qi

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
>>
>>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-03-24  6:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-27 23:48 [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error Daeseok Youn
2015-03-02  9:04 ` Richard Weinberger
2015-03-03  1:38   ` DaeSeok Youn
2015-03-05  9:36     ` DaeSeok Youn
2015-03-13  3:59 ` DaeSeok Youn
2015-03-13  8:15   ` Richard Weinberger
2015-03-19  6:24 ` [Ocfs2-devel] " Joseph Qi
2015-03-20  2:22   ` DaeSeok Youn
2015-03-19 22:23 ` Andrew Morton
2015-03-20  1:17   ` [Ocfs2-devel] " Joseph Qi
2015-03-24  6:17     ` DaeSeok Youn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).