On Sun, Mar 2, 2025 at 2:26 PM Mats Kindahl <m...@timescale.com> wrote:
> On Sat, Jan 18, 2025 at 8:44 PM Mats Kindahl <m...@timescale.com> wrote:
>
>> On Tue, Jan 14, 2025 at 4:19 PM Aleksander Alekseev <
>> aleksan...@timescale.com> wrote:
>>
>>> IMO the best solution would be re-submitting all the patches to this
>>> thread. Also please make sure the patchset is registered on the
>>> nearest open CF [1] This will ensure that the patchset is built on our
>>> CI (aka cfbot [2]) and will not be lost.
>>>
>>> [1]: https://commitfest.postgresql.org/
>>> [2]: http://cfbot.cputube.org/
>>>
>>>
>>
> Hi all,
>
> Here is a new set of patches rebased on the latest version of the
> Postgres. I decided to just include the semantic patches in each patch
> since it is trivial to generate the patch and build using:
>
>
> ninja coccicheck-patch | patch -d .. -p1 && ninja
>
>
> I repeat the description from the previous patch set and add comments
> where things have changed, but I have also added two semantic patches,
> which are described last.
>
> For those of you that are not aware of it: Coccinelle is a tool for
>> pattern matching and text transformation for C code and can be used for
>> detection of problematic programming patterns and to make complex,
>> tree-wide patches easy. It is aware of the structure of C code and is
>> better suited to make complicated changes than what is possible using
>> normal text substitution tools like Sed and Perl. I've noticed it's been
>> used at a few cases way back to fix issues.[1]
>>
>> Coccinelle have been successfully been used in the Linux project since
>> 2008 and is now an established tool for Linux development and a large
>> number of semantic patches have been added to the source tree to capture
>> everything from generic issues (like eliminating the redundant A in
>> expressions like "!A || (A && B)") to more Linux-specific problems like
>> adding a missing call to kfree().
>>
>> Although PostgreSQL is nowhere the size of the Linux kernel, it is
>> nevertheless of a significant size and would benefit from incorporating
>> Coccinelle into the development. I noticed it's been used in a few cases
>> way back (like 10 years back) to fix issues in the PostgreSQL code, but I
>> thought it might be useful to make it part of normal development practice
>> to, among other things:
>>
>> - Identify and correct bugs in the source code both during development
>> and review.
>> - Make large-scale changes to the source tree to improve the code based
>> on new insights.
>> - Encode and enforce APIs by ensuring that function calls are used
>> correctly.
>> - Use improved coding patterns for more efficient code.
>> - Allow extensions to automatically update code for later PostgreSQL
>> versions.
>>
>> To that end, I created a series of patches to show how it could be used
>> in the PostgreSQL tree. It is a lot easier to discuss concrete code and I
>> split it up into separate messages since that makes it easier to discuss
>> each individual patch. The series contains code to make it easy to work
>> with Coccinelle during development and reviews, as well as examples of
>> semantic patches that capture problems, demonstrate how to make large-scale
>> changes, how to enforce APIs, and also improve some coding patterns.
>>
>> The first three patches contain the coccicheck.py script and the
>> integration with the build system (both Meson and Autoconf).
>>
>> # Coccicheck Script
>>
>> It is a re-implementation of the coccicheck script that the Linux kernel
>> uses. We cannot immediately use the coccicheck script since it is quite
>> closely tied to the Linux source code tree and we need to have something
>> that both supports Autoconf and Meson. Since Python seems to be used more
>> and more in the tree, it seems to be the most natural choice. (I have no
>> strong opinion on what language to use, but think it would be good to have
>> something that is as platform-independent as possible.)
>>
>> The intention is that we should be able to use the Linux semantic patches
>> directly, so it supports the "Requires" and "Options" keywords, which can
>> be used to require a specific version of spatch(1) and add options to the
>> execution of that semantic patch, respectively.
>>
>
> I have added support for using multiple jobs similar to how "make -jN"
> works. This is also supported by the autoconf and ninja builds
>
>
>> # Autoconf support
>>
>> The changes to Autoconf modifies the configure.ac and related files (in
>> particular Makefile.global.in). At this point, I have deliberately not
>> added support for pgxs so extensions cannot use coccicheck through the
>> PostgreSQL installation. This is something that we can add later though.
>>
>> The semantic patches are expected to live in cocci/ directory under the
>> root and the patch uses the pattern cocci/**/*.cocci to find all semantic
>> patches. Right now there are no subdirectories for the semantic patches,
>> but this might be something we want to add to create different categories
>> of scripts.
>>
>> The coccicheck target is used in the same way as for the Linux kernel,
>> that is, to generate and apply all patches suggested by the semantic
>> patches, you type:
>>
>> make coccicheck MODE=patch | patch -p1
>>
>> Linux as support for a few more variables: V to set the verbosity, J to
>> use multiple jobs for processing the semantic patches, M to select a
>> different directory to apply the semantic patches to, and COCCI to use a
>> single specific semantic patch rather than all available. I have not added
>> support for this right now, but if you think this is valuable, it should be
>> straightforward to add.
>>
>> I used autoconf 2.69, as mentioned in configure.ac, but that generate a
>> bigger diff than I expected. Any advice here is welcome.
>>
>
> Using the parameter "JOBS" allow you to use multiple jobs, e.g.:
>
> make coccicheck MODE=patch JOBS=4 | patch -p1
>
>
>
>> # Meson Support
>>
>> The support for Meson is done by adding three coccicheck targets: one for
>> each mode. To apply all patches suggested by the semantic patches using
>> ninja (as is done in [2]), you type the following in the build directory
>> generated by Meson (e.g., the "build/" subdirectory).
>>
>> ninja coccicheck-patch | patch -p1 -d ..
>>
>> If you want to pass other flags you have to set the SPFLAGS environment
>> variable when calling ninja:
>>
>> SPFLAGS=--debug ninja coccicheck-report
>>
>
> If you want to use multiple jobs, you use something like this:
>
> JOBS=4 ninja coccicheck-patch | patch -d .. -p1
>
>
>
>> # Semantic Patch: Wrong type for palloc()
>>
>> This is the first example of a semantic patch and shows how to capture
>> and fix a common problem.
>>
>> If you use an palloc() to allocate memory for an object (or an array of
>> objects) and by mistake type something like:
>>
>> StringInfoData *info = palloc(sizeof(StringInfoData*));
>>
>> You will not allocate enough memory for storing the object. This semantic
>> patch catches any cases where you are either allocating an array of objects
>> or a single object that do not have corret types in this sense, more
>> precisely, it captures assignments to a variable of type T* where palloc()
>> uses sizeof(T) either alone or with a single expression (assuming this is
>> an array count).
>>
>> The semantic patch is overzealous in the sense that using the wrong
>> typedef will suggest a change (this can be seen in the patch). Although the
>> sizes of these are the same, it is probably be better to just follow the
>> convention of always using the type "T*" for any "palloc(sizeof(T))" since
>> the typedef can change at any point and would then introduce a bug.
>> Coccicheck can easily fix this for you, so it is straightforward to enforce
>> this. It also simplifies other automated checking to follow this convention.
>>
>> We don't really have any real bugs as a result from this, but we have one
>> case where an allocation of "sizeof(LLVMBasicBlockRef*)" is allocated to an
>> "LLVMBasicBlockRef*", which strictly speaking is not correct (it should be
>> "sizeof(LLVMBasicBlockRef)"). However, since they are both pointers, there
>> is no risk of incorrect allocation size. One typedef usage that does not
>> match.
>>
>> # Semantic Patch: Introduce palloc_array() and palloc_object() where
>> possible
>>
>> This is an example of a large-scale refactoring to improve the code.
>>
>> For PostgreSQL 16, Peter extended the palloc()/pg_malloc() interface in
>> commit 2016055a92f to provide more type-safety, but these functions are not
>> widely used. This semantic patch captures and replaces all uses of palloc()
>> where palloc_array() or palloc_object() could be used instead. It
>> deliberately does not touch cases where it is not clear that the
>> replacement can be done.
>>
>
> # Semantic Patch: replace code with pg_cmp_*
>
> This is an example of a large-scale refactoring to improve the code.
>
> In commit 3b42bdb4716 and 6b80394781c overflow-safe comparison functions
> were introduced, but they are not widely used. This semantic patch
> identifies some of the more common cases and replaces them with calls to
> the corresponding pg_cmp_* function.
>
> The patches give a few instructions of improvement in performance when
> checking with Godbolt. It's not much, but since it is so easy to apply
> them, it might still be worthwhile.
>
> # Semantic Patch: Replace dynamic allocation of StringInfo with
> StringInfoData
>
> Use improved coding patterns for more efficient code.
>
> This semantic patch replaces uses of StringInfo with StringInfoData where
> the info is dynamically allocated but (optionally) freed at the end of the
> block. This will avoid one dynamic allocation that otherwise has to be
> dealt with.
>
> For example, this code:
>
> StringInfo info = makeStringInfo();
> ...
> appendStringInfo(info, ...);
> ...
> return do_stuff(..., info->data, ...);
>
> Can be replaced with:
>
> StringInfoData info;
> initStringInfo(&info);
> ...
> appendStringInfo(&info, ...);
> ...
> return do_stuff(..., info.data, ...);
>
> It does not do a replacement in these cases:
>
> - If the variable is assigned to an expression. In this case, the
> pointer can "leak" outside the function either through a global variable or
> a parameter assignment.
> - If an assignment is done to the expression. This cannot leak the
> data, but could mean a value-assignment of a structure, so we avoid this
> case.
> - If the pointer is returned.
>
> The cases that this semantic patch fixed when I uploaded the first version
> of the other patches seems to have been dealt with, but having it as part
> of the code base prevents such cases from surfacing again.
>
>
>> [1]: https://coccinelle.gitlabpages.inria.fr/website/
>> [2]: https://www.postgresql.org/docs/current/install-meson.html
>>
> --
> Best wishes,
> Mats Kindahl, Timescale
>
Hi all,
There was a problem with the meson.build file causing errors in the build
farm (because spatch is not installed), so here is a new set of patches.
Only the one with the meson.build has changed, but I am unsure how patches
are picked up, so adding a new version of all files here.
--
Best wishes,
Mats Kindahl, Timescale
From 882f3b150c61884b9e176a1f506747c35235ab0a Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@kindahl.net>
Date: Sun, 29 Dec 2024 19:35:58 +0100
Subject: Add initial coccicheck script
The coccicheck.py script can be used to run several semantics patches on a
source tree to either generate a report, see the context of the modification
(what lines that requires changes), or generate a patch to correct an issue.
usage: coccicheck.py [-h] [--verbose] [--spatch SPATCH]
[--spflags SPFLAGS]
[--mode {patch,report,context}] [--jobs JOBS]
[--include DIR] [--patchdir DIR]
pattern path [path ...]
positional arguments:
pattern Pattern for Cocci files to use.
path Directory or source path to process.
options:
-h, --help show this help message and exit
--verbose, -v
--spatch SPATCH Path to spatch binary. Defaults to value of
environment variable SPATCH.
--spflags SPFLAGS Flags to pass to spatch call. Defaults to
value of enviroment variable SPFLAGS.
--mode {patch,report,context}
Mode to use for coccinelle. Defaults to
value of environment variable MODE.
--jobs JOBS Number of jobs to use for spatch. Defaults
to value of environment variable JOBS.
--include DIR, -I DIR
Extra include directories.
--patchdir DIR Path for which patch should be created
relative to.
---
src/tools/coccicheck.py | 185 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 185 insertions(+)
create mode 100755 src/tools/coccicheck.py
diff --git a/src/tools/coccicheck.py b/src/tools/coccicheck.py
new file mode 100755
index 00000000000..838f8184c54
--- /dev/null
+++ b/src/tools/coccicheck.py
@@ -0,0 +1,185 @@
+#!/usr/bin/env python3
+
+"""Run Coccinelle on a set of files and directories.
+
+This is a re-written version of the Linux ``coccicheck`` script.
+
+Coccicheck can run in two different modes (the original have four
+different modes):
+
+- *patch*: patch files using the cocci file.
+
+- *report*: report will report any improvements that this script can
+ make, but not show any patch.
+
+- *context*: show the context where the patch can be applied.
+
+The program will take a single cocci file and call spatch(1) with a
+set of paths that can be either files or directories.
+
+When starting, the cocci file will be parsed and any lines containing
+"Options:" or "Requires:" will be treated specially.
+
+- Lines containing "Options:" will have a list of options to add to
+ the call of the spatch(1) program. These options will be added last.
+
+- Lines containing "Requires:" can contain a version of spatch(1) that
+ is required for this cocci file. If the version requirements are not
+ satisfied, the file will not be used.
+
+When calling spatch(1), it will set the virtual rules "patch",
+"report", or "context" and the cocci file can use these to act
+differently depending on the mode.
+
+The following environment variables can be set:
+
+SPATCH: Path to spatch program. This will be used if no path is
+ passed using the option --spatch.
+
+SPFLAGS: Extra flags to use when calling spatch. These will be added
+ last.
+
+MODE: Mode to use. It will be used if no --mode is passed to
+ coccicheck.py.
+
+"""
+
+import argparse
+import os
+import sys
+import subprocess
+import re
+
+from pathlib import PurePath, Path
+from packaging import version
+
+VERSION_CRE = re.compile(
+ r'spatch version (\S+) compiled with OCaml version (\S+)'
+)
+
+
+def parse_metadata(cocci_file):
+ """Parse metadata in Cocci file."""
+ metadata = {}
+ with open(cocci_file) as fh:
+ for line in fh:
+ mre = re.match(r'(Options|Requires):(.*)', line, re.IGNORECASE)
+ if mre:
+ metadata[mre.group(1).lower()] = mre.group(2)
+ return metadata
+
+
+def get_config(args):
+ """Compute configuration information."""
+ # Figure out spatch version. We just need to read the first line
+ config = {}
+ cmd = [args.spatch, '--version']
+ with subprocess.Popen(cmd, stdout=subprocess.PIPE, text=True) as proc:
+ for line in proc.stdout:
+ mre = VERSION_CRE.match(line)
+ if mre:
+ config['spatch_version'] = mre.group(1)
+ break
+ return config
+
+
+def run_spatch(cocci_file, args, config, env):
+ """Run coccinelle on the provided file."""
+ if args.verbose > 1:
+ print("processing cocci file", cocci_file)
+ spatch_version = config['spatch_version']
+ metadata = parse_metadata(cocci_file)
+
+ # Check that we have a valid version
+ if 'required' in metadata:
+ required_version = version.parse(metadata['required'])
+ if required_version < spatch_version:
+ print(
+ f'Skipping SmPL patch {cocci_file}: '
+ f'requires {required_version} (had {spatch_version})'
+ )
+ return
+
+ command = [
+ args.spatch,
+ "-D", args.mode,
+ "--cocci-file", cocci_file,
+ "--very-quiet",
+ ]
+
+ if 'options' in metadata:
+ command.append(metadata['options'])
+ if args.mode == 'report':
+ command.append('--no-show-diff')
+ if args.patchdir:
+ command.extend(['--patch', args.patchdir])
+ if args.jobs:
+ command.extend(['--jobs', args.jobs])
+ if args.spflags:
+ command.append(args.spflags)
+
+ for path in args.path:
+ subprocess.run(command + [path], env=env, check=True)
+
+
+def coccinelle(args, config, env):
+ """Run coccinelle on all files matching the provided pattern."""
+ root = '/' if PurePath(args.cocci).is_absolute() else '.'
+ count = 0
+ for cocci_file in Path(root).glob(args.cocci):
+ count += 1
+ run_spatch(cocci_file, args, config, env)
+ return count
+
+
+def main(argv):
+ """Run coccicheck."""
+ parser = argparse.ArgumentParser()
+ parser.add_argument('--verbose', '-v', action='count', default=0)
+ parser.add_argument('--spatch', type=PurePath, metavar='SPATCH',
+ default=os.environ.get('SPATCH'),
+ help=('Path to spatch binary. Defaults to '
+ 'value of environment variable SPATCH.'))
+ parser.add_argument('--spflags', type=PurePath,
+ metavar='SPFLAGS',
+ default=os.environ.get('SPFLAGS', None),
+ help=('Flags to pass to spatch call. Defaults '
+ 'to value of enviroment variable SPFLAGS.'))
+ parser.add_argument('--mode', choices=['patch', 'report', 'context'],
+ default=os.environ.get('MODE', 'report'),
+ help=('Mode to use for coccinelle. Defaults to '
+ 'value of environment variable MODE.'))
+ parser.add_argument('--jobs', default=os.environ.get('JOBS', None),
+ help=('Number of jobs to use for spatch. Defaults to '
+ 'value of environment variable JOBS.'))
+ parser.add_argument('--include', '-I', type=PurePath,
+ metavar='DIR',
+ help='Extra include directories.')
+ parser.add_argument('--patchdir', type=PurePath, metavar='DIR',
+ help=('Path for which patch should be created '
+ 'relative to.'))
+ parser.add_argument('cocci', metavar='pattern',
+ help='Pattern for Cocci files to use.')
+ parser.add_argument('path', nargs='+', type=PurePath,
+ help='Directory or source path to process.')
+
+ args = parser.parse_args(argv)
+
+ if args.verbose > 1:
+ print("arguments:", args)
+
+ if args.spatch is None:
+ parser.error('spatch is part of the Coccinelle project and is '
+ 'available at http://coccinelle.lip6.fr/')
+
+ if coccinelle(args, get_config(args), os.environ) == 0:
+ parser.error(f'no coccinelle files found matching {args.cocci}')
+
+
+if __name__ == '__main__':
+ try:
+ main(sys.argv[1:])
+ except KeyboardInterrupt:
+ print("Execution aborted")
+ except Exception as exc:
+ print(exc)
--
2.43.0
From 7a0e89c7909e7a205f4724a11d73d00f38538e37 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@kindahl.net>
Date: Wed, 1 Jan 2025 14:15:51 +0100
Subject: Add meson build for coccicheck
This commit adds a run target `coccicheck` to meson build files.
Since ninja does not accept parameters the same way make does, there are three
run targets defined---"coccicheck-patch", "coccicheck-report", and
"coccicheck-context"---that you can use to generate a patch, get a report, and
get the context respectively. For example, to patch the tree from the "build"
subdirectory created by the meson run:
ninja coccicheck-patch | patch -d .. -p1
---
meson.build | 30 ++++++++++++++++++++++++++++++
meson_options.txt | 7 ++++++-
src/makefiles/meson.build | 6 ++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 13c13748e5d..0e0828f97f0 100644
--- a/meson.build
+++ b/meson.build
@@ -348,6 +348,7 @@ missing = find_program('config/missing', native: true)
cp = find_program('cp', required: false, native: true)
xmllint_bin = find_program(get_option('XMLLINT'), native: true, required: false)
xsltproc_bin = find_program(get_option('XSLTPROC'), native: true, required: false)
+spatch = find_program(get_option('SPATCH'), native: true, required: false)
bison_flags = []
if bison.found()
@@ -1642,6 +1643,34 @@ else
endif
+###############################################################
+# Option: Coccinelle checks
+###############################################################
+
+coccicheck_opt = get_option('coccicheck')
+coccicheck_dep = not_found_dep
+if not coccicheck_opt.disabled()
+ if spatch.found()
+ coccicheck_dep = declare_dependency()
+ elif coccicheck_opt.enabled()
+ error('missing required tools (spatch needed) for Coccinelle checks')
+ endif
+endif
+
+if coccicheck_opt.enabled()
+ coccicheck_modes = ['context', 'report', 'patch']
+ foreach mode : coccicheck_modes
+ run_target('coccicheck-' + mode,
+ command: [python, files('src/tools/coccicheck.py'),
+ '--mode', mode,
+ '--spatch', spatch,
+ '--patchdir', '@SOURCE_ROOT@',
+ '@SOURCE_ROOT@/cocci/**/*.cocci',
+ '@SOURCE_ROOT@/src',
+ '@SOURCE_ROOT@/contrib',
+ ])
+ endforeach
+endif
###############################################################
# Compiler tests
@@ -3808,6 +3837,7 @@ if meson.version().version_compare('>=0.57')
{
'bison': '@0@ @1@'.format(bison.full_path(), bison_version),
'dtrace': dtrace,
+ 'spatch': spatch,
'flex': '@0@ @1@'.format(flex.full_path(), flex_version),
},
section: 'Programs',
diff --git a/meson_options.txt b/meson_options.txt
index 702c4517145..37d6d43af93 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -43,6 +43,9 @@ option('cassert', type: 'boolean', value: false,
option('tap_tests', type: 'feature', value: 'auto',
description: 'Enable TAP tests')
+option('coccicheck', type: 'feature', value: 'auto',
+ description: 'Enable Coccinelle checks')
+
option('injection_points', type: 'boolean', value: false,
description: 'Enable injection points')
@@ -52,7 +55,6 @@ option('PG_TEST_EXTRA', type: 'string', value: '',
option('PG_GIT_REVISION', type: 'string', value: 'HEAD',
description: 'git revision to be packaged by pgdist target')
-
# Compilation options
option('extra_include_dirs', type: 'array', value: [],
@@ -195,6 +197,9 @@ option('PYTHON', type: 'array', value: ['python3', 'python'],
option('SED', type: 'string', value: 'gsed',
description: 'Path to sed binary')
+option('SPATCH', type: 'string', value: 'spatch',
+ description: 'Path to spatch binary, used for SmPL patches')
+
option('STRIP', type: 'string', value: 'strip',
description: 'Path to strip binary, used for PGXS emulation')
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index 60e13d50235..c66156d9046 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -57,6 +57,7 @@ pgxs_kv = {
'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
'enable_tap_tests': tap_tests_enabled ? 'yes' : 'no',
'enable_debug': get_option('debug') ? 'yes' : 'no',
+ 'enable_coccicheck': spatch.found() ? 'yes' : 'no',
'enable_coverage': 'no',
'enable_dtrace': dtrace.found() ? 'yes' : 'no',
@@ -151,6 +152,7 @@ pgxs_bins = {
'TAR': tar,
'ZSTD': program_zstd,
'DTRACE': dtrace,
+ 'SPATCH': spatch,
}
pgxs_empty = [
@@ -166,6 +168,10 @@ pgxs_empty = [
'DBTOEPUB',
'FOP',
+ # Coccinelle is not supported by pgxs
+ 'SPATCH',
+ 'SPFLAGS',
+
# supporting coverage for pgxs-in-meson build doesn't seem worth it
'GENHTML',
'LCOV',
--
2.43.0
From 3d7bca9422d5e9c851e42f442afeeb2dfc2104c3 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@kindahl.net>
Date: Sun, 5 Jan 2025 19:26:47 +0100
Subject: Semantic patch for sizeof() using palloc()
If palloc() is used to allocate elements of type T it should be assigned to a
variable of type T* or risk indexes out of bounds. This semantic patch checks
that allocations to variables of type T* are using sizeof(T) when allocating
memory using palloc().
---
cocci/palloc_sizeof.cocci | 49 +++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 cocci/palloc_sizeof.cocci
diff --git a/cocci/palloc_sizeof.cocci b/cocci/palloc_sizeof.cocci
new file mode 100644
index 00000000000..5f8593c2687
--- /dev/null
+++ b/cocci/palloc_sizeof.cocci
@@ -0,0 +1,49 @@
+virtual report
+virtual context
+virtual patch
+
+@initialize:python@
+@@
+import re
+
+CONST_CRE = re.compile(r'\bconst\b')
+
+def is_simple_type(s):
+ return s != 'void' and not CONST_CRE.search(s)
+
+@r1 depends on report || context@
+type T1 : script:python () { is_simple_type(T1) };
+idexpression T1 *I;
+type T2 != T1;
+position p;
+expression E;
+identifier func = {palloc, palloc0};
+@@
+(
+* I = func@p(sizeof(T2))
+|
+* I = func@p(E * sizeof(T2))
+)
+
+@script:python depends on report@
+T1 << r1.T1;
+T2 << r1.T2;
+I << r1.I;
+p << r1.p;
+@@
+coccilib.report.print_report(p[0], f"'{I}' has type '{T1}*' but 'sizeof({T2})' is used to allocate memory")
+
+@depends on patch@
+type T1 : script:python () { is_simple_type(T1) };
+idexpression T1 *I;
+type T2 != T1;
+expression E;
+identifier func = {palloc, palloc0};
+@@
+(
+- I = func(sizeof(T2))
++ I = func(sizeof(T1))
+|
+- I = func(E * sizeof(T2))
++ I = func(E * sizeof(T1))
+)
--
2.43.0
From 1c0e47883dca57ab9febf9af8b28bffa1e75c4f0 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@kindahl.net>
Date: Mon, 30 Dec 2024 19:58:07 +0100
Subject: Create coccicheck target for autoconf
This adds a coccicheck target for the autoconf-based build system. The
coccicheck target accepts one parameter MODE, which can be either "patch",
"report", or "context". The "patch" mode will generate a patch that can be
applied to the source tree, the "report" mode will generate a list of file
locations with information about what can be changed, and the "context" mode
will just highlight the line that will be affected by the semantic patch.
The following will generate a patch and apply it to the source code tree:
make coccicheck MODE=patch | patch -p1
---
configure | 100 ++++++++++++++++++++++++++++++++++++++---
configure.ac | 12 +++++
src/Makefile.global.in | 24 +++++++++-
src/makefiles/pgxs.mk | 3 ++
4 files changed, 132 insertions(+), 7 deletions(-)
diff --git a/configure b/configure
index 93fddd69981..109a4868de8 100755
--- a/configure
+++ b/configure
@@ -772,6 +772,9 @@ enable_coverage
GENHTML
LCOV
GCOV
+enable_coccicheck
+SPFLAGS
+SPATCH
enable_debug
enable_rpath
default_port
@@ -839,6 +842,7 @@ with_pgport
enable_rpath
enable_debug
enable_profiling
+enable_coccicheck
enable_coverage
enable_dtrace
enable_tap_tests
@@ -1534,6 +1538,7 @@ Optional Features:
executables
--enable-debug build with debugging symbols (-g)
--enable-profiling build with profiling enabled
+ --enable-coccicheck enable Coccinelle checks (requires spatch)
--enable-coverage build with coverage testing instrumentation
--enable-dtrace build with DTrace support
--enable-tap-tests enable TAP tests (requires Perl and IPC::Run)
@@ -3330,6 +3335,91 @@ fi
+#
+# --enable-coccicheck enables Coccinelle check target "coccicheck"
+#
+
+
+# Check whether --enable-coccicheck was given.
+if test "${enable_coccicheck+set}" = set; then :
+ enableval=$enable_coccicheck;
+ case $enableval in
+ yes)
+ if test -z "$SPATCH"; then
+ for ac_prog in spatch
+do
+ # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_SPATCH+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $SPATCH in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_SPATCH="$SPATCH" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_SPATCH="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ ;;
+esac
+fi
+SPATCH=$ac_cv_path_SPATCH
+if test -n "$SPATCH"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $SPATCH" >&5
+$as_echo "$SPATCH" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+ test -n "$SPATCH" && break
+done
+
+else
+ # Report the value of SPATCH in configure's output in all cases.
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for SPATCH" >&5
+$as_echo_n "checking for SPATCH... " >&6; }
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $SPATCH" >&5
+$as_echo "$SPATCH" >&6; }
+fi
+
+if test -z "$SPATCH"; then
+ as_fn_error $? "spatch not found" "$LINENO" 5
+fi
+
+ ;;
+ no)
+ :
+ ;;
+ *)
+ as_fn_error $? "no argument expected for --enable-coccicheck option" "$LINENO" 5
+ ;;
+ esac
+
+else
+ enable_coccicheck=no
+
+fi
+
+
+
+
#
# --enable-coverage enables generation of code coverage metrics with gcov
#
@@ -14998,7 +15088,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -15044,7 +15134,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -15068,7 +15158,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -15113,7 +15203,7 @@ else
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
@@ -15137,7 +15227,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
We can't simply define LARGE_OFF_T to be 9223372036854775807,
since some C++ compilers masquerading as C compilers
incorrectly reject 9223372036854775807. */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
&& LARGE_OFF_T % 2147483647 == 1)
? 1 : -1];
diff --git a/configure.ac b/configure.ac
index b6d02f5ecc7..fdcda3a2d57 100644
--- a/configure.ac
+++ b/configure.ac
@@ -199,6 +199,18 @@ AC_SUBST(enable_debug)
PGAC_ARG_BOOL(enable, profiling, no,
[build with profiling enabled ])
+#
+# --enable-coccicheck enables Coccinelle check target "coccicheck"
+#
+PGAC_ARG_BOOL(enable, coccicheck, no,
+ [enable Coccinelle checks (requires spatch)],
+[PGAC_PATH_PROGS(SPATCH, spatch)
+if test -z "$SPATCH"; then
+ AC_MSG_ERROR([spatch not found])
+fi
+AC_SUBST(SPFLAGS)])
+AC_SUBST(enable_coccicheck)
+
#
# --enable-coverage enables generation of code coverage metrics with gcov
#
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 3b620bac5ac..cf603e20b7e 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -19,7 +19,7 @@
#
# Meta configuration
-standard_targets = all install installdirs uninstall clean distclean coverage check checkprep installcheck init-po update-po
+standard_targets = all install installdirs uninstall clean distclean coccicheck coverage check checkprep installcheck init-po update-po
# these targets should recurse even into subdirectories not being built:
standard_always_targets = clean distclean
@@ -201,6 +201,7 @@ enable_rpath = @enable_rpath@
enable_nls = @enable_nls@
enable_debug = @enable_debug@
enable_dtrace = @enable_dtrace@
+enable_coccicheck = @enable_coccicheck@
enable_coverage = @enable_coverage@
enable_injection_points = @enable_injection_points@
enable_tap_tests = @enable_tap_tests@
@@ -374,7 +375,7 @@ CLDR_VERSION = 45
# If a particular subdirectory knows this isn't needed in itself or its
# children, it can set NO_GENERATED_HEADERS.
-all install check installcheck: submake-generated-headers
+all install check installcheck coccicheck: submake-generated-headers
.PHONY: submake-generated-headers
@@ -523,6 +524,11 @@ FOP = @FOP@
XMLLINT = @XMLLINT@
XSLTPROC = @XSLTPROC@
+# Coccinelle
+
+SPATCH = @SPATCH@
+SPFLAGS = @SPFLAGS@
+
# Code coverage
GCOV = @GCOV@
@@ -993,6 +999,20 @@ endif # nls.mk
endif # enable_nls
+##########################################################################
+#
+# Coccinelle checks
+#
+
+ifeq ($(enable_coccicheck), yes)
+coccicheck_py = $(top_srcdir)/src/tools/coccicheck.py
+coccicheck = SPATCH=$(SPATCH) SPFLAGS=$(SPFLAGS) $(PYTHON) $(coccicheck_py)
+
+.PHONY: coccicheck
+coccicheck:
+ $(coccicheck) --mode=$(MODE) 'cocci/**/*.cocci' $(top_srcdir)
+endif # enable_coccicheck
+
##########################################################################
#
# Coverage
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 0de3737e789..144459dccd2 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -95,6 +95,9 @@ endif
ifeq ($(FLEX),)
FLEX = flex
endif
+ifeq ($(SPATCH),)
+SPATCH = spatch
+endif
endif # PGXS
--
2.43.0
From f5f4d31746ed21233eeff46f21e48938c4d20fef Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@kindahl.net>
Date: Sun, 29 Dec 2024 20:23:25 +0100
Subject: Semantic patch for palloc_array and palloc_object
Macros were added to the palloc API in commit 2016055a92f to improve
type-safety, but very few instances were replaced. This adds a cocci script to
do that replacement. The semantic patch deliberately do not replace instances
where the type of the variable and the type used in the macro does not match.
---
cocci/palloc_array.cocci | 157 +++++++++++++++++++++++++++++++++++++++
1 file changed, 157 insertions(+)
create mode 100644 cocci/palloc_array.cocci
diff --git a/cocci/palloc_array.cocci b/cocci/palloc_array.cocci
new file mode 100644
index 00000000000..aeeab74c3a9
--- /dev/null
+++ b/cocci/palloc_array.cocci
@@ -0,0 +1,157 @@
+// Since PG16 there are array versions of common palloc operations, so
+// we can use those instead.
+//
+// We ignore cases where we have a anonymous struct and also when the
+// type of the variable being assigned to is different from the
+// inferred type.
+//
+// Options: --no-includes --include-headers
+
+virtual patch
+virtual report
+virtual context
+
+// These rules (soN) are needed to rewrite types of the form
+// sizeof(T[C]) to C * sizeof(T) since Cocci cannot (currently) handle
+// it.
+@initialize:python@
+@@
+import re
+
+CRE = re.compile(r'(.*)\s+\[\s+(\d+)\s+\]$')
+
+def is_array_type(s):
+ mre = CRE.match(s)
+ return (mre is not None)
+
+@so1 depends on patch@
+type T : script:python() { is_array_type(T) };
+@@
+palloc(sizeof(T))
+
+@script:python so2 depends on patch@
+T << so1.T;
+T2;
+E;
+@@
+mre = CRE.match(T)
+coccinelle.T2 = cocci.make_type(mre.group(1))
+coccinelle.E = cocci.make_expr(mre.group(2))
+
+@depends on patch@
+type so1.T;
+type so2.T2;
+expression so2.E;
+@@
+- palloc(sizeof(T))
++ palloc(E * sizeof(T2))
+
+@r1 depends on report || context@
+type T !~ "^struct {";
+expression E;
+position p;
+idexpression T *I;
+identifier alloc = {palloc0, palloc};
+@@
+* I = alloc@p(E * sizeof(T))
+
+@script:python depends on report@
+p << r1.p;
+alloc << r1.alloc;
+@@
+coccilib.report.print_report(p[0], f"this {alloc} can be replaced with {alloc}_array")
+
+@depends on patch@
+type T !~ "^struct {";
+expression E;
+T *P;
+idexpression T* I;
+constant C;
+identifier alloc = {palloc0, palloc};
+fresh identifier alloc_array = alloc ## "_array";
+@@
+(
+- I = (T*) alloc(E * sizeof( \( *P \| P[C] \) ))
++ I = alloc_array(T, E)
+|
+- I = (T*) alloc(E * sizeof(T))
++ I = alloc_array(T, E)
+|
+- I = alloc(E * sizeof( \( *P \| P[C] \) ))
++ I = alloc_array(T, E)
+|
+- I = alloc(E * sizeof(T))
++ I = alloc_array(T, E)
+)
+
+@r3 depends on report || context@
+type T !~ "^struct {";
+expression E;
+idexpression T *P;
+idexpression T *I;
+position p;
+@@
+* I = repalloc@p(P, E * sizeof(T))
+
+@script:python depends on report@
+p << r3.p;
+@@
+coccilib.report.print_report(p[0], "this repalloc can be replaced with repalloc_array")
+
+@depends on patch@
+type T !~ "^struct {";
+expression E;
+idexpression T *P1;
+idexpression T *P2;
+idexpression T *I;
+constant C;
+@@
+(
+- I = (T*) repalloc(P1, E * sizeof( \( *P2 \| P2[C] \) ))
++ I = repalloc_array(P1, T, E)
+|
+- I = (T*) repalloc(P1, E * sizeof(T))
++ I = repalloc_array(P1, T, E)
+|
+- I = repalloc(P1, E * sizeof( \( *P2 \| P2[C] \) ))
++ I = repalloc_array(P1, T, E)
+|
+- I = repalloc(P1, E * sizeof(T))
++ I = repalloc_array(P1, T, E)
+)
+
+@r4 depends on report || context@
+type T !~ "^struct {";
+position p;
+idexpression T* I;
+identifier alloc = {palloc, palloc0};
+@@
+* I = alloc@p(sizeof(T))
+
+@script:python depends on report@
+p << r4.p;
+alloc << r4.alloc;
+@@
+coccilib.report.print_report(p[0], f"this {alloc} can be replaced with {alloc}_object")
+
+@depends on patch@
+type T !~ "^struct {";
+T* P;
+idexpression T *I;
+constant C;
+identifier alloc = {palloc, palloc0};
+fresh identifier alloc_object = alloc ## "_object";
+@@
+(
+- I = (T*) alloc(sizeof( \( *P \| P[C] \) ))
++ I = alloc_object(T)
+|
+- I = (T*) alloc(sizeof(T))
++ I = alloc_object(T)
+|
+- I = alloc(sizeof( \( *P \| P[C] \) ))
++ I = alloc_object(T)
+|
+- I = alloc(sizeof(T))
++ I = alloc_object(T)
+)
--
2.43.0
From 3362026ed695b6a564eaba0c8f5fbd2bda1f53da Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@kindahl.net>
Date: Tue, 28 Jan 2025 14:09:41 +0100
Subject: Semantic patch to use stack-allocated StringInfoData
This semantic patch replace uses of StringInfo with StringInfoData where the
info is dynamically allocated but (optionally) freed at the end of the block.
This will avoid one dynamic allocation that otherwise have to be dealt with.
For example, this code:
StringInfo info = makeStringInfo();
...
appendStringInfo(info, ...);
...
return do_stuff(..., info->data, ...);
Can be replaced with:
StringInfoData info;
initStringInfo(&info);
...
appendStringInfo(&info, ...);
...
return do_stuff(..., info.data, ...);
It does not do a replacement in these cases:
- If the variable is assigned to an expression. In this case, the pointer can
"leak" outside the function either through a global variable or a parameter
assignment.
- If an assignment is done to the expression. This cannot leak the data, but
could mean a value-assignment of a structure, so we avoid this case.
- If the pointer is returned.
---
cocci/use_stringinfodata.cocci | 155 +++++++++++++++++++++++++++++++++
1 file changed, 155 insertions(+)
create mode 100644 cocci/use_stringinfodata.cocci
diff --git a/cocci/use_stringinfodata.cocci b/cocci/use_stringinfodata.cocci
new file mode 100644
index 00000000000..4186027f8c9
--- /dev/null
+++ b/cocci/use_stringinfodata.cocci
@@ -0,0 +1,155 @@
+// Replace uses of StringInfo with StringInfoData where the info is
+// dynamically allocated but (optionally) freed at the end of the
+// block. This will avoid one dynamic allocation that otherwise have
+// to be dealt with.
+//
+// For example, this code:
+//
+// StringInfo info = makeStringInfo();
+// ...
+// appendStringInfo(info, ...);
+// ...
+// return do_stuff(..., info->data, ...);
+//
+// Can be replaced with:
+//
+// StringInfoData info;
+// initStringInfo(&info);
+// ...
+// appendStringInfo(&info, ...);
+// ...
+// return do_stuff(..., info.data, ...);
+
+virtual report
+virtual context
+virtual patch
+
+// This rule captures the position of the makeStringInfo() and bases
+// all changes around that. It matches the case that we should *not*
+// replace, that is, those that either (1) return the pointer or (2)
+// assign the pointer to a variable or (3) assign a variable to the
+// pointer.
+//
+// The first two cases are matched because they could potentially leak
+// the pointer outside the function, for some expressions, but the
+// last one is just a convenience.
+//
+// If we replace this, the resulting change will result in a value
+// copy of a structure, which might not be optimal, so we do not do a
+// replacement.
+@id1 exists@
+typedef StringInfo;
+local idexpression StringInfo info;
+position pos;
+expression E;
+@@
+ info@pos = makeStringInfo()
+ ...
+(
+ return info;
+|
+ info = E
+|
+ E = info
+)
+
+@r1 depends on !patch disable decl_init exists@
+identifier info, fld;
+position dpos, pos != id1.pos;
+@@
+(
+* StringInfo@dpos info;
+ ...
+* info@pos = makeStringInfo();
+|
+* StringInfo@dpos info@pos = makeStringInfo();
+)
+<...
+(
+* \(pfree\|destroyStringInfo\)(info);
+|
+* info->fld
+|
+* *info
+|
+* info
+)
+...>
+
+@script:python depends on report@
+info << r1.info;
+dpos << r1.dpos;
+@@
+coccilib.report.print_report(dpos[0], f"Variable '{info}' of type StringInfo can be defined using StringInfoData")
+
+@depends on patch disable decl_init exists@
+identifier info, fld;
+position pos != id1.pos;
+@@
+- StringInfo info;
++ StringInfoData info;
+ ...
+- info@pos = makeStringInfo();
++ initStringInfo(&info);
+<...
+(
+- \(destroyStringInfo\|pfree\)(info);
+|
+ info
+- ->fld
++ .fld
+|
+- *info
++ info
+|
+- info
++ &info
+)
+...>
+
+// Here we repeat the matching of the "bad case" since we cannot
+// inherit over modifications
+@id2 exists@
+typedef StringInfo;
+local idexpression StringInfo info;
+position pos;
+expression E;
+@@
+ info@pos = makeStringInfo()
+ ...
+(
+ return info;
+|
+ info = E
+|
+ E = info
+)
+
+@depends on patch exists@
+identifier info, fld;
+position pos != id2.pos;
+statement S, S1;
+@@
+- StringInfo info@pos = makeStringInfo();
++ StringInfoData info;
+ ... when != S
+(
+<...
+(
+- \(destroyStringInfo\|pfree\)(info);
+|
+ info
+- ->fld
++ .fld
+|
+- *info
++ info
+|
+- info
++ &info
+)
+...>
+&
++ initStringInfo(&info);
+ S1
+)
--
2.43.0
From fa1f167246fe97aa26811898d7e19f456bd69896 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@kindahl.net>
Date: Thu, 23 Jan 2025 02:46:14 +0100
Subject: Semantic patch for pg_cmp_* functions
In commit 3b42bdb4716 and 6b80394781c overflow-safe comparison functions where
introduced, but they are not widely used. This semantic patch identifies some
of the more common cases and replaces them with calls to the corresponding
pg_cmp_* function.
---
cocci/use_pg_cmp.cocci | 125 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 125 insertions(+)
create mode 100644 cocci/use_pg_cmp.cocci
diff --git a/cocci/use_pg_cmp.cocci b/cocci/use_pg_cmp.cocci
new file mode 100644
index 00000000000..8a258e61e5d
--- /dev/null
+++ b/cocci/use_pg_cmp.cocci
@@ -0,0 +1,125 @@
+// Find cases where we can use the new pg_cmp_* functions.
+//
+// Copyright 2025 Mats Kindahl, Timescale.
+//
+// Options: --no-includes --include-headers
+
+virtual report
+virtual context
+virtual patch
+
+@initialize:python@
+@@
+
+import re
+
+TYPMAP = {
+ 'BlockNumber': 'pg_cmp_u32',
+ 'ForkNumber': 'pg_cmp_s32',
+ 'OffsetNumber': 'pg_cmp_s16',
+ 'int': 'pg_cmp_s32',
+ 'int16': 'pg_cmp_s16',
+ 'int32': 'pg_cmp_s32',
+ 'uint16': 'pg_cmp_u16',
+ 'uint32': 'pg_cmp_u32',
+ 'unsigned int': 'pg_cmp_u32',
+}
+
+def is_valid(expr):
+ return not re.search(r'DatumGet[A-Za-z]+', expr)
+
+@r1e depends on context || report expression@
+type TypeName : script:python() { TypeName in TYPMAP };
+position pos;
+TypeName lhs : script:python() { is_valid(lhs) };
+TypeName rhs : script:python() { is_valid(rhs) };
+@@
+* lhs@pos < rhs ? -1 : lhs > rhs ? 1 : 0
+
+@script:python depends on report@
+lhs << r1e.lhs;
+rhs << r1e.rhs;
+pos << r1e.pos;
+@@
+coccilib.report.print_report(pos[0], f"conditional checks between '{lhs}' and '{rhs}' can be replaced with a PostgreSQL comparison function")
+
+@r1 depends on context || report@
+type TypeName : script:python() { TypeName in TYPMAP };
+position pos;
+TypeName lhs : script:python() { is_valid(lhs) };
+TypeName rhs : script:python() { is_valid(rhs) };
+@@
+(
+* if@pos (lhs < rhs) return -1; else if (lhs > rhs) return 1; return 0;
+|
+* if@pos (lhs < rhs) return -1; else if (lhs > rhs) return 1; else return 0;
+|
+* if@pos (lhs < rhs) return -1; if (lhs > rhs) return 1; return 0;
+|
+* if@pos (lhs > rhs) return 1; if (lhs < rhs) return -1; return 0;
+|
+* if@pos (lhs == rhs) return 0; if (lhs > rhs) return 1; return -1;
+|
+* if@pos (lhs == rhs) return 0; return lhs > rhs ? 1 : -1;
+|
+* if@pos (lhs == rhs) return 0; return lhs < rhs ? -1 : 1;
+)
+
+@script:python depends on report@
+lhs << r1.lhs;
+rhs << r1.rhs;
+pos << r1.pos;
+@@
+coccilib.report.print_report(pos[0], f"conditional checks between '{lhs}' and '{rhs}' can be replaced with a PostgreSQL comparison function")
+
+@expr_repl depends on patch expression@
+type TypeName : script:python() { TypeName in TYPMAP };
+fresh identifier cmp = script:python(TypeName) { TYPMAP[TypeName] };
+TypeName lhs : script:python() { is_valid(lhs) };
+TypeName rhs : script:python() { is_valid(rhs) };
+@@
+- lhs < rhs ? -1 : lhs > rhs ? 1 : 0
++ cmp(lhs,rhs)
+
+@stmt_repl depends on patch@
+type TypeName : script:python() { TypeName in TYPMAP };
+fresh identifier cmp = script:python(TypeName) { TYPMAP[TypeName] };
+TypeName lhs : script:python() { is_valid(lhs) };
+TypeName rhs : script:python() { is_valid(rhs) };
+@@
+(
+- if (lhs < rhs) return -1; if (lhs > rhs) return 1; return 0;
++ return cmp(lhs,rhs);
+|
+- if (lhs < rhs) return -1; else if (lhs > rhs) return 1; return 0;
++ return cmp(lhs,rhs);
+|
+- if (lhs < rhs) return -1; else if (lhs > rhs) return 1; else return 0;
++ return cmp(lhs,rhs);
+|
+- if (lhs > rhs) return 1; if (lhs < rhs) return -1; return 0;
++ return cmp(lhs,rhs);
+|
+- if (lhs > rhs) return 1; else if (lhs < rhs) return -1; return 0;
++ return cmp(lhs,rhs);
+|
+- if (lhs == rhs) return 0; if (lhs > rhs) return 1; return -1;
++ return cmp(lhs,rhs);
+|
+- if (lhs == rhs) return 0; return lhs > rhs ? 1 : -1;
++ return cmp(lhs,rhs);
+|
+- if (lhs == rhs) return 0; return lhs < rhs ? -1 : 1;
++ return cmp(lhs,rhs);
+)
+
+// Add an include if there were none and we had to do some
+// replacements
+@has_include depends on patch@
+@@
+ #include "common/int.h"
+
+@depends on patch && !has_include && (stmt_repl || expr_repl)@
+@@
+ #include ...
++ #include "common/int.h"
--
2.43.0
