Hi,
There is a buildfarm failure on mylodon at [1] because of the new test
added. I will analyse and share the findings for the same.
[1] - https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mylodon&dt=2022-09-08%2001%3A40%3A27
Regards,
Vignesh
On Wed, 7 Sept 2022 at 17:10, vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, 7 Sept 2022 at 14:34, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Sep 7, 2022 at 9:53 AM vignesh C <vignesh21@gmail.com> wrote:
> > >
> > > Thanks for the comments, the attached v47 patch has the changes for the same.
> > >
> >
> > V47-0001* looks good to me apart from below minor things. I would like
> > to commit this tomorrow unless there are more comments on it.
> >
> > Few minor suggestions:
> > ==================
> > 1.
> > + list_free_deep(publist);
> > + pfree(pubnames->data);
> > + pfree(pubnames);
> >
> > I don't think we need to free this memory as it will automatically be
> > released at end of the command (this memory is allocated in portal
> > context). I understand there is no harm in freeing it as well but
> > better to leave it as there doesn't appear to be any danger of leaking
> > memory for a longer time.
>
> Modified
>
> > 2.
> > + res = walrcv_exec(wrconn, cmd.data, 1, tableRow);
> > + pfree(cmd.data);
> >
> > Don't you need to free cmd as well here? I think it is better to avoid
> > freeing it due to reasons mentioned in the previous point.
>
> cmd need not be freed here as we use initStringInfo for initialization
> and initStringInfo allocates memory for data member only.
>
> The attached patch has the changes for the same.
>
> Regards,
> Vignesh
