MDEV-19725 Incorrect error handling in ALTER TABLE

Some I/O functions and macros that are declared in os0file.h used to
return a Boolean status code (nonzero on success). In MySQL 5.7, they
were changed to return dberr_t instead. Alas, in MariaDB Server 10.2,
some uses of functions were not adjusted to the changed return value.

Until MDEV-19231, the valid values of dberr_t were always nonzero.
This means that some code that was incorrectly checking for a zero
return value from the functions would never detect a failure.

After MDEV-19231, some tests for ALTER ONLINE TABLE would fail with
cmake -DPLUGIN_PERFSCHEMA=NO. It turned out that the wrappers
pfs_os_file_read_no_error_handling_int_fd_func() and
pfs_os_file_write_int_fd_func() were wrongly returning
bool instead of dberr_t. Also the callers of these functions were
wrongly expecting bool (nonzero on success) instead of dberr_t.

This mistake had been made when the addition of these functions was
merged from MySQL 5.6.36 and 5.7.18 into MariaDB Server 10.2.7.

This fix also reverts commit 40becbc3
which attempted to work around the problem.

storage/innobase/include/os0file.h

@@ -2,7 +2,7 @@
 
 Copyright (c) 1995, 2017, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2009, Percona Inc.
-Copyright (c) 2013, 2017, MariaDB Corporation.
+Copyright (c) 2013, 2019, MariaDB Corporation.
 
 Portions of this file contain modifications contributed and copyrighted
 by Percona Inc.. Those modifications are
@@ -1197,12 +1197,12 @@ to original un-instrumented file I/O APIs */
 # define os_file_read_no_error_handling(type, file, buf, offset, n, o)	\
 	os_file_read_no_error_handling_func(type, file, buf, offset, n, o)
 # define os_file_read_no_error_handling_int_fd(type, file, buf, offset, n) \
-        (os_file_read_no_error_handling_func(type, OS_FILE_FROM_FD(file), buf, offset, n, NULL) == 0)
+	os_file_read_no_error_handling_func(type, OS_FILE_FROM_FD(file), buf, offset, n, NULL)
 
 # define os_file_write(type, name, file, buf, offset, n)	\
 	os_file_write_func(type, name, file, buf, offset, n)
 # define os_file_write_int_fd(type, name, file, buf, offset, n)	\
-        (os_file_write_func(type, name, OS_FILE_FROM_FD(file), buf, offset, n) == 0)
+	os_file_write_func(type, name, OS_FILE_FROM_FD(file), buf, offset, n)
 
 # define os_file_flush(file)	os_file_flush_func(file)
 

storage/innobase/include/os0file.ic

 /*****************************************************************************
 
 Copyright (c) 2010, 2017, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2013, 2017, MariaDB Corporation.
+Copyright (c) 2013, 2019, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -348,9 +348,10 @@ a synchronous read operation.
 @param[in]      n               number of bytes to read
 @param[in]      src_file        caller file name
 @param[in]      src_line        caller line number
-@return	   0 on error, 1 on success */
+@return error code
+@retval	DB_SUCCESS	if the operation succeeded */
 UNIV_INLINE
-bool
+dberr_t
 pfs_os_file_read_no_error_handling_int_fd_func(
 	const IORequest&	type,
 	int			file,
@@ -371,14 +372,14 @@ pfs_os_file_read_no_error_handling_int_fd_func(
 			__FILE__, __LINE__);
 	}
 
-	bool success = os_file_read_no_error_handling_func(
+	dberr_t err = os_file_read_no_error_handling_func(
 		type, OS_FILE_FROM_FD(file), buf, offset, n, NULL);
 
 	if (locker != NULL) {
 		PSI_FILE_CALL(end_file_wait)(locker, n);
 	}
 
-       return(success == DB_SUCCESS);  // Reverse result
+	return err;
 }
 
 /** NOTE! Please use the corresponding macro os_file_write(), not directly
@@ -435,9 +436,10 @@ os_file_write_int_fd() which requests a synchronous write operation.
 @param[in]	n		number of bytes
 @param[in]	src_file	file name where func invoked
 @param[in]	src_line	line where the func invoked
-@return	   0 on error, 1 on success */
+@return error code
+@retval	DB_SUCCESS	if the operation succeeded */
 UNIV_INLINE
-bool
+dberr_t
 pfs_os_file_write_int_fd_func(
 	const IORequest&	type,
 	const char*		name,
@@ -459,14 +461,14 @@ pfs_os_file_write_int_fd_func(
 			__FILE__, __LINE__);
 	}
 
