[tarantool-patches] Re: [PATCH v2 1/7] sql: refactor sql_alloc_src_list to set diag

[Kirill Shcherbatov]

On 07.03.2019 20:34, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
Hi! Thank you for review.

=====================================================

Refactored sqlAllocSrcList routine as sql_src_list_new and
reworked it to use diag_set in case of memory allocation error.
This will ensure that the sqlSrcListAppend function throws an
error using diag in subsequent patches.

This patch refers to a series of preparatory patches that provide
the use of Tarantool errors in the call tree that includes
sqlNormalizeName, since this call can later return errors.

This patch is not self-sufficient, its usage in sqlSrcListAppend
remained to be non-Tarantool (for now). It means, that if
sql_src_list_new fails in sqlSrcListAppend and sets a diag, it is
never thrown to a user (now).

Part of #3931
---
 src/box/sql/build.c  | 34 +++++++++++++++++++---------------
 src/box/sql/select.c |  2 +-
 src/box/sql/sqlInt.h | 13 +++++++++++--
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index e9851d9a1..760054552 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1396,9 +1396,12 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
 	assert(idx_name != NULL || table_name != NULL);
 	struct sql *db = parse->db;
 	assert(!db->mallocFailed);
-	struct SrcList *src_list = sql_alloc_src_list(db);
-	if (src_list != NULL)
-		src_list->a[0].zName = sqlDbStrDup(db, stat_table_name);
+	struct SrcList *src_list = sql_src_list_new(db);
+	if (src_list == NULL) {
+		sql_parser_error(parse);
+		return;
+	}
+	src_list->a[0].zName = sqlDbStrDup(db, stat_table_name);
 	struct Expr *where = NULL;
 	if (idx_name != NULL) {
 		struct Expr *expr = sql_id_eq_str_expr(parse, "idx", idx_name);
@@ -2666,19 +2669,20 @@ sqlSrcListEnlarge(sql * db,	/* Database connection to notify of OOM errors */
 	return pSrc;
 }
 
-SrcList *
-sql_alloc_src_list(sql *db)
+struct SrcList *
+sql_src_list_new(struct sql *db)
 {
-	SrcList *pList;
-
-	pList = sqlDbMallocRawNN(db, sizeof(SrcList));
-	if (pList == 0)
+	struct SrcList *src_list = sqlDbMallocRawNN(db, sizeof(SrcList));
+	if (src_list == NULL) {
+		diag_set(OutOfMemory, sizeof(SrcList), "sqlDbMallocRawNN",
+			 "src_list");
 		return NULL;
-	pList->nAlloc = 1;
-	pList->nSrc = 1;
-	memset(&pList->a[0], 0, sizeof(pList->a[0]));
-	pList->a[0].iCursor = -1;
-	return pList;
+	}
+	src_list->nAlloc = 1;
+	src_list->nSrc = 1;
+	memset(&src_list->a[0], 0, sizeof(src_list->a[0]));
+	src_list->a[0].iCursor = -1;
+	return src_list;
 }
 
 /*
@@ -2724,7 +2728,7 @@ sqlSrcListAppend(sql * db,	/* Connection to notify of malloc failures */
 	struct SrcList_item *pItem;
 	assert(db != 0);
 	if (pList == 0) {
-		pList = sql_alloc_src_list(db);
+		pList = sql_src_list_new(db);
 		if (pList == 0)
 			return 0;
 	} else {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 782da1f7c..16d535076 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -315,7 +315,7 @@ sql_select_expand_from_tables(struct Select *select)
 {
 	assert(select != NULL);
 	struct Walker walker;
-	struct SrcList *table_names = sql_alloc_src_list(sql_get());
+	struct SrcList *table_names = sql_src_list_new(sql_get());
 	if (table_names == NULL)
 		return NULL;
 	memset(&walker, 0, sizeof(walker));
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1d8fae5b0..4a4a748ba 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3391,8 +3391,17 @@ void *sqlArrayAllocate(sql *, void *, int, int *, int *);
 IdList *sqlIdListAppend(sql *, IdList *, Token *);
 int sqlIdListIndex(IdList *, const char *);
 SrcList *sqlSrcListEnlarge(sql *, SrcList *, int, int);
-SrcList *
-sql_alloc_src_list(sql *db);
+
+/**
+ * Allocate a new empty SrcList object.
+ *
+ * @param db The database connection.
+ * @retval Not NULL list pointer on success.
+ * @retval NULL Otherwise. The diag message is set.
+ */
+struct SrcList *
+sql_src_list_new(struct sql *db);
+
 SrcList *sqlSrcListAppend(sql *, SrcList *, Token *);
 SrcList *sqlSrcListAppendFromTerm(Parse *, SrcList *, Token *,
 				      Token *, Select *, Expr *, IdList *);
-- 
2.21.0