In initdb, many global string variables are initialized as empty strings ("") and then checked later with strcmp(), instead of just using NULL. I think this is probably left over from the shell script conversion. The style has also spread to pg_basebackup. So here is a patch to clean that up, and a second patch to clean up some other a bit confusing business in initdb that seems like old shell script code.
While looking around, I found some useless empty string initializations in the ecpg test suite as well, and here is another patch for that. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 13071706e5e82e2bebd68ae6b68af471e1141802 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Wed, 30 Aug 2017 22:28:36 -0400 Subject: [PATCH 1/3] Remove useless dead code --- .../ecpg/test/expected/pgtypeslib-dt_test.c | 22 +++++++++++----------- src/interfaces/ecpg/test/pgtypeslib/dt_test.pgc | 22 +++++++++++----------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test.c b/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test.c index 00d43915b2..69a605c7e6 100644 --- a/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test.c +++ b/src/interfaces/ecpg/test/expected/pgtypeslib-dt_test.c @@ -155,7 +155,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} /* rdate_defmt_asc() */ - date1 = 0; text = ""; + date1 = 0; fmt = "yy/mm/dd"; in = "In the year 1995, the month of December, it is the 25th day"; /* 0123456789012345678901234567890123456789012345678901234567890 @@ -166,7 +166,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc1: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmmm. dd. yyyy"; in = "12/25/95"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -174,7 +174,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc2: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "yy/mm/dd"; in = "95/12/25"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -182,7 +182,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc3: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "yy/mm/dd"; in = "1995, December 25th"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -190,7 +190,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc4: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "dd-mm-yy"; in = "This is 25th day of December, 1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -198,7 +198,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc5: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmddyy"; in = "Dec. 25th, 1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -206,7 +206,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc6: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmm. dd. yyyy"; in = "dec 25th 1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -214,7 +214,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc7: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmm. dd. yyyy"; in = "DEC-25-1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -222,7 +222,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc8: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mm yy dd."; in = "12199525"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -230,7 +230,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc9: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "yyyy fierj mm dd."; in = "19951225"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -238,7 +238,7 @@ if (sqlca.sqlcode < 0) sqlprint ( );} printf("date_defmt_asc10: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mm/dd/yy"; in = "122595"; PGTYPESdate_defmt_asc(&date1, fmt, in); diff --git a/src/interfaces/ecpg/test/pgtypeslib/dt_test.pgc b/src/interfaces/ecpg/test/pgtypeslib/dt_test.pgc index 768cbd5e6f..35f0b6dda5 100644 --- a/src/interfaces/ecpg/test/pgtypeslib/dt_test.pgc +++ b/src/interfaces/ecpg/test/pgtypeslib/dt_test.pgc @@ -81,7 +81,7 @@ main(void) /* rdate_defmt_asc() */ - date1 = 0; text = ""; + date1 = 0; fmt = "yy/mm/dd"; in = "In the year 1995, the month of December, it is the 25th day"; /* 0123456789012345678901234567890123456789012345678901234567890 @@ -92,7 +92,7 @@ main(void) printf("date_defmt_asc1: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmmm. dd. yyyy"; in = "12/25/95"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -100,7 +100,7 @@ main(void) printf("date_defmt_asc2: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "yy/mm/dd"; in = "95/12/25"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -108,7 +108,7 @@ main(void) printf("date_defmt_asc3: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "yy/mm/dd"; in = "1995, December 25th"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -116,7 +116,7 @@ main(void) printf("date_defmt_asc4: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "dd-mm-yy"; in = "This is 25th day of December, 1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -124,7 +124,7 @@ main(void) printf("date_defmt_asc5: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmddyy"; in = "Dec. 25th, 1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -132,7 +132,7 @@ main(void) printf("date_defmt_asc6: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmm. dd. yyyy"; in = "dec 25th 1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -140,7 +140,7 @@ main(void) printf("date_defmt_asc7: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mmm. dd. yyyy"; in = "DEC-25-1995"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -148,7 +148,7 @@ main(void) printf("date_defmt_asc8: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mm yy dd."; in = "12199525"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -156,7 +156,7 @@ main(void) printf("date_defmt_asc9: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "yyyy fierj mm dd."; in = "19951225"; PGTYPESdate_defmt_asc(&date1, fmt, in); @@ -164,7 +164,7 @@ main(void) printf("date_defmt_asc10: %s\n", text); free(text); - date1 = 0; text = ""; + date1 = 0; fmt = "mm/dd/yy"; in = "122595"; PGTYPESdate_defmt_asc(&date1, fmt, in); base-commit: 04e9678614ec64ad9043174ac99a25b1dc45233a -- 2.14.1
From c7aab662f1217c555170b4a6c416e7bb4bf734e7 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Wed, 30 Aug 2017 22:28:36 -0400 Subject: [PATCH 2/3] Remove useless empty string initializations This coding style probably stems from the days of shell scripts. --- src/bin/initdb/initdb.c | 70 ++++++++++++++++++----------------- src/bin/pg_basebackup/pg_basebackup.c | 6 +-- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 7303bbe892..e4a0aba1eb 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -118,29 +118,29 @@ static const char *const auth_methods_local[] = { static char *share_path = NULL; /* values to be obtained from arguments */ -static char *pg_data = ""; -static char *encoding = ""; -static char *locale = ""; -static char *lc_collate = ""; -static char *lc_ctype = ""; -static char *lc_monetary = ""; -static char *lc_numeric = ""; -static char *lc_time = ""; -static char *lc_messages = ""; -static const char *default_text_search_config = ""; -static char *username = ""; +static char *pg_data = NULL; +static char *encoding = NULL; +static char *locale = NULL; +static char *lc_collate = NULL; +static char *lc_ctype = NULL; +static char *lc_monetary = NULL; +static char *lc_numeric = NULL; +static char *lc_time = NULL; +static char *lc_messages = NULL; +static const char *default_text_search_config = NULL; +static char *username = NULL; static bool pwprompt = false; static char *pwfilename = NULL; static char *superuser_password = NULL; -static const char *authmethodhost = ""; -static const char *authmethodlocal = ""; +static const char *authmethodhost = NULL; +static const char *authmethodlocal = NULL; static bool debug = false; static bool noclean = false; static bool do_sync = true; static bool sync_only = false; static bool show_setting = false; static bool data_checksums = false; -static char *xlog_dir = ""; +static char *xlog_dir = NULL; /* internal vars */ @@ -1285,7 +1285,6 @@ bootstrap_template1(void) { PG_CMD_DECL; char **line; - char *talkargs = ""; char **bki_lines; char headerline[MAXPGPATH]; char buf[64]; @@ -1293,9 +1292,6 @@ bootstrap_template1(void) printf(_("running bootstrap script ... ")); fflush(stdout); - if (debug) - talkargs = "-d 5"; - bki_lines = readfile(bki_file); /* Check that bki file appears to be of the right version */ @@ -1359,7 +1355,9 @@ bootstrap_template1(void) "\"%s\" --boot -x1 %s %s %s", backend_exec, data_checksums ? "-k" : "", - boot_options, talkargs); + boot_options, + debug ? "-d 5" : ""); + PG_CMD_OPEN; @@ -2136,6 +2134,10 @@ check_locale_name(int category, const char *locale, char **canonname) /* save may be pointing at a modifiable scratch variable, so copy it. */ save = pg_strdup(save); + /* for setlocale() call */ + if (!locale) + locale = ""; + /* set the locale with setlocale, to see if it accepts it. */ res = setlocale(category, locale); @@ -2223,19 +2225,19 @@ setlocales(void) /* set empty lc_* values to locale config if set */ - if (strlen(locale) > 0) + if (locale) { - if (strlen(lc_ctype) == 0) + if (!lc_ctype) lc_ctype = locale; - if (strlen(lc_collate) == 0) + if (!lc_collate) lc_collate = locale; - if (strlen(lc_numeric) == 0) + if (!lc_numeric) lc_numeric = locale; - if (strlen(lc_time) == 0) + if (!lc_time) lc_time = locale; - if (strlen(lc_monetary) == 0) + if (!lc_monetary) lc_monetary = locale; - if (strlen(lc_messages) == 0) + if (!lc_messages) lc_messages = locale; } @@ -2310,7 +2312,7 @@ usage(const char *progname) static void check_authmethod_unspecified(const char **authmethod) { - if (*authmethod == NULL || strlen(*authmethod) == 0) + if (*authmethod == NULL) { authwarning = _("\nWARNING: enabling \"trust\" authentication for local connections\n" "You can change this by editing pg_hba.conf or using the option -A, or\n" @@ -2367,7 +2369,7 @@ setup_pgdata(void) char *pgdata_get_env, *pgdata_set_env; - if (strlen(pg_data) == 0) + if (!pg_data) { pgdata_get_env = getenv("PGDATA"); if (pgdata_get_env && strlen(pgdata_get_env)) @@ -2479,7 +2481,7 @@ setup_locale_encoding(void) lc_time); } - if (strlen(encoding) == 0) + if (!encoding) { int ctype_enc; @@ -2589,10 +2591,10 @@ setup_data_file_paths(void) void setup_text_search(void) { - if (strlen(default_text_search_config) == 0) + if (!default_text_search_config) { default_text_search_config = find_matching_ts_config(lc_ctype); - if (default_text_search_config == NULL) + if (!default_text_search_config) { printf(_("%s: could not find suitable text search configuration for locale \"%s\"\n"), progname, lc_ctype); @@ -2728,7 +2730,7 @@ create_xlog_or_symlink(void) /* form name of the place for the subdirectory or symlink */ subdirloc = psprintf("%s/pg_wal", pg_data); - if (strcmp(xlog_dir, "") != 0) + if (xlog_dir) { int ret; @@ -3131,7 +3133,7 @@ main(int argc, char *argv[]) * Non-option argument specifies data directory as long as it wasn't * already specified with -D / --pgdata */ - if (optind < argc && strlen(pg_data) == 0) + if (optind < argc && !pg_data) { pg_data = pg_strdup(argv[optind]); optind++; @@ -3187,7 +3189,7 @@ main(int argc, char *argv[]) setup_bin_paths(argv[0]); effective_user = get_id(); - if (strlen(username) == 0) + if (!username) username = effective_user; if (strncmp(username, "pg_", 3) == 0) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index dfb9b5ddcb..51509d150e 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -76,7 +76,7 @@ typedef enum /* Global options */ static char *basedir = NULL; static TablespaceList tablespace_dirs = {NULL, NULL}; -static char *xlog_dir = ""; +static char *xlog_dir = NULL; static char format = 'p'; /* p(lain)/t(ar) */ static char *label = "pg_basebackup base backup"; static bool noclean = false; @@ -2347,7 +2347,7 @@ main(int argc, char **argv) temp_replication_slot = false; } - if (strcmp(xlog_dir, "") != 0) + if (xlog_dir) { if (format != 'p') { @@ -2398,7 +2398,7 @@ main(int argc, char **argv) } /* Create pg_wal symlink, if required */ - if (strcmp(xlog_dir, "") != 0) + if (xlog_dir) { char *linkloc; -- 2.14.1
From 1522287389b8873a872d5dc31d11c8b998c6239e Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Wed, 30 Aug 2017 22:28:36 -0400 Subject: [PATCH 3/3] Clean up excessive code The encoding ID was converted between string and number too many times, probably a remnant from the shell script days. --- src/bin/initdb/initdb.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index e4a0aba1eb..9d1e5d789f 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -145,7 +145,7 @@ static char *xlog_dir = NULL; /* internal vars */ static const char *progname; -static char *encodingid = "0"; +static int encodingid; static char *bki_file; static char *desc_file; static char *shdesc_file; @@ -236,7 +236,7 @@ static void writefile(char *path, char **lines); static FILE *popen_check(const char *command, const char *mode); static void exit_nicely(void); static char *get_id(void); -static char *get_encoding_id(char *encoding_name); +static int get_encoding_id(char *encoding_name); static void set_input(char **dest, char *filename); static void check_input(char *path); static void write_version_file(char *extrapath); @@ -636,7 +636,7 @@ encodingid_to_string(int enc) /* * get the encoding id for a given encoding name */ -static char * +static int get_encoding_id(char *encoding_name) { int enc; @@ -644,7 +644,7 @@ get_encoding_id(char *encoding_name) if (encoding_name && *encoding_name) { if ((enc = pg_valid_server_encoding(encoding_name)) >= 0) - return encodingid_to_string(enc); + return enc; } fprintf(stderr, _("%s: \"%s\" is not a valid server encoding name\n"), progname, encoding_name ? encoding_name : "(null)"); @@ -1328,7 +1328,7 @@ bootstrap_template1(void) bki_lines = replace_token(bki_lines, "POSTGRES", escape_quotes(username)); - bki_lines = replace_token(bki_lines, "ENCODING", encodingid); + bki_lines = replace_token(bki_lines, "ENCODING", encodingid_to_string(encodingid)); bki_lines = replace_token(bki_lines, "LC_COLLATE", escape_quotes(lc_collate)); @@ -2454,8 +2454,6 @@ setup_bin_paths(const char *argv0) void setup_locale_encoding(void) { - int user_enc; - setlocales(); if (strcmp(lc_ctype, lc_collate) == 0 && @@ -2505,12 +2503,11 @@ setup_locale_encoding(void) * UTF-8. */ #ifdef WIN32 + encodingid = PG_UTF8; printf(_("Encoding \"%s\" implied by locale is not allowed as a server-side encoding.\n" "The default database encoding will be set to \"%s\" instead.\n"), pg_encoding_to_char(ctype_enc), - pg_encoding_to_char(PG_UTF8)); - ctype_enc = PG_UTF8; - encodingid = encodingid_to_string(ctype_enc); + pg_encoding_to_char(encodingid)); #else fprintf(stderr, _("%s: locale \"%s\" requires unsupported encoding \"%s\"\n"), @@ -2524,17 +2521,16 @@ setup_locale_encoding(void) } else { - encodingid = encodingid_to_string(ctype_enc); + encodingid = ctype_enc; printf(_("The default database encoding has accordingly been set to \"%s\".\n"), - pg_encoding_to_char(ctype_enc)); + pg_encoding_to_char(encodingid)); } } else encodingid = get_encoding_id(encoding); - user_enc = atoi(encodingid); - if (!check_locale_encoding(lc_ctype, user_enc) || - !check_locale_encoding(lc_collate, user_enc)) + if (!check_locale_encoding(lc_ctype, encodingid) || + !check_locale_encoding(lc_collate, encodingid)) exit(1); /* check_locale_encoding printed the error */ } -- 2.14.1
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers