Tarantool development patches archive
 help / color / mirror / Atom feed
From: Leonid Vasiliev <lvasiliev@tarantool.org>
To: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org,
	imeevma@tarantool.org, korablev@tarantool.org,
	sergos@tarantool.org
Subject: [Tarantool-patches] [PATCH v3 1/2] sql: add missing diag_set on failure when working inside os_unix.c
Date: Thu, 17 Dec 2020 02:09:06 +0300	[thread overview]
Message-ID: <54ef9ccd2a7bee1f5f53a811c7edea1ba034c4ca.1608159414.git.lvasiliev@tarantool.org> (raw)
In-Reply-To: <cover.1608159414.git.lvasiliev@tarantool.org>
In-Reply-To: <cover.1608159414.git.lvasiliev@tarantool.org>

SQL module didn't set an error in the diagnostics area on failure
inside unix.c. This could lead to a crash like in #5537.

Co-authored-by: Mergen Imeev<imeevma@gmail.com>

Follow-up #5537
---
 src/box/sql/os_unix.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 5 deletions(-)

diff --git a/src/box/sql/os_unix.c b/src/box/sql/os_unix.c
index d64f1bd..4f59767 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, "failed to open file '%s'", z);
 			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, "failed to open '/dev/null'");
 			break;
+		}
 	}
 	if (fd >= 0) {
 		if (m != 0) {
@@ -193,6 +196,10 @@ robust_ftruncate(int h, sql_int64 sz)
 	do {
 		rc = ftruncate(h, sz);
 	} while (rc < 0 && errno == EINTR);
+
+	if (rc < 0)
+		diag_set(SystemError, "failed to truncate file");
+
 	return rc;
 }
 
@@ -395,6 +402,9 @@ findInodeInfo(unixFile * pFile,	/* Unix file with file desc used in the key */
 	fd = pFile->h;
 	rc = fstat(fd, &statbuf);
 	if (rc != 0) {
+		diag_set(SystemError,
+			 "failed to retrive information about the file '%s'",
+			 pFile->zPath);
 		storeLastErrno(pFile, errno);
 		return -1;
 	}
@@ -433,7 +443,13 @@ static int
 fileHasMoved(unixFile * pFile)
 {
 	struct stat buf;
-	return pFile->pInode != NULL && (stat(pFile->zPath, &buf) != 0 ||
+	int rc = stat(pFile->zPath, &buf);
+	if (rc < 0) {
+		diag_set(SystemError,
+			 "failed to retrive information about the file '%s'",
+			 pFile->zPath);
+	}
+	return pFile->pInode != NULL && (rc != 0 ||
 					 (u64) buf.st_ino !=
 					 pFile->pInode->fileId.ino);
 }
@@ -473,8 +489,12 @@ unixFileLock(unixFile * pFile, struct flock *pLock)
 			lock.l_len = SHARED_SIZE;
 			lock.l_type = F_WRLCK;
 			rc = fcntl(pFile->h, F_SETLK, &lock);
-			if (rc < 0)
+			if (rc < 0) {
+				diag_set(SystemError,
+					 "failed to acquire a lock on the"
+					 " file '%s'", pFile->zPath);
 				return rc;
+			}
 			pInode->bProcessLock = 1;
 			pInode->nLock++;
 		} else {
@@ -482,6 +502,11 @@ unixFileLock(unixFile * pFile, struct flock *pLock)
 		}
 	} else {
 		rc = fcntl(pFile->h, F_SETLK, pLock);
+		if (rc < 0) {
+			diag_set(SystemError,
+				 "failed to acquire a lock on the file '%s'",
+				 pFile->zPath);
+		}
 	}
 	return rc;
 }
@@ -729,6 +754,9 @@ seekAndRead(unixFile * id, sql_int64 offset, void *pBuf, int cnt)
 	do {
 		newOffset = lseek(id->h, offset, SEEK_SET);
 		if (newOffset < 0) {
+			diag_set(SystemError,
+				 "failed to reposition the offset of '%s' file",
+				 id->zPath);
 			storeLastErrno((unixFile *) id, errno);
 			return -1;
 		}
@@ -740,6 +768,8 @@ seekAndRead(unixFile * id, sql_int64 offset, void *pBuf, int cnt)
 				got = 1;
 				continue;
 			}
+			diag_set(SystemError, "failed to read from file '%s'",
+				 id->zPath);
 			prior = 0;
 			storeLastErrno((unixFile *) id, errno);
 			break;
