Hi, Very often we allocate a structure with palloc0() and then initialize its fields a second time with 0 / NULL / false / etc. For instance, in toasting.c we have:``` indexInfo = makeNode(IndexInfo); // uses palloc0() indexInfo->ii_NumIndexAttrs = 2; indexInfo->ii_NumIndexKeyAttrs = 2; indexInfo->ii_IndexAttrNumbers[0] = 1; indexInfo->ii_IndexAttrNumbers[1] = 2; indexInfo->ii_Expressions = NIL; indexInfo->ii_ExpressionsState = NIL; indexInfo->ii_Predicate = NIL; indexInfo->ii_PredicateState = NULL; indexInfo->ii_ExclusionOps = NULL; indexInfo->ii_ExclusionProcs = NULL; indexInfo->ii_ExclusionStrats = NULL; indexInfo->ii_Unique = true; indexInfo->ii_NullsNotDistinct = false; indexInfo->ii_ReadyForInserts = true; indexInfo->ii_CheckedUnchanged = false; indexInfo->ii_IndexUnchanged = false; indexInfo->ii_Concurrent = false; indexInfo->ii_BrokenHotChain = false; indexInfo->ii_ParallelWorkers = 0; indexInfo->ii_Am = BTREE_AM_OID; indexInfo->ii_AmCache = NULL; indexInfo->ii_Context = CurrentMemoryContext; ``` There are more complicated cases. For instance in execMain.c (and similarly elsewhere) we have: ``` rInfo = makeNode(ResultRelInfo); InitResultRelInfo(rInfo, rel, 0, /* dummy rangetable index */ rootRelInfo, estate->es_instrument); ``` ... where InitResultRelInfo does: ``` MemSet(resultRelInfo, 0, sizeof(ResultRelInfo)); // !!! resultRelInfo->type = T_ResultRelInfo; resultRelInfo->ri_RangeTableIndex = resultRelationIndex; resultRelInfo->ri_RelationDesc = resultRelationDesc; resultRelInfo->ri_NumIndices = 0; resultRelInfo->ri_IndexRelationDescs = NULL; resultRelInfo->ri_IndexRelationInfo = NULL; resultRelInfo->ri_needLockTagTuple = IsInplaceUpdateRelation(resultRelationDesc); ``` Here the fields are initialized 3 times in a row which is rather strange IMO. In the same time other places e.g. copy.c may just do: ``` select = makeNode(SelectStmt); select->targetList = targetList; select->fromClause = list_make1(from); ``` IMO zeroing structures once is preferable in terms of both performance and readability. Unless there are strong objections I would like to propose a refactoring that would make the code consistent and use the pattern as in the latest example, perhaps with few exceptions e.g. for enum values that happened to be zero. Please let me know whether `false` and perhaps other values should be an exception too. If there are objections, I would like to know which method of initialization is preferable so that alternatively we could follow this pattern. Perhaps makeNode() shouldn't in fact use palloc0()? Or maybe we should generate functions like makeSelectStmt() and makeIndexInfo() ? Honestly I don't know which approach is best and whether this is even a problem we should invest our time into. Please let me know what you think.
Something to keep in mind is that palloc0() or memset(..., 0, ...) will also zero any padding bytes in the structure which setting the individual fields will not. This may be important if the structure is written to disk for example. I like setting fields that are expected to be zero by the code to zero explicitly for clarity. And also I would prefer to only not set the padding to 0 when absolutely sure the structure will never be directly written anywhere were we don't want to risk leaking data. -- Anders Åstrand Percona
