[Tarantool-patches] [PATCH v2 1/2] sql: fix build with GCC 10

[Artem]

Hi, thanks for your letter!

Answering your question: we can't just return NULL, because this 
function, oddly enough,

is responsible for clearing of its arguments in case of allocation 
failure. In order to clean

them correctly we need to fill `Select` structure and invoke 
`clearSelect` for it.

09.12.2020 11:43, Leonid Vasiliev пишет:
> 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;
>>   }
>>