@@ -825,10 +855,16 @@ seekAndWriteFd(int fd,		/* File descriptor to write to */
 	do {
 		i64 iSeek = lseek(fd, iOff, SEEK_SET);
 		if (iSeek < 0) {
+			diag_set(SystemError,
+				 "failed to reposition file offset");
 			rc = -1;
 			break;
 		}
 		rc = write(fd, pBuf, nBuf);
+		if (rc < 0) {
+			diag_set(SystemError,
+				 "failed to write %i bytes to file", nBuf);
+		}
 	} while (rc < 0 && errno == EINTR);
 
 	if (rc < 0)
@@ -940,8 +976,12 @@ fcntlSizeHint(unixFile * pFile, i64 nByte)
 		i64 nSize;	/* Required file size */
 		struct stat buf;	/* Used to hold return values of fstat() */
 
-		if (fstat(pFile->h, &buf))
+		if (fstat(pFile->h, &buf)) {
+			diag_set(SystemError,
+				 "failed to retrive information about the"
+				 " file '%s'", pFile->zPath);
 			return -1;
+		}
 
 		nSize =
 		    ((nByte + pFile->szChunk -
@@ -1165,8 +1205,12 @@ unixMapfile(unixFile * pFd, i64 nMap)
 
 	if (nMap < 0) {
 		struct stat statbuf;	/* Low-level file information */
-		if (fstat(pFd->h, &statbuf))
+		if (fstat(pFd->h, &statbuf)) {
+			diag_set(SystemError,
+				 "failed to retrive information about the"
+				 " file '%s'", pFd->zPath);
 			return -1;
+		}
 		nMap = statbuf.st_size;
 	}
 	if (nMap > pFd->mmapSizeMax) {
@@ -1449,6 +1493,8 @@ unixTempFileDir(void)
 			break;
 		zDir = azDirs[i++];
 	}
+	diag_set(ClientError, ER_SYSTEM,
+		 "No access to any temporary directory");
 	return 0;
 }
 
@@ -1558,6 +1604,9 @@ getFileMode(const char *zFile,	/* File name */
 		*pUid = sStat.st_uid;
 		*pGid = sStat.st_gid;
 	} else {
+		diag_set(SystemError,
+			 "failed to retrive information about the file '%s'",
+			 zFile);
 		rc = -1;
 	}
 	return rc;
@@ -1813,6 +1862,7 @@ unixDelete(sql_vfs * NotUsed,	/* VFS containing this as the xDelete method */
 	int rc = 0;
 	UNUSED_PARAMETER(NotUsed);
 	if (unlink(zPath) == (-1)) {
+		diag_set(SystemError, "failed to unlink the file '%s'", zPath);
 		return -1;
 	}
 	if ((dirSync & 1) != 0) {
@@ -1821,6 +1871,9 @@ unixDelete(sql_vfs * NotUsed,	/* VFS containing this as the xDelete method */
 		if (rc == 0) {
 			struct stat buf;
 			if (fstat(fd, &buf)) {
+				diag_set(SystemError,
+					 "failed to retrive information about"
+					 " the file '%s'", zPath);
 				rc = -1;
 			}
 			close(fd);
-- 
2.7.4

  reply	other threads:[~2020-12-16 23:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 23:09 [Tarantool-patches] [PATCH v3 0/2] Fix working with VDBE Leonid Vasiliev
2020-12-16 23:09 ` Leonid Vasiliev [this message]
2020-12-20 16:02   ` [Tarantool-patches] [PATCH v3 1/2] sql: add missing diag_set on failure when working inside os_unix.c Vladislav Shpilevoy
2020-12-23 23:27     ` Leonid Vasiliev
2020-12-24 16:00       ` Vladislav Shpilevoy
2020-12-24 16:59         ` Nikita Pettik
2020-12-16 23:09 ` [Tarantool-patches] [PATCH v3 2/2] sql: add panic() call in sql_execute() on complete failure Leonid Vasiliev
2020-12-20 16:02   ` Vladislav Shpilevoy
2020-12-23 22:38     ` Leonid Vasiliev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54ef9ccd2a7bee1f5f53a811c7edea1ba034c4ca.1608159414.git.lvasiliev@tarantool.org \
    --to=lvasiliev@tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v3 1/2] sql: add missing diag_set on failure when working inside os_unix.c' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox