Em sex., 28 de ago. de 2020 às 04:45, escreveu:
>
>
>
>
>
> ‐‐‐ Original Message ‐‐‐
> On Friday, 28 August 2020 03:22, Ranier Vilela
> wrote:
>
> > Hi,
> >
> > Per Coverity.
> >
> > When "Prepare for toasting", it is necessary to turn off the flag
> TOAST_NEEDS_DELETE_OLD,
> > if there is no need to delete external values from the old tuple,
> otherwise,
> > there are dereference NULL at toast_helper.c (on toast_tuple_cleanup
> function).
> >
>
> Excuse my ignorance, isn't this a false positive?
>
Yes, you're right.
Coverity fails with &.
if (oldtup == NULL)
147{
3. assign_zero: Assigning: ttc.ttc_oldvalues = NULL.
148ttc.ttc_oldvalues = NULL;
149ttc.ttc_oldisnull = NULL;
4. Falling through to end of if statement.
150}
151else
152{
153ttc.ttc_oldvalues = toast_oldvalues;
154ttc.ttc_oldisnull = toast_oldisnull;
155}
156ttc.ttc_attr = toast_attr;
157toast_tuple_init(&ttc); // Coverity ignores the call completely
here.
toast_tuple_init, solves the bug, because reset ttc->flags.
> Regardless right after prepare for toasting, a call to toast_tuple_init is
> made which will explicitly and unconditionally set ttc_flags to zero so the
> flag bit set in the patch will be erased anyways. This patch may make
> coverity happy but does not really change anything in the behaviour of the
> code.
>
That's right, the patch doesn't change anything.
>
> Furthermore, in the same function, toast_tuple_init, the flag is set to
> TOAST_NEEDS_DELETE_OLD after the old value is actually inspected and found
> to not be null, be stored on disk and to be different than the new value.
> To my understanding, this seems to be correct.
>
Very correct.
Thanks for taking a look here.
You could take a look at the attached patch,
would it be an improvement?
toast_tuple_init, it seems to me that it can be improved.
ttc->ttc_oldvalues is constant, and it could be unlooping?
regards,
Ranier Vilela
unloop_toast_tuple_init.patch
Description: Binary data
