Re: [BUG] (firsttupleslot)==NULL is redundant or is possible null dereference?

On Fri, Nov 22, 2019 at 10:57:13PM +0000, Ranier Vilela wrote:
>Hi,
>Sorry, you are right.
>Had not seen this line:
>firsttupleslot = node->ss.ss_ScanTupleSlot;
>

OK, no problem. When writing future messages to this list, please

* Make sure you explain why you think a given code is broken. Ideally,
bug reports come with a reproducer (instructions how to hit it) but
that may be difficult in some cases.

* Don't top post, but respond in-line. Top posting makes it much harder
to follow the discussion, in-line replies are customary here.

* Don't mark questions as bugs in the subject.

Otherwise you'll just annoy people to the extent that they'll start
ignoring your posts entirely.

We're OK with answering querstions and helping people learn the code
base, but the other side needs to make a bit of effort too.

regards

>Best regards.
>Ranier Vilela
>________________________________________
>De: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
>Enviado: sexta-feira, 22 de novembro de 2019 22:54
>Para: Ranier Vilela
>Cc: pgsql-hackers(at)postgresql(dot)org
>Assunto: Re: [BUG] (firsttupleslot)==NULL is redundant or is possible null dereference?
>
>On Fri, Nov 22, 2019 at 10:32:11PM +0000, Ranier Vilela wrote:
>>Hi,
>>This is real bug? firsttupleslot == NULL.
>>
>
>Ranier, I don't want to be rude, but I personally am getting a bit
>annoyed by this torrent of bug reports that are essentially just a bunch
>of copy-pasted chunks of code, without any specification of bench,
>position in the file, etc.
>
>And more importantly, without any clear explanation why you think it is
>a bug (or even a demonstration of an issue), and "Is this a bug?"
>
>>\backend\executor\nodeGroup.c
>> if (TupIsNull(firsttupleslot))
>> {
>> outerslot = ExecProcNode(outerPlanState(node));
>> if (TupIsNull(outerslot))
>> {
>> /* empty input, so return nothing */
>> node->grp_done = true;
>> return NULL;
>> }
>> /* Copy tuple into firsttupleslot */
>> ExecCopySlot(firsttupleslot, outerslot);
>>
>>include\executor\tuptable.h:
>>#define TupIsNull(slot) \
>> ((slot) == NULL || TTS_EMPTY(slot))
>>
>>static inline TupleTableSlot *
>>ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
>>{
>> Assert(!TTS_EMPTY(srcslot));
>>
>> dstslot->tts_ops->copyslot(dstslot, srcslot);
>>
>> return dstslot;
>>}
>>
>
>And why do you think this is a bug? Immediately before the part of code
>you copied we have this:
>
> /*
> * The ScanTupleSlot holds the (copied) first tuple of each group.
> */
> firsttupleslot = node->ss.ss_ScanTupleSlot;
>
>And node->ss.ss_ScanTupleSlot is expected to be non-NULL. So the initial
>assumption that firsttupleslot is NULL is incorrect.
>
>regards
>
>--
>Tomas Vondra http://www.2ndQuadrant.com
>PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services