Re: Use C99 designated initializers for some structs

From: Mark Dilger <hornschnorter(at)gmail(dot)com>
To: David Steele <david(at)pgmasters(dot)net>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Use C99 designated initializers for some structs
Date: 2018-08-30 15:59:05
Message-ID: 2E1373CA-2C4C-4CCA-AD4F-583E588503BD@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Aug 29, 2018, at 1:51 PM, David Steele <david(at)pgmasters(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gunnlaugur Thor Briem 2018-08-30 16:03:11 Re: pg_upgrade fails saying function unaccent(text) doesn't exist
Previous Message Tom Lane 2018-08-30 15:55:27 Re: Startup cost of sequential scan