Re: Fix calloc check if oom (PQcancelCreate)

Em seg., 27 de mai. de 2024 às 12:40, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>
escreveu:

> On Mon, 27 May 2024 at 16:06, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> > Em seg., 27 de mai. de 2024 às 10:23, Daniel Gustafsson <daniel(at)yesql(dot)se>
> escreveu:
> >> > On 27 May 2024, at 14:25, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> >> > I think that commit 61461a3, left some oversight.
> >> > The function *PQcancelCreate* fails in check,
> >> > return of *calloc* function.
> >> >
> >> > Trivial fix is attached.
> >>
> >> Agreed, this looks like a copy/paste from the calloc calls a few lines
> up.
> >
> > Yeah.
>
> Agreed, this was indeed a copy paste mistake
>
>
> >> > But, IMO, I think that has more problems.
> >> > If any allocation fails, all allocations must be cleared.
> >> > Or is the current behavior acceptable?
> >>
> >> Since this is frontend library code I think we should free all the
> allocations
> >> in case of OOM.
> >
> > Agreed.
> >
> > With v1 patch, it is handled.
>
> I much prefer the original trivial patch to the v1. Even in case of
> OOM users are expected to call PQcancelFinish on a non-NULL result,
> which in turn calls freePGConn.

Is it mandatory to call PQcancelFinish in case PQcancelCreate fails?
IMO, I would expect problems with users.

And that function will free any
> partially initialized PGconn correctly. This is also how
> pqConnectOptions2 works.
>
Well, I think that function *pqReleaseConnHost*, is incomplete.
1. Don't free connhost field;
2. Don't free addr field;
3. Leave nconnhost and naddr, non-zero.

So trust in *pqReleaseConnHost *, to clean properly, It's insecure.

best regards,
Ranier Vilela