Andrew Morton
2015-Mar-19 22:23 UTC
[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn at 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.
Joseph Qi
2015-Mar-20 01:17 UTC
[Ocfs2-devel] [PATCH 1/4 V2] ocfs2: use retval instead of status for checking error
Hi Andrew, On 2015/3/20 6:23, Andrew Morton wrote:> On Sat, 28 Feb 2015 08:48:40 +0900 Daeseok Youn <daeseok.youn at 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 > >