diff mbox

ocfs2_direct_IO_write misses ocfs2_is_overwrite error code

Message ID 55ECF26C.4000100@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Norton.Zhu Sept. 7, 2015, 2:11 a.m. UTC 
If ocfs2_is_overwrite failed, ocfs2_direct_IO_write mays till return success to the caller.

Signed-off-by: Norton.Zhu <norton.zhu@huawei.com>
---
 aops.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andrew Morton Sept. 11, 2015, 9:20 p.m. UTC | #1 
On Mon, 7 Sep 2015 10:11:56 +0800 "Norton.Zhu" <norton.zhu@huawei.com> wrote:

> If ocfs2_is_overwrite failed, ocfs2_direct_IO_write mays till return success to the caller.
> 
> ...
>
> --- a/aops.c
> +++ b/aops.c
> @@ -847,6 +847,7 @@ static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb,
>  		is_overwrite = ocfs2_is_overwrite(osb, inode, offset);
>  		if (is_overwrite < 0) {
>  			mlog_errno(is_overwrite);
> +			ret = is_overwrite;
>  			ocfs2_inode_unlock(inode, 1);
>  			goto clean_orphan;
>  		}

Looks OK.

We do `goto clean_orphan' and if (orphan), the code then proceeds to
overwrite the error code in `ret'.  This is odd, and probably wrong -
it's usually best to return the first-encountered error.
diff mbox

Patch

diff --git a/aops.c b/aops.c
index 0f5fd9d..4107279 100644
--- a/aops.c
+++ b/aops.c
@@ -847,6 +847,7 @@  static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb,
 		is_overwrite = ocfs2_is_overwrite(osb, inode, offset);
 		if (is_overwrite < 0) {
 			mlog_errno(is_overwrite);
+			ret = is_overwrite;
 			ocfs2_inode_unlock(inode, 1);
 			goto clean_orphan;
 		}