On Sun, Apr 4, 2021, at 20:42, Dagfinn Ilmari Mannsåker wrote:
>>> The actual source looks pretty straightforward. I'm wondering whether pg
>>> style would suggest to write motd != NULL instead of just motd.
>>
>> That's what I had originally, but when reviewing my code verifying code style,
>> I noticed the other case it more common:
>>
>> if \([a-z]* != NULL &&
>> 119 results in 72 files
>>
>> if \([a-z]* &&
>> 936 results in 311 files
>
> If other cases are indeed pointers. For pgbench, all direct "if (xxx &&"
> cases are simple booleans or integers, pointers seem to have "!=
> NULL". While looking quickly at the grep output, ISTM that most obvious
> pointers have "!= NULL" and other cases often look like booleans:
>
> catalog/pg_operator.c: if (isDelete && t->oprcom == baseId)
> catalog/toasting.c: if (check && lockmode != AccessExclusiveLock)
> commands/async.c: if (amRegisteredListener && listenChannels == NIL)
> commands/explain.c: if (es->analyze && es->timing)
> …
>
> I'm sure there are exceptions, but ISTM that the local style is "!= NULL".
Looking specifically at code checking an expression before dereferencing
it, we get:
$ ag '(?:if|Assert)\s*\(\s*(\S+)\s*&&\s*\1->\w+' | wc -l
247
$ ag '(?:if|Assert)\s*\(\s*(\S+)\s*!=\s*NULL\s*&&\s*\1->\w+' | wc -l
74
So the shorter 'foo && foo->bar' form (which I personally prefer) is
considerably more common than the longer 'foo != NULL && foo->bar' form.
Oh, I see. This gets more and more interesting.
More of the most popular variant like a good rule to follow,
except when a new improved pattern is invented and new code
written in a new way, but all old code written in the old way remains,
so less experienced developers following such a rule,
will continue to write code in the old way.
I sometimes do "git log -p" grepping for recent code changes,
to see how new code is written.
It would be nice if there would be a grep similar to "ag" that could
also dig the git repo and show date/time when such code lines
were added.
I was looking for some PostgreSQL coding convention document,
Maybe "foo != NULL && foo->bar" XOR "foo && foo->bar" should be added to such document?
Is it an ambition to normalize the entire code base, to use just one of the two?
If so, maybe we could use some C compiler to get the AST
for all the C files and search it for occurrences, and then after normalizing
compiling again to verify the AST is unchanged (or changed in the desired way)?
/Joel
