From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] libpq improvements and fixes |
Date: | 2020-02-14 11:07:15 |
Message-ID: | CAEudQArcJPhixHOJBRGd1qmCPOBMZbp=zh8gvn0mRmGOZQ6ayQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Em sex., 14 de fev. de 2020 às 03:13, Michael Paquier <michael(at)paquier(dot)xyz>
escreveu:
> On Thu, Feb 13, 2020 at 02:22:36PM -0300, Ranier Vilela wrote:
> > I just kept it, even if I duplicated the error message, the style was
> kept
> > and in my opinion it is much more coherent and readable.
> > But your solution is also good, and yes, it is worth it, because even
> with
> > small benefits, the change improves the code and prevents Coverity or
> > another tool from continuing to report false positives or not.
>
> Complaints from static analyzers need to be taken with a pinch of
> salt, and I agree with Tom here.
>
That's right, I will try avoid sending patches that only satisfy static
analysis tools.
> > Virtually no code will break for the change, since bool and int are
> > internally the same types.
> > I believe that no code will have either adjusted to work with corrected
> > functions, even if they use compiled libraries.
> > And again, it is worth correcting at least the static ones, because the
> > goal here, too, is to improve readability.
>
> FWIW, looking at the patch from upthread, I think that it is not that
> wise to blindly break the error compatibility handling of all PQsend*
> routines by switching the error handling of the connection to be after
> the compatibility checks, and all the other changes don't justify a
> breakage making back-patching more complicated nor do they improve
> readability at great lengths.
>
It is difficult to understand what you consider to be improvement.
Another programming principle I follow is to remove anything static from
loops that can be executed outside the loop.
In this specific case, from the loop modified in fe-exec, two branches were
removed, is this an improvement for you or not?
See patch attached.
regards,
Ranier Vilela
Attachment | Content-Type | Size |
---|---|---|
fe-exec.patch | application/octet-stream | 2.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2020-02-14 11:56:48 | Re: Parallel copy |
Previous Message | John Naylor | 2020-02-14 10:42:32 | Re: assert pg_class.relnatts is consistent |