On Thu, Apr 3, 2025 at 10:26 AM Andres Freund <and...@anarazel.de> wrote:
>
> Hi,
>
> On 2025-04-03 09:58:30 +1100, Peter Smith wrote:
> > I saw that a new errhint_internal() function was recently committed
> > [1]. I had also posted above asking about this same missing function a
> > month ago [2].
> >
> > But, your patch only added the new function -- it does not make any
> > use of it for existing code that was using the errhint("%s", str)
> > method.
> >
> > I wondered, given your commit message "but that's not exactly pretty
> > and makes it harder to avoid memory leaks", if you think it is
> > worthwhile to revisit those existing "%s" usages and modify them to
> > use the new errhint_internal? Tom above [3] seemed not keen to modify
> > those without performance reasons, although at that time
> > errhint_internal didn't even exist.
>
> I'd not go around and just categorically convert all users of errhint("%s",
> str), that's probably not worth the noise. And plenty of them won't
> benefit. E.g. the first one I just looked at is dblink_res_error(), where I
> don't think using it would bring meaningful benefit. I suspect a bunch of
> other places are going to be similar.
>
Yes, I found 19 examples, but they are all similar to that.
I think they could all be changed
FROM
errhint("%s", str)
TO one of
errhint_internal("%s", str)
errhint_internal(str)
...but due to noise/benefit trade-off, I won't bother.
Thanks for your feedback.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
