This is master process code and should not use exit on malloc failure. This actually shows issue that the return value is not checked. Basically the idea is that in such situation we prefer OS to kill one of the children.

Right, I added this only to code that doesn't already check for NULL, i.e. to abort gracefully rather than crash with a NULL pointer access. This is an issue that should be solved separately then.

This might be also worth checking if exit would make sense here. I haven't checked it out.

Can we take the opportunity of the churn to also avoid repeating the type unnecessarily?

see: https://github.com/haproxy/haproxy/blob/master/dev/coccinelle/xalloc_size.cocci

I don't usually use the sizeof(*ht) style, and I don't think most people do. I don't particularly care, but I think moving towards one style only makes sense if it becomes policy. Maybe suggest it on Slack first?

We are not going to change all such usage so I would prefer to stick with full types. I find it also more readable.

We are not going to change all such usage

What do you mean by "we are not going to"?

I'm not sure if you meant doing just for pmalloc of changing every single sizeof.

If former, then I don't think there is much point in creating inconsistency that is less readable. If latter, you don't get full agreement as it would be a big churn so you would probably have to go through RFC which is not exactly meant for internal changes like this. It reminds me those header includes refactoring where we ended up not having any process to decide it.

It can also be used as some kind of static analyzer to find incorrect constructs (e.g. cases where the types already differ).

This is what I'm most interested in. It's hard to remember rules, and it's even harder for everyone to remember all rules. IMO, guidelines are most useful when they are enforced, even if only for the 95% cases. At least that helps you internalize the rules.

Regardless, I think such a decision would need discussion, though I fully acknowledge there's no good channel to do so. I think Slack (e.g. #php-src) would make the most sense.

This kind of conflict is the easiest possible conflict to resolve: It affects a single line that doesn't require much thought.

But it's a still delay to check it, edit and commit for something that has no value and is not even an improvement.

It depends on the change. There's a non-trivial amount of time spent in PRs discussing the same nits (e.g. prefer /* */ over //, though I have no idea whether these kinds of things are solvable with Coccinelle).

There's a non-trivial amount of time spent in PRs discussing the same nits (e.g. prefer /* */ over //, though I have no idea whether these kinds of things are solvable with Coccinelle).

That example would probably rather be clang-format. I believe Coccinelle can also deal with comments, but the intended use case is making changes to the code based on type and control-flow information with a convenient syntax that resembles a patch file and thus is easy to grok even for less experienced users.

To use malloc() as an example:

@@
type T;
T *x;
@@
 x = malloc(
- sizeof(T)
+ sizeof(*x)
 )

This defines placeholders of an arbitrary type T and a variable x that is a pointer to T. It then searches for constructs x = malloc(sizeof(T)) and replaces the sizeof(T) by sizeof(*x).

For a C program:

#include <stdlib.h>
int
main(void) {
	int *foo = malloc(sizeof(int));
	int *bar = malloc(sizeof(double));
}

running that Coccinelle patch would make the following changes:

$ spatch -sp_file example.cocci test.c 
init_defs_builtins: /usr/lib/coccinelle/standard.h
HANDLING: test.c
diff = 
--- test.c
+++ /tmp/cocci-output-4310-cbc9b9-test.c
@@ -2,6 +2,6 @@
 
 int
 main(void) {
-	int *foo = malloc(sizeof(int));
+	int *foo = malloc(sizeof(*foo));
 	int *bar = malloc(sizeof(double));
 }

If we now wanted to find places where the types differ, we could use the following patch:

@@
type T;
type U;
T *x;
@@
 x = malloc(
(
 sizeof(T)
|
 sizeof(*x)
|
* sizeof(U)
)
 )

The * is an indicator to just "highlight" the match and not perform any replacement. We define an additional type U and then define multiple alternatives. For the correct variants sizeof(T) and sizeof(*x) nothing is reported. For sizeof(U) (where U != T, since T was already handled), a report is created:

$ spatch -sp_file example.cocci test.c 
init_defs_builtins: /usr/lib/coccinelle/standard.h
HANDLING: test.c
diff = 
--- test.c
+++ /tmp/cocci-output-4315-fa4dae-test.c
@@ -3,5 +3,4 @@
 int
 main(void) {
 	int *foo = malloc(sizeof(int));
-	int *bar = malloc(sizeof(double));
 }

Coccinelle works very well and is easy to use. I've used it a few times and it's been battle tested against the Linux kernel, which isn't particularly known for it's consistent coding style and is known for it's complex macros etc.
I'm in favor to use the malloc sizeof pattern Tim proposes, and if we don't want to convert existing code (which I think we should), we should at least use that pattern for new code.