From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c) |
Date: | 2023-09-27 13:37:45 |
Message-ID: | CAEudQAoDFONwejH8ZSBBq5jeQnweNDWFcCdH=Z94MntH6pdsTg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Em qua., 27 de set. de 2023 às 04:35, David Rowley <dgrowleyml(at)gmail(dot)com>
escreveu:
> On Wed, 27 Sept 2023 at 06:10, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> > As suggested, casting is the best option that does not add any overhead
> and improves the robustness of the find_base_rel function.
> > I propose patch v1, with the additional addition of fixing the
> find_base_rel_ignore_join function,
> > which despite not appearing in Coverity reports, suffers from the same
> problem.
>
> Can you confirm that this silences the Converity warning?
>
CID#1518088
This is a historical version of the file displaying the issue before it was
in the Fixed state.
> I think it probably warrants a comment to mention why we cast to uint32.
>
> e.g. /* perform an unsigned comparison so that we also catch negative
> relid values */
>
I'm ok.
>
> > Taking advantage, I also propose a scope reduction,
> > as well as the const of the root parameter, which is very appropriate.
>
> Can you explain why adding the const qualifier is "very appropriate"
> to catching negative relids?
>
Of course that has nothing to do with it.
> Please check [1] for the mention of:
>
> "The fastest way to get your patch rejected is to make unrelated
> changes. Reformatting lines that haven't changed, changing unrelated
> comments you felt were poorly worded, touching code not necessary to
> your change, etc. Each patch should have the minimum set of changes
> required to work robustly. If you do not follow the code formatting
> suggestions above, expect your patch to be returned to you with the
> feedback of "follow the code conventions", quite likely without any
> other review."
>
Forgive my impulsiveness, anyone who loves perfect, well written code,
would understand.
Do you have an objection to fixing the function find_base_rel_ignore_join?
Or is it included in unrelated changes?
Ranier Vilela
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-09-27 13:38:17 | Re: dikkop seems unhappy because of openssl stuff (FreeBSD 14-BETA1) |
Previous Message | tender wang | 2023-09-27 13:06:03 | Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop() |