Re: code cleanups

Justin Pryzby <pryzby(at)telsasoft(dot)com> writes:
> Some modest cleanups I've accumulated

Hi Justin.

0001:
Regarding initializer {0}, the problem is still with old compilers, which
don't initialize exactly like memset.
Only more modern compilers fill in any "holes" that may exist.
This means that as old compilers are not supported, this will no longer be
a problem.
Fast and secure solution: memset

+1 to switch from loop to memset, same as many places in the code base.

- /* initialize nulls and values */
- for (i = 0; i < Natts_pg_constraint; i++)
- {
- nulls[i] = false;
- values[i] = (Datum) NULL;
- }
+ memset(nulls, false, sizeof(nulls));
+ memset(values, 0, sizeof(values));

What made me curious about this excerpt is that the Datum values are NULL,
but aren't nulls?
Would it not be?
+ memset(nulls, true, sizeof(nulls));
+ memset(values, 0, sizeof(values));

src/backend/tcop/pquery.c:
/* single format specified, use for all columns */
-int16 format1 = formats[0];
-
-for (i = 0; i < natts; i++)
-portal->formats[i] = format1;
+ memset(portal->formats, formats[0], natts * sizeof(*portal->formats));

0002:
contrib/sslinfo/sslinfo.c

memset is faster than intercalated stores.

src/backend/replication/logical/origin.c
+1
one store, is better than three.
but, should be:
- memset(nulls, 1, sizeof(nulls));
+memset(nulls, false, sizeof(nulls));

The correct style is false, not 0.

src/backend/utils/adt/misc.c:
-1
It got worse.
It's only one store, which could be avoided by the "continue" statement.

src/backend/utils/misc/pg_controldata.c:
+1
+memset(nulls, false, sizeof(nulls));

or
nulls[0] = false;
nulls[1] = false;
nulls[2] = false;
nulls[3] = false;

Bad style, intercalated stores are worse.

0003:
+1

But you should reduce the scope of vars:
RangeTblEntry *rte
Oid userid;

+ if (varno != relid)
+ {
+ RangeTblEntry *rte;
+ Oid userid;

0005:
+1

0006:
+1

regards,
Ranier Vilela