On Aug 28, 2025, at 09:29, Daniel Gustafsson <[email protected]> wrote:

> A tiny nitpick is that all the other generator scripts (that I looked at) in
> the tree use GetOptions() with named parameter rather than dereference ARGV
> directly:
> 
> +my $input_fname = $ARGV[0];
> +my $output_fname = $ARGV[1];

GetOptions() is overkill when there are no options. I’d opt for:

die "Usage: $0 INPUT_FILE OUTPUT_FILE\n" unless @ARGV == 2;
my ($input_fname, $output_fname) = @ARGV;

And since I griped about Perl style previously, I made a pass over modernizing 
it a bit. One might argue it’s less clear, of course; there is less alignment 
of the printing than in the original. Otherwise, I’d note:

* Use the /r regex return sequence to simplify dquote() (requires Perl 5.14, 
IIRC)
* Iterate over the data types in a single line
* Pass lists of values to `print`
* Use {$fh} syntax to make file handle arguments clearer

But do with it what you will.

One other thing, as an aside, and probably not worth changing this patch: I’d 
prefer to see the use of explicit I/O layers. IOW, rather than

    open my $ofh, '>', $output_fname

Use the UTF-8 layer, which encodes strings as UTF-8 bytes:

    open my $ofh, '>:utf8', $output_fname

Or perhaps use pure binary:

    open my $ofh, '>:raw', $output_fname

Though then things like `ucfirst` won’t work properly for non-ASCII strings.

The default layer, when not specified, is Latin-1 (because 1994). It’s not a 
problem if we’re certain we’ll never use anything other than ASCII, but more 
explicit I/O layers would be clearer, IMO. I didn’t change it in the attached 
because Catalog.pm doesn’t use an I/O layer, either, so it’s best if they’re 
consistent.

So, I guess, would there be interest in a patch to update I/O layer handling in 
the core Perl code?

Best,

David

#!/usr/bin/perl -w
#----------------------------------------------------------------------
#
# Generate guc_tables.c from guc_parameters.dat.
#
# Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
# Portions Copyright (c) 1994, Regents of the University of California
#
# src/backend/utils/misc/gen_guc_tables.pl
#
#----------------------------------------------------------------------

use strict;
use warnings FATAL => 'all';
use v5.14;

use FindBin;
use lib "$FindBin::RealBin/../../catalog";
use Catalog;

die "Usage: $0 INPUT_FILE OUTPUT_FILE\n" unless @ARGV == 2;
my ($input_fname, $output_fname) = @ARGV;

my $parse = Catalog::ParseData($input_fname);

open my $ofh, '>', $output_fname or die "Cannot open $output_fname: $!\n";

print_boilerplate($ofh, $output_fname, 'GUC tables');
print_one_table($ofh, $_) for (qw(bool int real string enum));

close $ofh;

# Adds double quotes and escapes as necessary for C strings.
sub dquote($) { '"' . $_[0] =~ s/"/\\"/gr . '"' }

# Print GUC table for one type.
sub print_one_table
{
	my ($ofh, $type) = @_;
	my $Type = ucfirst $type;

	print {$ofh}
	  "\n\n",
	  "struct config_${type} ConfigureNames${Type}[] =\n",
	  "{\n";

	for my $entry (@{$parse})
	{
		next if $entry->{type} ne $type;
		my $is_num = $entry->{type} eq 'int' || $entry->{type} eq 'real';

		say {$ofh} "#ifdef $entry->{ifdef}" if $entry->{ifdef};
		print {$ofh} "\t{",
		  sprintf(
			"\t\t{%s, %s, %s,\n",
			dquote $entry->{name},
			$entry->{context}, $entry->{group},),
		  sprintf("\t\t\tgettext_noop(%s),\n", dquote $entry->{short_desc}),
		  ( $entry->{long_desc}
			? sprintf("\t\t\tgettext_noop(%s)", dquote $entry->{long_desc})
			: ("\t\t\tNULL")),
		  ($entry->{flags} ? (",\n\t\t\t$entry->{flags}\n") : "\n"),
		  "\t\t},\n",
		  "\t\t&$entry->{variable},\n",
		  "\t\t$entry->{boot_val},",
		  ($is_num ? (" $entry->{min}, $entry->{max},")       : ()),
		  ($entry->{type} eq 'enum' ? (" $entry->{options},") : ()), "\n",
		  sprintf(
			"\t\t%s, %s, %s\n",
			$entry->{check_hook} || 'NULL',
			$entry->{assign_hook} || 'NULL',
			$entry->{show_hook} || 'NULL',),
		  "\t},\n",
		  ($entry->{ifdef} ? ("#endif") : ()),
		  "\n",
		  ;
	}

	print {$ofh}
	  "\t/* End-of-list marker */\n",
	  "\t{{0}}\n",
	  "};\n";

	return;
}

sub print_boilerplate
{
	my ($fh, $fname, $descr) = @_;
	printf {$fh} <<EOM, $fname, $descr;
/*-------------------------------------------------------------------------
 *
 * %s
 *    %s
 *
 * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
 * Portions Copyright (c) 1994, Regents of the University of California
 *
 * NOTES
 *  ******************************
 *  *** DO NOT EDIT THIS FILE! ***
 *  ******************************
 *
 *  It has been GENERATED by src/backend/utils/misc/gen_guc_tables.pl
 *
 *-------------------------------------------------------------------------
 */
EOM

	return;
}

Attachment: signature.asc
Description: Message signed with OpenPGP

Reply via email to