Re: [PATCH] Implement motd for PostgreSQL

On Sun, Apr 4, 2021, at 20:42, Dagfinn Ilmari Mannsåker wrote:
> Fabien COELHO <coelho@cri.ensmp.fr <mailto:coelho%40cri.ensmp.fr>> writes:
> 
> >>> 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,
and found https://www.postgresql.org/docs/current/source-conventions.html

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