PostgreSQL's approach to assertion usage: seeking best practices - Mailing list pgsql-hackers

From Alexander Kuznetsov
Subject PostgreSQL's approach to assertion usage: seeking best practices
Date
Msg-id 5273c2cb-1bde-413b-8bec-a87ec6bfc6b5@altlinux.org
Whole thread Raw
Responses Re: PostgreSQL's approach to assertion usage: seeking best practices
List pgsql-hackers
Hello everyone,

The ALT Linux Team has recently initiated a focused testing effort on PostgreSQL, with an emphasis on its security
aspects.

As part of this effort, we conducted static analysis using Svace, which raised some questions regarding the use of the
Assert()function.
 
We were unable to find clear answers in the documentation or previous discussions and would greatly appreciate your
insights,
as these will influence how we approach future patch submissions:

1. General purpose of Assert() function:
- Is the primary purpose of the Assert() function to:
     - Inform developers of the assumptions made in the code,
     - Facilitate testing of new builds with assertions enabled,
     - Accelerate development by temporarily placing assertions where defensive code may later be required,
     - Or does it serve some other purpose?

2. Assertions and defensive code:
We have observed patches where assertions were added to defensive code (e.g., [1]) and where defensive code was added
toassertions.
 
Is it generally advisable and encouraged to incorporate defensive code into assertions' assumptions?

For instance, we encountered cases where a null pointer dereference occurs immediately after an assertion that the
pointeris not null.
 
In assertion-enabled builds, this results in an assertion failure, while in non-assert builds, it leads to a
dereference,which we aim to avoid.
 
However, it is unclear to us whether the use of an assertion in such cases indicates that this flaw is known and will
notbe addressed specifically,
 
or if additional protective code should be introduced.

A clearer understanding of whether assertions should signal potential failure points that might later be rewritten or
protectedwould assist us
 
in quickly developing fixes for such cases, particularly where we believe the issue could occur in practice.

3. Approach to fixing flaws:
How should we generally fix the flaws - by adding protection code, or by adding assertions?
I previously submitted two patches and encountered some confusion based on the feedback: in one instance,
I added protective code but was asked whether an assertion would suffice [2],
and in another, I added an assertion but was questioned on its necessity, given that it would cause a failure
regardless,which I agreed with [3].
 


Your guidance on these matters would be invaluable in helping us align our patch submissions with the community’s best
practices.

Thank you in advance for your time and assistance.

1. https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=30aaab26e52144097a1a5bbb0bb66ea1ebc0cb81
2. https://www.postgresql.org/message-id/e22df993-cdb4-4d0a-b629-42211ebed582%40altlinux.org
3. https://www.postgresql.org/message-id/6d0323c3-3f5d-4137-af73-98a5ab90e77c%40altlinux.org

-- 
Best regards,
Alexander Kuznetsov



pgsql-hackers by date:

Previous
From: Michael Banck
Date:
Subject: Re: Improve error message for ICU libraries if pkg-config is absent
Next
From: Junwang Zhao
Date:
Subject: Re: [Patch] remove duplicated smgrclose