Re: Use C99 designated initializers for some structs

Mark Dilger <hornschnorter@gmail.com>

From: Mark Dilger <hornschnorter@gmail.com>
To: David Steele <david@pgmasters.net>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2018-08-30T15:59:05Z
Lists: pgsql-hackers

> On Aug 29, 2018, at 1:51 PM, David Steele <david@pgmasters.net> wrote:
> 
> On 8/29/18 5:14 AM, Peter Eisentraut wrote:
>> On 29/08/2018 12:13, Peter Eisentraut wrote:
>>> Here is a patch to change some struct initializations to use C99-style
>>> designated initializers.  These are just a few particularly egregious
>>> cases that were hard to read and write, and error prone because of many
>>> similar adjacent types.
>>> 
>>> (The PL/Python changes currently don't compile with Python 3 because of
>>> the situation described in the parallel thread "PL/Python: Remove use of
>>> simple slicing API".)
>>> 
>>> Thoughts?
> 
> +1.  This is an incredible win for readability/maintainability.

I tried doing this perhaps a year ago, and there are a few files with arrays
of structs whose representations get much larger when you change the format
in this way.  For instance, in guc.c:

static struct config_bool ConfigureNamesBool[] =
{
    {
        {"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
            gettext_noop("Enables the planner's use of sequential-scan plans."),
            NULL
        },
        &enable_seqscan,
        true,
        NULL, NULL, NULL
    },

becomes:

static struct config_bool ConfigureNamesBool[] =
{
    {
        .gen = {
            .name = "enable_seqscan",
            .context = PGC_USERSET,
            .group = QUERY_TUNING_METHOD,
            .short_desc = gettext_noop("Enables the planner's use of sequential-scan plans."),
            .long_desc = NULL
        },
        .variable = &enable_seqscan,
        .boot_val = true,
        .check_hook = NULL,
        .assign_hook = NULL,
        .show_hook = NULL
    },

and then gets much longer if you do as per Tom's general suggestion and make
each field explicit (though Tom might not apply that rule to this case):

static struct config_bool ConfigureNamesBool[] =
{   
    {
        .gen = {
            .name = "enable_seqscan",
            .context = PGC_USERSET, 
            .group = QUERY_TUNING_METHOD,
            .short_desc = gettext_noop("Enables the planner's use of sequential-scan plans."),
            .long_desc = NULL,
            .flags = 0,
            .vartype = 0,
            .status = 0,        
            .source = 0,
            .reset_source = 0,
            .scontext = 0,
            .reset_scontext = 0,
            .stack = NULL,
            .extra = NULL,
            .sourcefile = NULL,
            .sourceline = 0     
        },
        .variable = &enable_seqscan,
        .boot_val = true,
        .check_hook = NULL,
        .assign_hook = NULL,
        .show_hook = NULL,
        .reset_val = false,
        .reset_extra = NULL
    },

This sort of thing happens an awful lot in guc.c, but it comes up in syscache.c
also, and perhaps other places, though I don't recall any longer which other files
were like this.

What should the general rule be for initializing arrays of structs such as these?

mark



Commits

  1. Use C99 designated initializers for some structs