-        bool success = os_file_write_func(
+	dberr_t err = os_file_write_func(
 		type, name, OS_FILE_FROM_FD(file), buf, offset, n);
 
         if (locker != NULL) {
                 PSI_FILE_CALL(end_file_wait)(locker, n);
         }
 
-        return(success == DB_SUCCESS); // Reverse result
+	return err;
 }
 
 /** NOTE! Please use the corresponding macro os_file_flush(), not directly

storage/innobase/include/row0merge.h

@@ -370,7 +370,9 @@ row_merge_buf_sort(
 
 /********************************************************************//**
 Write a merge block to the file system.
-@return whether the request was completed successfully */
+@return whether the request was completed successfully
+@retval	false	on error
+@retval	true	on success */
 UNIV_INTERN
 bool
 row_merge_write(

storage/innobase/os/os0file.cc

@@ -5038,7 +5038,8 @@ Requests a synchronous write operation.
 @param[out]	buf		buffer from which to write
 @param[in]	offset		file offset from the start where to read
 @param[in]	n		number of bytes to read, starting from offset
-@return DB_SUCCESS if request was successful, false if fail */
+@return error code
+@retval	DB_SUCCESS	if the operation succeeded */
 dberr_t
 os_file_write_func(
 	const IORequest&	type,
@@ -5497,7 +5498,8 @@ Requests a synchronous positioned read operation.
 @param[out]	buf		buffer where to read
 @param[in]	offset		file offset from the start where to read
 @param[in]	n		number of bytes to read, starting from offset
-@return DB_SUCCESS or error code */
+@return error code
+@retval	DB_SUCCESS	if the operation succeeded */
 dberr_t
 os_file_read_func(
 	const IORequest&	type,

storage/innobase/row/row0log.cc

@@ -396,7 +396,7 @@ row_log_online_op(
 		}
 
 		log->tail.blocks++;
-		if (!os_file_write_int_fd(
+		if (DB_SUCCESS != os_file_write_int_fd(
 			    request,
 			    "(modification log)",
 			    log->fd,
@@ -534,7 +534,7 @@ row_log_table_close_func(
 		}
 
 		log->tail.blocks++;
-		if (!os_file_write_int_fd(
+		if (DB_SUCCESS != os_file_write_int_fd(
 			    request,
 			    "(modification log)",
 			    log->fd,
@@ -2658,7 +2658,7 @@ row_log_table_apply_ops(
 		IORequest		request(IORequest::READ);
 		byte*			buf = index->online_log->head.block;
 
-		if (!os_file_read_no_error_handling_int_fd(
+		if (DB_SUCCESS != os_file_read_no_error_handling_int_fd(
 			    request, index->online_log->fd,
 			    buf, ofs, srv_sort_buf_size)) {
 			ib::error()
@@ -3529,7 +3529,7 @@ row_log_apply_ops(
 
 		byte*	buf = index->online_log->head.block;
 
-		if (!os_file_read_no_error_handling_int_fd(
+		if (DB_SUCCESS != os_file_read_no_error_handling_int_fd(
 			    request, index->online_log->fd,
 			    buf, ofs, srv_sort_buf_size)) {
 			ib::error()

storage/innobase/row/row0merge.cc

@@ -1087,8 +1087,9 @@ row_merge_read(
 	DBUG_EXECUTE_IF("row_merge_read_failure", DBUG_RETURN(FALSE););
 
 	IORequest	request(IORequest::READ);
-	const bool	success = os_file_read_no_error_handling_int_fd(
-		request, fd, buf, ofs, srv_sort_buf_size);
+	const bool	success = DB_SUCCESS
+		== os_file_read_no_error_handling_int_fd(
+			request, fd, buf, ofs, srv_sort_buf_size);
 
 	/* If encryption is enabled decrypt buffer */
 	if (success && log_tmp_is_encrypted()) {
@@ -1115,7 +1116,9 @@ row_merge_read(
 
 /********************************************************************//**
 Write a merge block to the file system.
-@return 0 on error, 1 if write succeded */
+@return whether the request was completed successfully
+@retval	false	on error
+@retval	true	on success */
 UNIV_INTERN
 bool
 row_merge_write(
@@ -1149,7 +1152,7 @@ row_merge_write(
 	}
 
 	IORequest	request(IORequest::WRITE);
-	const bool	success = os_file_write_int_fd(
+	const bool	success = DB_SUCCESS == os_file_write_int_fd(
 		request, "(merge)", fd, out_buf, ofs, buf_len);
 
 #ifdef POSIX_FADV_DONTNEED