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

From: DaeSeok Youn
Date: Tue Mar 24 2015 - 02:17:26 EST


Hi, Andrew.

2015-03-20 10:17 GMT+09:00 Joseph Qi <joseph.qi@xxxxxxxxxx>:
> Hi Andrew,
>
> On 2015/3/20 6:23, Andrew Morton wrote:
>> On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn@xxxxxxxxx> 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@xxxxxxxxxxxxxx
>> https://oss.oracle.com/mailman/listinfo/ocfs2-devel
>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/