‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, 28 August 2020 03:22, Ranier Vilela <ranier.vf@gmail.com> 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 {
148 ttc.ttc_oldvalues = NULL;
149 ttc.ttc_oldisnull = NULL;
150 }
151 else
152 {
153 ttc.ttc_oldvalues = toast_oldvalues;
154 ttc.ttc_oldisnull = toast_oldisnull;
155 }
156 ttc.ttc_attr = toast_attr;
157 toast_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