From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 036C945C304 for ; Wed, 9 Dec 2020 11:44:36 +0300 (MSK) References: From: Leonid Vasiliev Message-ID: Date: Wed, 9 Dec 2020 11:43:39 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 1/2] sql: fix build with GCC 10 List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Artem Starshov , Alexander Turenko Cc: tarantool-patches@dev.tarantool.org Hi! Thank you for the patch. Looks correctly, but I have one question: On 27.11.2020 15:43, Artem Starshov via Tarantool-patches wrote: > GCC 10 produces the following error: > cc1: warning: function may return address of local variable [-Wreturn-local-addr] > > Fix it. > > Part-of #4966 > --- > Got LGTM from Nikita Pettik: > https://lists.tarantool.org/pipermail/tarantool-patches/2020-November/020940.html > > src/box/sql/select.c | 56 ++++++++++++++++++++------------------------ > 1 file changed, 26 insertions(+), 30 deletions(-) > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index b0554a172..5d4b2f624 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -154,56 +154,52 @@ sqlSelectNew(Parse * pParse, /* Parsing context */ > Expr * pLimit, /* LIMIT value. NULL means not used */ > Expr * pOffset) /* OFFSET value. NULL means no offset */ > { > - Select *pNew; > Select standin; > sql *db = pParse->db; > - pNew = sqlDbMallocRawNN(db, sizeof(*pNew)); > - if (pNew == 0) { 1) Why can't we return NULL if malloc has failed and don't work with standin at all? > - assert(db->mallocFailed); > - pNew = &standin; > - } > if (pEList == 0) { > struct Expr *expr = sql_expr_new_anon(db, TK_ASTERISK); > if (expr == NULL) > pParse->is_aborted = true; > pEList = sql_expr_list_append(db, NULL, expr); > } > - pNew->pEList = pEList; > - pNew->op = TK_SELECT; > - pNew->selFlags = selFlags; > - pNew->iLimit = 0; > - pNew->iOffset = 0; > + standin.pEList = pEList; > + standin.op = TK_SELECT; > + standin.selFlags = selFlags; > + standin.iLimit = 0; > + standin.iOffset = 0; > #ifdef SQL_DEBUG > - pNew->zSelName[0] = 0; > + standin.zSelName[0] = 0; > if ((pParse->sql_flags & SQL_SelectTrace) != 0) > sqlSelectTrace = 0xfff; > else > sqlSelectTrace = 0; > #endif > - pNew->addrOpenEphm[0] = -1; > - pNew->addrOpenEphm[1] = -1; > - pNew->nSelectRow = 0; > + standin.addrOpenEphm[0] = -1; > + standin.addrOpenEphm[1] = -1; > + standin.nSelectRow = 0; > if (pSrc == 0) > pSrc = sqlDbMallocZero(db, sizeof(*pSrc)); > - pNew->pSrc = pSrc; > - pNew->pWhere = pWhere; > - pNew->pGroupBy = pGroupBy; > - pNew->pHaving = pHaving; > - pNew->pOrderBy = pOrderBy; > - pNew->pPrior = 0; > - pNew->pNext = 0; > - pNew->pLimit = pLimit; > - pNew->pOffset = pOffset; > - pNew->pWith = 0; > + standin.pSrc = pSrc; > + standin.pWhere = pWhere; > + standin.pGroupBy = pGroupBy; > + standin.pHaving = pHaving; > + standin.pOrderBy = pOrderBy; > + standin.pPrior = 0; > + standin.pNext = 0; > + standin.pLimit = pLimit; > + standin.pOffset = pOffset; > + standin.pWith = 0; > assert(pOffset == 0 || pLimit != 0 || pParse->is_aborted > || db->mallocFailed != 0); > + Select *pNew = sqlDbMallocRawNN(db, sizeof(*pNew)); > if (db->mallocFailed) { > - clearSelect(db, pNew, pNew != &standin); > - pNew = 0; > - } else { > - assert(pNew->pSrc != 0 || pParse->is_aborted); > + clearSelect(db, &standin, 0); > + if (pNew != NULL) > + sqlDbFree(db, pNew); > + return NULL; > } > - assert(pNew != &standin); > + assert(standin.pSrc != 0 || pParse->is_aborted); > + memcpy(pNew, &standin, sizeof(standin)); > return pNew; > } > >