From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 045916EC5F; Fri, 9 Apr 2021 22:46:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 045916EC5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617997567; bh=jqnVayey/WHgCzZeSQCYtv1LJRvT0DAfHOa66asTFxc=; h=To:Cc:Date:In-Reply-To:References:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=i++lNq6+WaLlVDlZ+lxSko4GbzIC3qPeydYI67KT/G+6dCA1Sp9TT3TGpcK1+1Uyk jLf5KJ6uY/CCR3aH5FT8J6Sy9fkSRVr+OQ5clKJb31xzGKAGcZQRU2/9zdBwpl5Vnz /FaKWGU/T0WkTgUhDDGKmv2vk05KMeee9cDsCou4= Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 645B56EC60 for ; Fri, 9 Apr 2021 22:45:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 645B56EC60 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lUx4T-0006pl-HI; Fri, 09 Apr 2021 22:45:37 +0300 To: v.shpilevoy@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Fri, 9 Apr 2021 22:45:37 +0300 Message-Id: X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E7480EBD5CA77A668ECB87DA2124B0A8E6609182A05F538085040C0654FBC0926D42E8D374CAB6B1C027E8CAD228F6ACB658662D51572D63EB70E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A3295C83650092F9EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063749A0D61DF1984B008638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B25D4BDFB68FC84D50F7CFCB275DE150E0E8578C74B6F99271D2E47CDBA5A96583C09775C1D3CA48CFE97D2AE7161E217F117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE759A2DA0C93DFCD719FA2833FD35BB23DF004C906525384302BEBFE083D3B9BA73A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735E4A630A5B664A4FFC4224003CC83647689D4C264860C145E X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2AD77751E876CB595E8F7B195E1C97831712C5303C9CF66A695489C0DEC852D72 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975CD0035DD76F8A8A4F54D617570B3D5A4CFEF328A3CDE899339C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EF0417BEADF48D1460699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34AF12ADB97C97CD890AB72E048E2E0E93A5365FD1DDC0D08B4BABA204C18B50F639893BF41A49A6511D7E09C32AA3244C7FD62328B7FF249AE397557C4743A3F881560E2432555DBBFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyO2lHpuZu4S32PAVpKwzwg== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822F663AC62DDEB02CDA0EF973EFD3991E383D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v5 27/52] sql: introduce mem_set_str_*() functions X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mergen Imeev via Tarantool-patches Reply-To: imeevma@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thank you for the review! My answers and new patch below. On 30.03.2021 02:05, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 2 comments below. > > 1. I think it would be better to have mem_set_str_* as a prefix > for these functions. 'string' is too long, and by placing allocation > type in the beginning you complicate grep by mem_set_str_* and > autocompletion when I type 'mem_set_str_' and want to see all the > options. > Fixed. Now all functions named as mem_set_str_*() or mem_set_str0_*(). > On 23.03.2021 10:35, Mergen Imeev via Tarantool-patches wrote: >> This patch introduces mem_set_*_string() functions. Functions >> mem_set_*_string() clears MEM and sets it to given string value. >> >> Part of #5818 >> --- >> src/box/sql/mem.c | 111 +++++++++++++++++++++++++++++++++--------- >> src/box/sql/mem.h | 26 +++++++++- >> src/box/sql/sqlInt.h | 4 -- >> src/box/sql/vdbe.c | 5 +- >> src/box/sql/vdbeapi.c | 47 ++++++++++++++---- >> src/box/sql/vdbeaux.c | 45 +++++++---------- >> 6 files changed, 171 insertions(+), 67 deletions(-) >> >> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c >> index 47a71fb30..91ef7f3c8 100644 >> --- a/src/box/sql/mem.c >> +++ b/src/box/sql/mem.c >> @@ -2780,13 +2849,9 @@ vdbe_decode_msgpack_into_ephemeral_mem(const char *buf, struct Mem *mem, >> break; >> } >> case MP_STR: { >> - /* XXX u32->int */ >> - mem->n = (int) mp_decode_strl(&buf); >> - mem->flags = MEM_Str | MEM_Ephem; >> - mem->field_type = FIELD_TYPE_STRING; >> -install_blob: >> - mem->z = (char *)buf; >> - buf += mem->n; >> + uint32_t len = mp_decode_strl(&buf); >> + mem_set_ephemeral_string(mem, (char *)buf, len); > > 2. It adds clear() call which is not necessary here. I would > propose to keep the old code for the sake of speed and > consistency with the other mp types. Thanks, fixed. New patch: commit f82e5a93ce35b81b682195e92284fa37a707b14e Author: Mergen Imeev Date: Mon Mar 15 14:28:29 2021 +0300 sql: introduce mem_set_str_*() functions This patch introduces set of mem_set_str_*() functions. These functions clears MEM and sets it to given string value. Degree of clearing and type of allocation of the string is determined by the function used. Part of #5818 diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index c16f3a28a..87e1bcfd1 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -311,6 +311,89 @@ mem_set_double(struct Mem *mem, double value) mem->flags = MEM_Real; } +static inline void +set_str_const(struct Mem *mem, char *value, uint32_t len, int alloc_type) +{ + assert((alloc_type & (MEM_Static | MEM_Ephem)) != 0); + mem_clear(mem); + mem->z = value; + mem->n = len; + mem->flags = MEM_Str | alloc_type; + mem->field_type = FIELD_TYPE_STRING; +} + +static inline void +set_str_dynamic(struct Mem *mem, char *value, uint32_t len, int alloc_type) +{ + assert((mem->flags & MEM_Dyn) == 0 || value != mem->z); + assert(mem->szMalloc == 0 || value != mem->zMalloc); + assert(alloc_type == MEM_Dyn || alloc_type == 0); + mem_destroy(mem); + mem->z = value; + mem->n = len; + mem->flags = MEM_Str | alloc_type; + mem->field_type = FIELD_TYPE_STRING; + if (alloc_type == MEM_Dyn) { + mem->xDel = sql_free; + } else { + mem->xDel = NULL; + mem->zMalloc = mem->z; + mem->szMalloc = sqlDbMallocSize(mem->db, mem->zMalloc); + } +} + +void +mem_set_str_ephemeral(struct Mem *mem, char *value, uint32_t len) +{ + set_str_const(mem, value, len, MEM_Ephem); +} + +void +mem_set_str_static(struct Mem *mem, char *value, uint32_t len) +{ + set_str_const(mem, value, len, MEM_Static); +} + +void +mem_set_str_dynamic(struct Mem *mem, char *value, uint32_t len) +{ + set_str_dynamic(mem, value, len, MEM_Dyn); +} + +void +mem_set_str_allocated(struct Mem *mem, char *value, uint32_t len) +{ + set_str_dynamic(mem, value, len, 0); +} + +void +mem_set_str0_ephemeral(struct Mem *mem, char *value) +{ + set_str_const(mem, value, strlen(value), MEM_Ephem); + mem->flags |= MEM_Term; +} + +void +mem_set_str0_static(struct Mem *mem, char *value) +{ + set_str_const(mem, value, strlen(value), MEM_Static); + mem->flags |= MEM_Term; +} + +void +mem_set_str0_dynamic(struct Mem *mem, char *value) +{ + set_str_dynamic(mem, value, strlen(value), MEM_Dyn); + mem->flags |= MEM_Term; +} + +void +mem_set_str0_allocated(struct Mem *mem, char *value) +{ + set_str_dynamic(mem, value, strlen(value), 0); + mem->flags |= MEM_Term; +} + int mem_copy(struct Mem *to, const struct Mem *from) { @@ -2085,20 +2168,6 @@ sqlVdbeMemSetZeroBlob(Mem * pMem, int n) pMem->z = 0; } -/* - * Change the string value of an sql_value object - */ -void -sqlValueSetStr(sql_value * v, /* Value to be set */ - int n, /* Length of string z */ - const void *z, /* Text of the new string */ - void (*xDel) (void *) /* Destructor for the string */ - ) -{ - if (v) - sqlVdbeMemSetStr((Mem *) v, z, n, 1, xDel); -} - /* * Free an sql_value object */ diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 8022f53d8..ecaa1edb8 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -182,6 +182,59 @@ mem_set_bool(struct Mem *mem, bool value); void mem_set_double(struct Mem *mem, double value); +/** Clear MEM and set it to STRING. The string belongs to another object. */ +void +mem_set_str_ephemeral(struct Mem *mem, char *value, uint32_t len); + +/** Clear MEM and set it to STRING. The string is static. */ +void +mem_set_str_static(struct Mem *mem, char *value, uint32_t len); + +/** + * Clear MEM and set it to STRING. The string was allocated by another object + * and passed to MEM. MEMs with this allocation type must free given memory + * whenever the MEM changes. + */ +void +mem_set_str_dynamic(struct Mem *mem, char *value, uint32_t len); + +/** + * Clear MEM and set it to STRING. The string was allocated by another object + * and passed to MEM. MEMs with this allocation type only deallocate the string + * on destruction. Also, the memory may be reallocated if MEM is set to a + * different value of this allocation type. + */ +void +mem_set_str_allocated(struct Mem *mem, char *value, uint32_t len); + +/** + * Clear MEM and set it to NULL-terminated STRING. The string belongs to + * another object. + */ +void +mem_set_str0_ephemeral(struct Mem *mem, char *value); + +/** Clear MEM and set it to NULL-terminated STRING. The string is static. */ +void +mem_set_str0_static(struct Mem *mem, char *value); + +/** + * Clear MEM and set it to NULL-terminated STRING. The string was allocated by + * another object and passed to MEM. MEMs with this allocation type must free + * given memory whenever the MEM changes. + */ +void +mem_set_str0_dynamic(struct Mem *mem, char *value); + +/** + * Clear MEM and set it to NULL-terminated STRING. The string was allocated by + * another object and passed to MEM. MEMs with this allocation type only + * deallocate the string on destruction. Also, the memory may be reallocated if + * MEM is set to a different value of this allocation type. + */ +void +mem_set_str0_allocated(struct Mem *mem, char *value); + /** * Copy content of MEM from one MEM to another. In case source MEM contains * string or binary and allocation type is not STATIC, this value is copied to @@ -464,8 +517,6 @@ int sqlVdbeMemSetStr(struct Mem *, const char *, int, u8, void (*)(void *)); void sqlVdbeMemSetZeroBlob(struct Mem *, int); -void sqlValueSetStr(struct Mem *, int, const void *, - void (*)(void *)); void sqlValueFree(struct Mem *); struct Mem *sqlValueNew(struct sql *); diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 259ba3833..7a026d21b 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -641,10 +641,6 @@ sql_bind_uint64(struct sql_stmt *stmt, int i, uint64_t value); int sql_bind_null(sql_stmt *, int); -int -sql_bind_text(sql_stmt *, int, const char *, int, - void (*)(void *)); - int sql_bind_text64(sql_stmt *, int, const char *, sql_uint64, void (*)(void *)); diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index eb997fa3d..943179c55 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -841,9 +841,8 @@ case OP_String8: { /* same as TK_STRING, out2 */ case OP_String: { /* out2 */ assert(pOp->p4.z!=0); pOut = vdbe_prepare_null_out(p, pOp->p2); - pOut->flags = MEM_Str|MEM_Static|MEM_Term; - pOut->z = pOp->p4.z; - pOut->n = pOp->p1; + assert(strlen(pOp->p4.z) == (size_t)pOp->p1); + mem_set_str0_static(pOut, pOp->p4.z); UPDATE_MAX_BLOBSIZE(pOut); break; } diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index 6c08e772d..e0903e3b0 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -125,6 +125,27 @@ setResultStrOrError(sql_context * pCtx, /* Function context */ void (*xDel) (void *) /* Destructor function */ ) { + if (xDel == SQL_STATIC) { + if (n < 0) + mem_set_str0_static(pCtx->pOut, (char *)z); + else + mem_set_str_static(pCtx->pOut, (char *)z, n); + return; + } + if (xDel == SQL_DYNAMIC) { + if (n < 0) + mem_set_str0_allocated(pCtx->pOut, (char *)z); + else + mem_set_str_allocated(pCtx->pOut, (char *)z, n); + return; + } + if (xDel != SQL_TRANSIENT) { + if (n < 0) + mem_set_str0_dynamic(pCtx->pOut, (char *)z); + else + mem_set_str_dynamic(pCtx->pOut, (char *)z, n); + return; + } if (sqlVdbeMemSetStr(pCtx->pOut, z, n, 1, xDel) != 0) pCtx->is_aborted = true; } @@ -762,8 +783,24 @@ bindText(sql_stmt * pStmt, /* The statement to bind against */ if (zData == NULL) return 0; pVar = &p->aVar[i - 1]; - if (sqlVdbeMemSetStr(pVar, zData, nData, 1, xDel) != 0) + if (xDel == SQL_STATIC) { + if (nData < 0) + mem_set_str0_static(pVar, (char *)zData); + else + mem_set_str_static(pVar, (char *)zData, nData); + } else if (xDel == SQL_DYNAMIC) { + if (nData < 0) + mem_set_str0_allocated(pVar, (char *)zData); + else + mem_set_str_allocated(pVar, (char *)zData, nData); + } else if (xDel != SQL_TRANSIENT) { + if (nData < 0) + mem_set_str0_dynamic(pVar, (char *)zData); + else + mem_set_str_dynamic(pVar, (char *)zData, nData); + } else if (sqlVdbeMemSetStr(pVar, zData, nData, 1, xDel) != 0) { return -1; + } return sql_bind_type(p, i, "text"); } @@ -876,14 +913,6 @@ sql_bind_ptr(struct sql_stmt *stmt, int i, void *ptr) return rc; } -int -sql_bind_text(sql_stmt * pStmt, - int i, const char *zData, int nData, void (*xDel) (void *) - ) -{ - return bindText(pStmt, i, zData, nData, xDel); -} - int sql_bind_text64(sql_stmt * pStmt, int i, diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index ae55e4c29..171cb8946 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -1295,10 +1295,8 @@ sqlVdbeList(Vdbe * p) pMem++; - pMem->flags = MEM_Static | MEM_Str | MEM_Term; - pMem->z = (char *)sqlOpcodeName(pOp->opcode); /* Opcode */ - assert(pMem->z != 0); - pMem->n = sqlStrlen30(pMem->z); + char *value = (char *)sqlOpcodeName(pOp->opcode); + mem_set_str0_static(pMem, value); pMem++; /* When an OP_Program opcode is encounter (the only opcode that has @@ -1333,41 +1331,34 @@ sqlVdbeList(Vdbe * p) mem_set_int(pMem, pOp->p3, pOp->p3 < 0); pMem++; - if (sqlVdbeMemClearAndResize(pMem, 256)) { - assert(p->db->mallocFailed); + char *buf = sqlDbMallocRaw(sql_get(), 256); + if (buf == NULL) return -1; - } - pMem->flags = MEM_Str | MEM_Term; - zP4 = displayP4(pOp, pMem->z, pMem->szMalloc); - - if (zP4 != pMem->z) { - pMem->n = 0; - sqlVdbeMemSetStr(pMem, zP4, -1, 1, 0); + zP4 = displayP4(pOp, buf, sqlDbMallocSize(sql_get(), buf)); + if (zP4 != buf) { + sqlDbFree(sql_get(), buf); + mem_set_str0_ephemeral(pMem, zP4); } else { - assert(pMem->z != 0); - pMem->n = sqlStrlen30(pMem->z); + mem_set_str0_allocated(pMem, zP4); } pMem++; if (p->explain == 1) { - if (sqlVdbeMemClearAndResize(pMem, 4)) { - assert(p->db->mallocFailed); + buf = sqlDbMallocRaw(sql_get(), 4); + if (buf == NULL) return -1; - } - pMem->flags = MEM_Str | MEM_Term; - pMem->n = 2; - sql_snprintf(3, pMem->z, "%.2x", pOp->p5); /* P5 */ + sql_snprintf(3, buf, "%.2x", pOp->p5); + mem_set_str0_allocated(pMem, buf); pMem++; #ifdef SQL_ENABLE_EXPLAIN_COMMENTS - if (sqlVdbeMemClearAndResize(pMem, 500)) { - assert(p->db->mallocFailed); + buf = sqlDbMallocRaw(sql_get(), 500); + if (buf == NULL) return -1; - } - pMem->flags = MEM_Str | MEM_Term; - pMem->n = displayComment(pOp, zP4, pMem->z, 500); + displayComment(pOp, zP4, buf, 500); + mem_set_str0_allocated(pMem, buf); #else - pMem->flags = MEM_Null; /* Comment */ + mem_set_null(pMem); #endif }