initdb.c::main() too large
In looking to add an fsync-only option to initdb, I found its main()
function to be 743 lines long, and very hard to understand.
The attached patch moves much of that code into separate functions,
which will make initdb.c easier to understand, and easier to add an
fsync-only option. The original initdb.c author, Andrew Dunstan, has
accepted the restructuring, in principle.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
Attachments:
initdb.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
new file mode 100644
index 0aa1e80..68080f2
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** static char infoversion[100];
*** 144,149 ****
--- 144,150 ----
static bool caught_signal = false;
static bool output_failed = false;
static int output_errno = 0;
+ static char *pgdata_native;
/* defaults */
static int n_connections = 10;
*************** static char *authwarning = NULL;
*** 172,177 ****
--- 173,227 ----
static const char *boot_options = "-F";
static const char *backend_options = "--single -F -O -c search_path=pg_catalog -c exit_on_error=true";
+ struct option long_options[] = {
+ {"pgdata", required_argument, NULL, 'D'},
+ {"encoding", required_argument, NULL, 'E'},
+ {"locale", required_argument, NULL, 1},
+ {"lc-collate", required_argument, NULL, 2},
+ {"lc-ctype", required_argument, NULL, 3},
+ {"lc-monetary", required_argument, NULL, 4},
+ {"lc-numeric", required_argument, NULL, 5},
+ {"lc-time", required_argument, NULL, 6},
+ {"lc-messages", required_argument, NULL, 7},
+ {"no-locale", no_argument, NULL, 8},
+ {"text-search-config", required_argument, NULL, 'T'},
+ {"auth", required_argument, NULL, 'A'},
+ {"auth-local", required_argument, NULL, 10},
+ {"auth-host", required_argument, NULL, 11},
+ {"pwprompt", no_argument, NULL, 'W'},
+ {"pwfile", required_argument, NULL, 9},
+ {"username", required_argument, NULL, 'U'},
+ {"help", no_argument, NULL, '?'},
+ {"version", no_argument, NULL, 'V'},
+ {"debug", no_argument, NULL, 'd'},
+ {"show", no_argument, NULL, 's'},
+ {"noclean", no_argument, NULL, 'n'},
+ {"nosync", no_argument, NULL, 'N'},
+ {"xlogdir", required_argument, NULL, 'X'},
+ {NULL, 0, NULL, 0}
+ };
+
+ #ifdef WIN32
+ char *restrict_env;
+ #endif
+ const char *subdirs[] = {
+ "global",
+ "pg_xlog",
+ "pg_xlog/archive_status",
+ "pg_clog",
+ "pg_notify",
+ "pg_serial",
+ "pg_snapshots",
+ "pg_subtrans",
+ "pg_twophase",
+ "pg_multixact/members",
+ "pg_multixact/offsets",
+ "base",
+ "base/1",
+ "pg_tblspc",
+ "pg_stat_tmp"
+ };
+
/* path to 'initdb' binary directory */
static char bin_path[MAXPGPATH];
*************** static bool check_locale_name(int catego
*** 227,232 ****
--- 277,290 ----
static bool check_locale_encoding(const char *locale, int encoding);
static void setlocales(void);
static void usage(const char *progname);
+ void get_restricted_token(void);
+ void setup_pgdata(void);
+ void setup_bin_paths(const char *argv0);
+ void setup_data_file_paths(void);
+ void setup_locale_encoding(void);
+ void setup_signals_umask(void);
+ void process(const char *argv0);
+
#ifdef WIN32
static int CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo);
*************** check_need_password(const char *authmeth
*** 2823,3064 ****
}
}
! int
! main(int argc, char *argv[])
{
- /*
- * options with no short version return a low integer, the rest return
- * their short version value
- */
- static struct option long_options[] = {
- {"pgdata", required_argument, NULL, 'D'},
- {"encoding", required_argument, NULL, 'E'},
- {"locale", required_argument, NULL, 1},
- {"lc-collate", required_argument, NULL, 2},
- {"lc-ctype", required_argument, NULL, 3},
- {"lc-monetary", required_argument, NULL, 4},
- {"lc-numeric", required_argument, NULL, 5},
- {"lc-time", required_argument, NULL, 6},
- {"lc-messages", required_argument, NULL, 7},
- {"no-locale", no_argument, NULL, 8},
- {"text-search-config", required_argument, NULL, 'T'},
- {"auth", required_argument, NULL, 'A'},
- {"auth-local", required_argument, NULL, 10},
- {"auth-host", required_argument, NULL, 11},
- {"pwprompt", no_argument, NULL, 'W'},
- {"pwfile", required_argument, NULL, 9},
- {"username", required_argument, NULL, 'U'},
- {"help", no_argument, NULL, '?'},
- {"version", no_argument, NULL, 'V'},
- {"debug", no_argument, NULL, 'd'},
- {"show", no_argument, NULL, 's'},
- {"noclean", no_argument, NULL, 'n'},
- {"nosync", no_argument, NULL, 'N'},
- {"xlogdir", required_argument, NULL, 'X'},
- {NULL, 0, NULL, 0}
- };
-
- int c,
- i,
- ret;
- int option_index;
- char *effective_user;
- char *pgdenv; /* PGDATA value gotten from and sent to
- * environment */
- char bin_dir[MAXPGPATH];
- char *pg_data_native;
- int user_enc;
-
- #ifdef WIN32
- char *restrict_env;
- #endif
- static const char *subdirs[] = {
- "global",
- "pg_xlog",
- "pg_xlog/archive_status",
- "pg_clog",
- "pg_notify",
- "pg_serial",
- "pg_snapshots",
- "pg_subtrans",
- "pg_twophase",
- "pg_multixact/members",
- "pg_multixact/offsets",
- "base",
- "base/1",
- "pg_tblspc",
- "pg_stat_tmp"
- };
-
- progname = get_progname(argv[0]);
- set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("initdb"));
-
- if (argc > 1)
- {
- if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
- {
- usage(progname);
- exit(0);
- }
- if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
- {
- puts("initdb (PostgreSQL) " PG_VERSION);
- exit(0);
- }
- }
-
- /* process command-line options */
-
- while ((c = getopt_long(argc, argv, "dD:E:L:nNU:WA:sT:X:", long_options, &option_index)) != -1)
- {
- switch (c)
- {
- case 'A':
- authmethodlocal = authmethodhost = pg_strdup(optarg);
-
- /*
- * When ident is specified, use peer for local connections.
- * Mirrored, when peer is specified, use ident for TCP/IP
- * connections.
- */
- if (strcmp(authmethodhost, "ident") == 0)
- authmethodlocal = "peer";
- else if (strcmp(authmethodlocal, "peer") == 0)
- authmethodhost = "ident";
- break;
- case 10:
- authmethodlocal = pg_strdup(optarg);
- break;
- case 11:
- authmethodhost = pg_strdup(optarg);
- break;
- case 'D':
- pg_data = pg_strdup(optarg);
- break;
- case 'E':
- encoding = pg_strdup(optarg);
- break;
- case 'W':
- pwprompt = true;
- break;
- case 'U':
- username = pg_strdup(optarg);
- break;
- case 'd':
- debug = true;
- printf(_("Running in debug mode.\n"));
- break;
- case 'n':
- noclean = true;
- printf(_("Running in noclean mode. Mistakes will not be cleaned up.\n"));
- break;
- case 'N':
- do_sync = false;
- break;
- case 'L':
- share_path = pg_strdup(optarg);
- break;
- case 1:
- locale = pg_strdup(optarg);
- break;
- case 2:
- lc_collate = pg_strdup(optarg);
- break;
- case 3:
- lc_ctype = pg_strdup(optarg);
- break;
- case 4:
- lc_monetary = pg_strdup(optarg);
- break;
- case 5:
- lc_numeric = pg_strdup(optarg);
- break;
- case 6:
- lc_time = pg_strdup(optarg);
- break;
- case 7:
- lc_messages = pg_strdup(optarg);
- break;
- case 8:
- locale = "C";
- break;
- case 9:
- pwfilename = pg_strdup(optarg);
- break;
- case 's':
- show_setting = true;
- break;
- case 'T':
- default_text_search_config = pg_strdup(optarg);
- break;
- case 'X':
- xlog_dir = pg_strdup(optarg);
- break;
- default:
- /* getopt_long already emitted a complaint */
- fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
- progname);
- exit(1);
- }
- }
-
-
- /*
- * Non-option argument specifies data directory as long as it wasn't
- * already specified with -D / --pgdata
- */
- if (optind < argc && strlen(pg_data) == 0)
- {
- pg_data = pg_strdup(argv[optind]);
- optind++;
- }
-
- if (optind < argc)
- {
- fprintf(stderr, _("%s: too many command-line arguments (first is \"%s\")\n"),
- progname, argv[optind]);
- fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
- progname);
- exit(1);
- }
-
- if (pwprompt && pwfilename)
- {
- fprintf(stderr, _("%s: password prompt and password file cannot be specified together\n"), progname);
- exit(1);
- }
-
- check_authmethod_unspecified(&authmethodlocal);
- check_authmethod_unspecified(&authmethodhost);
-
- check_authmethod_valid(authmethodlocal, auth_methods_local, "local");
- check_authmethod_valid(authmethodhost, auth_methods_host, "host");
-
- check_need_password(authmethodlocal, authmethodhost);
-
- if (strlen(pg_data) == 0)
- {
- pgdenv = getenv("PGDATA");
- if (pgdenv && strlen(pgdenv))
- {
- /* PGDATA found */
- pg_data = pg_strdup(pgdenv);
- }
- else
- {
- fprintf(stderr,
- _("%s: no data directory specified\n"
- "You must identify the directory where the data for this database system\n"
- "will reside. Do this with either the invocation option -D or the\n"
- "environment variable PGDATA.\n"),
- progname);
- exit(1);
- }
- }
-
- pg_data_native = pg_data;
- canonicalize_path(pg_data);
-
#ifdef WIN32
/*
* Before we execute another program, make sure that we are running with a
--- 2881,2889 ----
}
}
! void
! get_restricted_token(void)
{
#ifdef WIN32
/*
* Before we execute another program, make sure that we are running with a
*************** main(int argc, char *argv[])
*** 3101,3106 ****
--- 2926,2960 ----
}
}
#endif
+ }
+
+ void
+ setup_pgdata(void)
+ {
+ char *pgdata_get_env, *pgdata_set_env;
+
+ if (strlen(pg_data) == 0)
+ {
+ pgdata_get_env = getenv("PGDATA");
+ if (pgdata_get_env && strlen(pgdata_get_env))
+ {
+ /* PGDATA found */
+ pg_data = pg_strdup(pgdata_get_env);
+ }
+ else
+ {
+ fprintf(stderr,
+ _("%s: no data directory specified\n"
+ "You must identify the directory where the data for this database system\n"
+ "will reside. Do this with either the invocation option -D or the\n"
+ "environment variable PGDATA.\n"),
+ progname);
+ exit(1);
+ }
+ }
+
+ pgdata_native = pg_strdup(pg_data);
+ canonicalize_path(pg_data);
/*
* we have to set PGDATA for postgres rather than pass it on the command
*************** main(int argc, char *argv[])
*** 3108,3123 ****
* need quotes otherwise on Windows because paths there are most likely to
* have embedded spaces.
*/
! pgdenv = pg_malloc(8 + strlen(pg_data));
! sprintf(pgdenv, "PGDATA=%s", pg_data);
! putenv(pgdenv);
! if ((ret = find_other_exec(argv[0], "postgres", PG_BACKEND_VERSIONSTR,
backend_exec)) < 0)
{
char full_path[MAXPGPATH];
! if (find_my_exec(argv[0], full_path) < 0)
strlcpy(full_path, progname, sizeof(full_path));
if (ret == -1)
--- 2962,2984 ----
* need quotes otherwise on Windows because paths there are most likely to
* have embedded spaces.
*/
! pgdata_set_env = pg_malloc(8 + strlen(pg_data));
! sprintf(pgdata_set_env, "PGDATA=%s", pg_data);
! putenv(pgdata_set_env);
! }
!
! void
! setup_bin_paths(const char *argv0)
! {
! int ret;
!
! if ((ret = find_other_exec(argv0, "postgres", PG_BACKEND_VERSIONSTR,
backend_exec)) < 0)
{
char full_path[MAXPGPATH];
! if (find_my_exec(argv0, full_path) < 0)
strlcpy(full_path, progname, sizeof(full_path));
if (ret == -1)
*************** main(int argc, char *argv[])
*** 3153,3215 ****
}
canonicalize_path(share_path);
! effective_user = get_id();
! if (strlen(username) == 0)
! username = effective_user;
!
! set_input(&bki_file, "postgres.bki");
! set_input(&desc_file, "postgres.description");
! set_input(&shdesc_file, "postgres.shdescription");
! set_input(&hba_file, "pg_hba.conf.sample");
! set_input(&ident_file, "pg_ident.conf.sample");
! set_input(&conf_file, "postgresql.conf.sample");
! set_input(&conversion_file, "conversion_create.sql");
! set_input(&dictionary_file, "snowball_create.sql");
! set_input(&info_schema_file, "information_schema.sql");
! set_input(&features_file, "sql_features.txt");
! set_input(&system_views_file, "system_views.sql");
!
! set_info_version();
!
! if (show_setting || debug)
! {
! fprintf(stderr,
! "VERSION=%s\n"
! "PGDATA=%s\nshare_path=%s\nPGPATH=%s\n"
! "POSTGRES_SUPERUSERNAME=%s\nPOSTGRES_BKI=%s\n"
! "POSTGRES_DESCR=%s\nPOSTGRES_SHDESCR=%s\n"
! "POSTGRESQL_CONF_SAMPLE=%s\n"
! "PG_HBA_SAMPLE=%s\nPG_IDENT_SAMPLE=%s\n",
! PG_VERSION,
! pg_data, share_path, bin_path,
! username, bki_file,
! desc_file, shdesc_file,
! conf_file,
! hba_file, ident_file);
! if (show_setting)
! exit(0);
! }
!
! check_input(bki_file);
! check_input(desc_file);
! check_input(shdesc_file);
! check_input(hba_file);
! check_input(ident_file);
! check_input(conf_file);
! check_input(conversion_file);
! check_input(dictionary_file);
! check_input(info_schema_file);
! check_input(features_file);
! check_input(system_views_file);
setlocales();
- printf(_("The files belonging to this database system will be owned "
- "by user \"%s\".\n"
- "This user must also own the server process.\n\n"),
- effective_user);
-
if (strcmp(lc_ctype, lc_collate) == 0 &&
strcmp(lc_ctype, lc_time) == 0 &&
strcmp(lc_ctype, lc_numeric) == 0 &&
--- 3014,3028 ----
}
canonicalize_path(share_path);
+ }
! void
! setup_locale_encoding(void)
! {
! int user_enc;
setlocales();
if (strcmp(lc_ctype, lc_collate) == 0 &&
strcmp(lc_ctype, lc_time) == 0 &&
strcmp(lc_ctype, lc_numeric) == 0 &&
*************** main(int argc, char *argv[])
*** 3289,3294 ****
--- 3102,3161 ----
!check_locale_encoding(lc_collate, user_enc))
exit(1); /* check_locale_encoding printed the error */
+ }
+
+
+ void
+ setup_data_file_paths(void)
+ {
+ set_input(&bki_file, "postgres.bki");
+ set_input(&desc_file, "postgres.description");
+ set_input(&shdesc_file, "postgres.shdescription");
+ set_input(&hba_file, "pg_hba.conf.sample");
+ set_input(&ident_file, "pg_ident.conf.sample");
+ set_input(&conf_file, "postgresql.conf.sample");
+ set_input(&conversion_file, "conversion_create.sql");
+ set_input(&dictionary_file, "snowball_create.sql");
+ set_input(&info_schema_file, "information_schema.sql");
+ set_input(&features_file, "sql_features.txt");
+ set_input(&system_views_file, "system_views.sql");
+
+ if (show_setting || debug)
+ {
+ fprintf(stderr,
+ "VERSION=%s\n"
+ "PGDATA=%s\nshare_path=%s\nPGPATH=%s\n"
+ "POSTGRES_SUPERUSERNAME=%s\nPOSTGRES_BKI=%s\n"
+ "POSTGRES_DESCR=%s\nPOSTGRES_SHDESCR=%s\n"
+ "POSTGRESQL_CONF_SAMPLE=%s\n"
+ "PG_HBA_SAMPLE=%s\nPG_IDENT_SAMPLE=%s\n",
+ PG_VERSION,
+ pg_data, share_path, bin_path,
+ username, bki_file,
+ desc_file, shdesc_file,
+ conf_file,
+ hba_file, ident_file);
+ if (show_setting)
+ exit(0);
+ }
+
+ check_input(bki_file);
+ check_input(desc_file);
+ check_input(shdesc_file);
+ check_input(hba_file);
+ check_input(ident_file);
+ check_input(conf_file);
+ check_input(conversion_file);
+ check_input(dictionary_file);
+ check_input(info_schema_file);
+ check_input(features_file);
+ check_input(system_views_file);
+ }
+
+
+ void
+ setup_text_search(void)
+ {
if (strlen(default_text_search_config) == 0)
{
default_text_search_config = find_matching_ts_config(lc_ctype);
*************** main(int argc, char *argv[])
*** 3318,3331 ****
printf(_("The default text search configuration will be set to \"%s\".\n"),
default_text_search_config);
! printf("\n");
!
! umask(S_IRWXG | S_IRWXO);
- /*
- * now we are starting to do real work, trap signals so we can clean up
- */
/* some of these are not valid on Windows */
#ifdef SIGHUP
pqsignal(SIGHUP, trapsig);
--- 3185,3196 ----
printf(_("The default text search configuration will be set to \"%s\".\n"),
default_text_search_config);
! }
+ void
+ setup_signals_umask(void)
+ {
/* some of these are not valid on Windows */
#ifdef SIGHUP
pqsignal(SIGHUP, trapsig);
*************** main(int argc, char *argv[])
*** 3345,3350 ****
--- 3210,3227 ----
pqsignal(SIGPIPE, SIG_IGN);
#endif
+ umask(S_IRWXG | S_IRWXO);
+ }
+
+
+ void
+ process(const char *argv0)
+ {
+ int i;
+ char bin_dir[MAXPGPATH];
+
+ setup_signals_umask();
+
switch (pg_check_dir(pg_data))
{
case 0:
*************** main(int argc, char *argv[])
*** 3554,3560 ****
fprintf(stderr, "%s", authwarning);
/* Get directory specification used to start this executable */
! strlcpy(bin_dir, argv[0], sizeof(bin_dir));
get_parent_directory(bin_dir);
printf(_("\nSuccess. You can now start the database server using:\n\n"
--- 3431,3437 ----
fprintf(stderr, "%s", authwarning);
/* Get directory specification used to start this executable */
! strlcpy(bin_dir, argv0, sizeof(bin_dir));
get_parent_directory(bin_dir);
printf(_("\nSuccess. You can now start the database server using:\n\n"
*************** main(int argc, char *argv[])
*** 3562,3570 ****
"or\n"
" %s%s%spg_ctl%s -D %s%s%s -l logfile start\n\n"),
QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
! QUOTE_PATH, pg_data_native, QUOTE_PATH,
QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
! QUOTE_PATH, pg_data_native, QUOTE_PATH);
return 0;
}
--- 3439,3633 ----
"or\n"
" %s%s%spg_ctl%s -D %s%s%s -l logfile start\n\n"),
QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
! QUOTE_PATH, pgdata_native, QUOTE_PATH,
QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
! QUOTE_PATH, pgdata_native, QUOTE_PATH);
! }
!
!
! int
! main(int argc, char *argv[])
! {
! /*
! * options with no short version return a low integer, the rest return
! * their short version value
! */
! int c;
! int option_index;
! char *effective_user;
!
! progname = get_progname(argv[0]);
! set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("initdb"));
+ if (argc > 1)
+ {
+ if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
+ {
+ usage(progname);
+ exit(0);
+ }
+ if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
+ {
+ puts("initdb (PostgreSQL) " PG_VERSION);
+ exit(0);
+ }
+ }
+
+ /* process command-line options */
+
+ while ((c = getopt_long(argc, argv, "dD:E:L:nNU:WA:sT:X:", long_options, &option_index)) != -1)
+ {
+ switch (c)
+ {
+ case 'A':
+ authmethodlocal = authmethodhost = pg_strdup(optarg);
+
+ /*
+ * When ident is specified, use peer for local connections.
+ * Mirrored, when peer is specified, use ident for TCP/IP
+ * connections.
+ */
+ if (strcmp(authmethodhost, "ident") == 0)
+ authmethodlocal = "peer";
+ else if (strcmp(authmethodlocal, "peer") == 0)
+ authmethodhost = "ident";
+ break;
+ case 10:
+ authmethodlocal = pg_strdup(optarg);
+ break;
+ case 11:
+ authmethodhost = pg_strdup(optarg);
+ break;
+ case 'D':
+ pg_data = pg_strdup(optarg);
+ break;
+ case 'E':
+ encoding = pg_strdup(optarg);
+ break;
+ case 'W':
+ pwprompt = true;
+ break;
+ case 'U':
+ username = pg_strdup(optarg);
+ break;
+ case 'd':
+ debug = true;
+ printf(_("Running in debug mode.\n"));
+ break;
+ case 'n':
+ noclean = true;
+ printf(_("Running in noclean mode. Mistakes will not be cleaned up.\n"));
+ break;
+ case 'N':
+ do_sync = false;
+ break;
+ case 'L':
+ share_path = pg_strdup(optarg);
+ break;
+ case 1:
+ locale = pg_strdup(optarg);
+ break;
+ case 2:
+ lc_collate = pg_strdup(optarg);
+ break;
+ case 3:
+ lc_ctype = pg_strdup(optarg);
+ break;
+ case 4:
+ lc_monetary = pg_strdup(optarg);
+ break;
+ case 5:
+ lc_numeric = pg_strdup(optarg);
+ break;
+ case 6:
+ lc_time = pg_strdup(optarg);
+ break;
+ case 7:
+ lc_messages = pg_strdup(optarg);
+ break;
+ case 8:
+ locale = "C";
+ break;
+ case 9:
+ pwfilename = pg_strdup(optarg);
+ break;
+ case 's':
+ show_setting = true;
+ break;
+ case 'T':
+ default_text_search_config = pg_strdup(optarg);
+ break;
+ case 'X':
+ xlog_dir = pg_strdup(optarg);
+ break;
+ default:
+ /* getopt_long already emitted a complaint */
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+ }
+
+
+ /*
+ * Non-option argument specifies data directory as long as it wasn't
+ * already specified with -D / --pgdata
+ */
+ if (optind < argc && strlen(pg_data) == 0)
+ {
+ pg_data = pg_strdup(argv[optind]);
+ optind++;
+ }
+
+ if (optind < argc)
+ {
+ fprintf(stderr, _("%s: too many command-line arguments (first is \"%s\")\n"),
+ progname, argv[optind]);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit(1);
+ }
+
+ if (pwprompt && pwfilename)
+ {
+ fprintf(stderr, _("%s: password prompt and password file cannot be specified together\n"), progname);
+ exit(1);
+ }
+
+ check_authmethod_unspecified(&authmethodlocal);
+ check_authmethod_unspecified(&authmethodhost);
+
+ check_authmethod_valid(authmethodlocal, auth_methods_local, "local");
+ check_authmethod_valid(authmethodhost, auth_methods_host, "host");
+
+ check_need_password(authmethodlocal, authmethodhost);
+
+ get_restricted_token();
+
+ setup_pgdata();
+
+ setup_bin_paths(argv[0]);
+
+ effective_user = get_id();
+ if (strlen(username) == 0)
+ username = effective_user;
+
+ printf(_("The files belonging to this database system will be owned "
+ "by user \"%s\".\n"
+ "This user must also own the server process.\n\n"),
+ effective_user);
+
+ set_info_version();
+
+ setup_data_file_paths();
+
+ setup_locale_encoding();
+
+ setup_text_search();
+
+ printf("\n");
+
+ process(argv[0]);
+
return 0;
}
Bruce Momjian <bruce@momjian.us> writes:
In looking to add an fsync-only option to initdb, I found its main()
function to be 743 lines long, and very hard to understand.
The attached patch moves much of that code into separate functions,
which will make initdb.c easier to understand, and easier to add an
fsync-only option. The original initdb.c author, Andrew Dunstan, has
accepted the restructuring, in principle.
No objection to breaking it into multiple functions --- but I do say
it's a lousy idea to put the long_options[] constant at the front of
the file, thousands of lines away from the switch construct that it
has to be in sync with. We don't do that in any other program AFAIR.
Keep that in the main() function, please.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 29, 2012 at 11:23:59PM -0500, Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
In looking to add an fsync-only option to initdb, I found its main()
function to be 743 lines long, and very hard to understand.The attached patch moves much of that code into separate functions,
which will make initdb.c easier to understand, and easier to add an
fsync-only option. The original initdb.c author, Andrew Dunstan, has
accepted the restructuring, in principle.No objection to breaking it into multiple functions --- but I do say
it's a lousy idea to put the long_options[] constant at the front of
the file, thousands of lines away from the switch construct that it
has to be in sync with. We don't do that in any other program AFAIR.
Keep that in the main() function, please.
Good point. I had not noticed that. I fixed my initdb patch, and
adjusted a few other C files to be consistent.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 29, 2012 at 11:12:23PM -0500, Bruce Momjian wrote:
In looking to add an fsync-only option to initdb, I found its main()
function to be 743 lines long, and very hard to understand.The attached patch moves much of that code into separate functions,
which will make initdb.c easier to understand, and easier to add an
fsync-only option. The original initdb.c author, Andrew Dunstan, has
accepted the restructuring, in principle.
Applied.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 11/30/2012 04:45 PM, Bruce Momjian wrote:
On Thu, Nov 29, 2012 at 11:12:23PM -0500, Bruce Momjian wrote:
In looking to add an fsync-only option to initdb, I found its main()
function to be 743 lines long, and very hard to understand.The attached patch moves much of that code into separate functions,
which will make initdb.c easier to understand, and easier to add an
fsync-only option. The original initdb.c author, Andrew Dunstan, has
accepted the restructuring, in principle.Applied.
Sorry I didn't have time to review this before it was applied.
A few minor nitpicks:
* process() is a fairly uninformative function name, not sure what I'd
call it, but something more descriptive.
* the setup_signals_and_umask() call and possibly the final message
section of process() would be better placed back in main() IMNSHO.
* the large statements for setting up the datadir and the xlogdir
should be factored out of process() into their own functions, I
think. That would make it much more readable.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Nov 30, 2012 at 06:06:39PM -0500, Andrew Dunstan wrote:
On 11/30/2012 04:45 PM, Bruce Momjian wrote:
On Thu, Nov 29, 2012 at 11:12:23PM -0500, Bruce Momjian wrote:
In looking to add an fsync-only option to initdb, I found its main()
function to be 743 lines long, and very hard to understand.The attached patch moves much of that code into separate functions,
which will make initdb.c easier to understand, and easier to add an
fsync-only option. The original initdb.c author, Andrew Dunstan, has
accepted the restructuring, in principle.Applied.
Sorry I didn't have time to review this before it was applied.
A few minor nitpicks:
* process() is a fairly uninformative function name, not sure what I'd
call it, but something more descriptive.
* the setup_signals_and_umask() call and possibly the final message
section of process() would be better placed back in main() IMNSHO.
* the large statements for setting up the datadir and the xlogdir
should be factored out of process() into their own functions, I
think. That would make it much more readable.
OK, I will make those changes. Thanks.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Nov 30, 2012 at 06:06:39PM -0500, Andrew Dunstan wrote:
On 11/30/2012 04:45 PM, Bruce Momjian wrote:
On Thu, Nov 29, 2012 at 11:12:23PM -0500, Bruce Momjian wrote:
In looking to add an fsync-only option to initdb, I found its main()
function to be 743 lines long, and very hard to understand.The attached patch moves much of that code into separate functions,
which will make initdb.c easier to understand, and easier to add an
fsync-only option. The original initdb.c author, Andrew Dunstan, has
accepted the restructuring, in principle.Applied.
Sorry I didn't have time to review this before it was applied.
A few minor nitpicks:
* process() is a fairly uninformative function name, not sure what I'd
call it, but something more descriptive.
* the setup_signals_and_umask() call and possibly the final message
section of process() would be better placed back in main() IMNSHO.
* the large statements for setting up the datadir and the xlogdir
should be factored out of process() into their own functions, I
think. That would make it much more readable.
Done with the attached patch. I kept the signals in their own function,
but moved the umask() call out --- I was not happy mixing those either.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
Attachments:
initdb.difftext/x-diff; charset=us-asciiDownload
commit 26374f2a0fc02b76a91b7565e908dbae99a3b5f9
Author: Bruce Momjian <bruce@momjian.us>
Date: Mon Dec 3 23:22:56 2012 -0500
In initdb.c, rename some newly created functions, and move the directory
creation and xlog symlink creation to separate functions.
Per suggestions from Andrew Dunstan.
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
new file mode 100644
index 8c0a9f4..d44281b
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** void setup_pgdata(void);
*** 255,263 ****
void setup_bin_paths(const char *argv0);
void setup_data_file_paths(void);
void setup_locale_encoding(void);
! void setup_signals_and_umask(void);
void setup_text_search(void);
! void process(const char *argv0);
#ifdef WIN32
--- 255,265 ----
void setup_bin_paths(const char *argv0);
void setup_data_file_paths(void);
void setup_locale_encoding(void);
! void setup_signals(void);
void setup_text_search(void);
! void create_data_directory(void);
! void create_xlog_symlink(void);
! void initialize_data_directory(void);
#ifdef WIN32
*************** setup_text_search(void)
*** 3164,3170 ****
void
! setup_signals_and_umask(void)
{
/* some of these are not valid on Windows */
#ifdef SIGHUP
--- 3166,3172 ----
void
! setup_signals(void)
{
/* some of these are not valid on Windows */
#ifdef SIGHUP
*************** setup_signals_and_umask(void)
*** 3184,3202 ****
#ifdef SIGPIPE
pqsignal(SIGPIPE, SIG_IGN);
#endif
-
- umask(S_IRWXG | S_IRWXO);
}
void
! process(const char *argv0)
{
- int i;
- char bin_dir[MAXPGPATH];
-
- setup_signals_and_umask();
-
switch (pg_check_dir(pg_data))
{
case 0:
--- 3186,3197 ----
#ifdef SIGPIPE
pqsignal(SIGPIPE, SIG_IGN);
#endif
}
void
! create_data_directory(void)
{
switch (pg_check_dir(pg_data))
{
case 0:
*************** process(const char *argv0)
*** 3249,3255 ****
--- 3244,3255 ----
progname, pg_data, strerror(errno));
exit_nicely();
}
+ }
+
+ void
+ create_xlog_symlink(void)
+ {
/* Create transaction log symlink, if required */
if (strcmp(xlog_dir, "") != 0)
{
*************** process(const char *argv0)
*** 3336,3341 ****
--- 3336,3356 ----
exit_nicely();
#endif
}
+ }
+
+
+ void
+ initialize_data_directory(void)
+ {
+ int i;
+
+ setup_signals();
+
+ umask(S_IRWXG | S_IRWXO);
+
+ create_data_directory();
+
+ create_xlog_symlink();
/* Create required subdirectories */
printf(_("creating subdirectories ... "));
*************** process(const char *argv0)
*** 3404,3422 ****
if (authwarning != NULL)
fprintf(stderr, "%s", authwarning);
-
- /* Get directory specification used to start this executable */
- strlcpy(bin_dir, argv0, sizeof(bin_dir));
- get_parent_directory(bin_dir);
-
- printf(_("\nSuccess. You can now start the database server using:\n\n"
- " %s%s%spostgres%s -D %s%s%s\n"
- "or\n"
- " %s%s%spg_ctl%s -D %s%s%s -l logfile start\n\n"),
- QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
- QUOTE_PATH, pgdata_native, QUOTE_PATH,
- QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
- QUOTE_PATH, pgdata_native, QUOTE_PATH);
}
--- 3419,3424 ----
*************** main(int argc, char *argv[])
*** 3459,3464 ****
--- 3461,3467 ----
int c;
int option_index;
char *effective_user;
+ char bin_dir[MAXPGPATH];
progname = get_progname(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("initdb"));
*************** main(int argc, char *argv[])
*** 3642,3648 ****
printf("\n");
! process(argv[0]);
return 0;
}
--- 3645,3664 ----
printf("\n");
! initialize_data_directory();
+ /* Get directory specification used to start this executable */
+ strlcpy(bin_dir, argv[0], sizeof(bin_dir));
+ get_parent_directory(bin_dir);
+
+ printf(_("\nSuccess. You can now start the database server using:\n\n"
+ " %s%s%spostgres%s -D %s%s%s\n"
+ "or\n"
+ " %s%s%spg_ctl%s -D %s%s%s -l logfile start\n\n"),
+ QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
+ QUOTE_PATH, pgdata_native, QUOTE_PATH,
+ QUOTE_PATH, bin_dir, (strlen(bin_dir) > 0) ? DIR_SEP : "", QUOTE_PATH,
+ QUOTE_PATH, pgdata_native, QUOTE_PATH);
+
return 0;
}