Re: PATCH for BUG #18785: Pointer bmr.rel, dereferenced by passing as 1st parameter to function is checked for NULL later

В письме от четверг, 18 сентября 2025 г. 11:44:57 Москва, стандартное время
пользователь Алена Васильева написал:

> I propose adding an Assert at this point as a way to document the contract
> between this code and its environment.
> This was previously discussed in BUG #18785 in pqsql-bugs
> https://www.postgresql.org/message-id/flat/a6oxxee6blexicuark46yydtaqulsjvkr
> wkri6aic4vbofjxse%404a6j4kuwda7u#c52de413b182d9c42fe1eb34a82871b5

Hi!

I'd like to join reviewing process. I am already familiar with this issue from
previous discussions.

PROS:
This issue as I know have been found while applying static analyzer to
PostgreSQL code. This is not a bug per ce, everything works as expected, there
is certain calling convention that makes dubious situation unreachable.

But what static analyzer is actually reporting here: there is some calling
convention that have not been explicitly declared, and this might lead to
problems in future: somebody breaks this convention, but we will never know
about it.

From my point of view, the best way to declare calling convention in
PostgreSQL are Asserts + human readable comments explaining these Asserts.

This will stop static analyzer from raising false alarm, and these Asserts
will raise a true alarm if some developer tries to break that convention. And
comments near the Assert will hopefully help him understanding what is wrong
here.

So it is win-win situation: we improve code quality, and future users of
static analyzers are no longer bored with false alarms.

So in general this patch should be acceppted

CONS:
But this patch application should be redo from scratch.

1. If you want to submit patch, add it to the commit fest. Not only post it on
the mailing list.

2. Patch should be attach to a letter, not embedded in message text.
Commitfest robot will not be able to extract it from there

3. To prepare patch you should use `git format-patch` command. And please add
please create a prototype of a commit message that commiter will use to
creating final one, he will use while committing this patch. Do not make
committer invent it from scratch :-)

You can find more guides for patch submitting via these links:
https://wiki.postgresql.org/wiki/Submitting_a_Patch

https://peter.eisentraut.org/blog/2023/05/09/how-to-submit-a-patch-by-email-2023-edition

4.

> +   /*
> +    * Validate the contract between flags and bmr.rel.
> +    *

This "validate" word might accidentally confuse someone. Because Asserts in
postgres clearly do no validation, they are completely removed from code in
production builds, so no checks are done. Asserts are more a way of setting
traps for future call convention violator,s that will be triggered while
running tests, fuzzing or any other non-production activities that includes
turning Asserts on. So "valudate" is a bad word here....


So please submit patch accordingly and I will be a reviewer for it.


--
Nikolay Shaplov aka Nataraj
Fuzzing Engineer at Postgres Professional
Matrix IM: @dhyan:nataraj.su