Re: Possible NULL dereferencing null pointer (src/backend/executor/nodeIncrementalSort.c)
От | Tomas Vondra |
---|---|
Тема | Re: Possible NULL dereferencing null pointer (src/backend/executor/nodeIncrementalSort.c) |
Дата | |
Msg-id | 20201009221259.hz663wygcsuly4vc@development обсуждение исходный текст |
Ответ на | Re: Possible NULL dereferencing null pointer (src/backend/executor/nodeIncrementalSort.c) (Ranier Vilela <ranier.vf@gmail.com>) |
Ответы |
Re: Possible NULL dereferencing null pointer (src/backend/executor/nodeIncrementalSort.c)
|
Список | pgsql-hackers |
On Fri, Oct 09, 2020 at 05:50:09PM -0300, Ranier Vilela wrote: >Em sex., 9 de out. de 2020 às 17:47, Tom Lane <tgl@sss.pgh.pa.us> escreveu: > >> Ranier Vilela <ranier.vf@gmail.com> writes: >> > The trap is not on the second part of expression. Is in the first. >> > If the pointer is NULL, ExecCopySlot will be called. >> >> Your initial comment indicated that you were worried about >> IncrementalSortState's group_pivot slot, but that is never going >> to be null in any execution function of nodeIncrementalSort.c, >> because ExecInitIncrementalSort always creates it. >> >> (The test whether it's NULL in ExecReScanIncrementalSort therefore >> seems rather useless and misleading, but it's not actually a bug.) >> >> The places that use TupIsNull are just doing so because that's >> the standard way to check whether a slot is empty. The null >> test inside the macro is pointless in this context (and in a lot >> of its other use-cases, too) but we don't worry about that. >> >So I said that TupIsNull was not the most appropriate. > >Doesn't it look better? >(node->group_pivot != NULL && TTS_EMPTY(node->group_pivot)) > My (admittedly very subjective) opinion is that it looks much worse. The TupIsNull is pretty self-descriptive, unlike this proposed code. That could be fixed by defining a new macro, perhaps something like SlotIsEmpty() or so. But as was already explained, Incremental Sort can't actually have a NULL slot here, so it makes no difference there. And in the other places we can't just mechanically replace the macros because it'd quite likely silently hide pre-existing bugs. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
В списке pgsql-hackers по дате отправления: