Re: [Tarantool-patches] [PATCH 1/2] sql: add missing diag_set on failure when working with files inside SQL module

[Sergey Ostanevich <sergos@tarantool.org>]

Thanks for the patch!

I have a question if those diags are too low in the call stack?
Apparently, the unixOpen() is a single space the robust_open()
is called, so we’d have only one diag set instead of two?

Following this path - we should just cover UNIXVFS and sql_io_methods
members with diag set, given the errno is preserved.

By now the solution is partial, so can be applied only if we’re in a
rush.

Sergos

> On 8 Dec 2020, at 22:59, Leonid Vasiliev <lvasiliev@tarantool.org> wrote:
> 
> From: Mergen Imeev <imeevma@gmail.com>
> 
> SQL module didn't set an error in the diagnostics area on a file
> operation failure. This could lead to a crash like in #5537.
> 
> Part of #5537
> ---
> src/box/sql/os_unix.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/box/sql/os_unix.c b/src/box/sql/os_unix.c
> index b351c55..557d709 100644
> --- a/src/box/sql/os_unix.c
> +++ b/src/box/sql/os_unix.c
> @@ -159,14 +159,17 @@ robust_open(const char *z, int f, mode_t m)
> 		if (fd < 0) {
> 			if (errno == EINTR)
> 				continue;
> +			diag_set(SystemError, strerror(errno));
> 			break;
> 		}
> 		if (fd >= SQL_MINIMUM_FILE_DESCRIPTOR)
> 			break;
> 		close(fd);
> 		fd = -1;
> -		if (open("/dev/null", f, m) < 0)
> +		if (open("/dev/null", f, m) < 0) {
> +			diag_set(SystemError, strerror(errno));
> 			break;
> +		}
> 	}
> 	if (fd >= 0) {
> 		if (m != 0) {
> @@ -831,8 +834,10 @@ seekAndWriteFd(int fd,		/* File descriptor to write to */
> 		rc = write(fd, pBuf, nBuf);
> 	} while (rc < 0 && errno == EINTR);
> 
> -	if (rc < 0)
> +	if (rc < 0) {
> +		diag_set(SystemError, strerror(errno));
> 		*piErrno = errno;
> +	}
> 	return rc;
> }
> 
> -- 
> 2.7.4
>