From b1aebbb6a86e96d7b8f3035ac730dfc24652496c Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Sat, 1 Mar 2014 22:14:14 -0500 Subject: [PATCH] Various Coverity-spotted fixes A number of issues were identified by the Coverity scanner and are addressed in this patch. None of these appear to be security issues and many are mostly cosmetic changes. Short comments for each of the changes follows. Correct the semi-colon placement in be-secure.c regarding SSL retries. Remove a useless comparison-to-NULL in proc.c (value is dereferenced prior to this check and therefore can't be NULL). Add checking of chmod() return values to initdb. Fix a couple minor memory leaks in initdb. Fix memory leak in pg_ctl- involves free'ing the config file contents. Use an int to capture fgetc() return instead of an enum in pg_dump. Fix minor memory leaks in pg_dump. (note minor change to convertOperatorReference()'s API) Check fclose()/remove() return codes in psql. Check fstat(), find_my_exec() return codes in psql. Various ECPG memory leak fixes. Check find_my_exec() return in ECPG. Explicitly ignore pqFlush return in libpq error-path. Change PQfnumber() to avoid doing an strdup() when no changes required. Remove a few useless check-against-NULL's (value deref'd beforehand). Check rmtree(), malloc() results in pg_regress. Also check get_alternative_expectfile() return in pg_regress. --- src/backend/libpq/be-secure.c | 2 +- src/backend/storage/lmgr/proc.c | 3 +- src/bin/initdb/initdb.c | 36 ++++++++++-- src/bin/pg_ctl/pg_ctl.c | 34 +++++++++++ src/bin/pg_dump/pg_backup_archiver.c | 4 +- src/bin/pg_dump/pg_dump.c | 79 ++++++++++++++++++-------- src/bin/psql/command.c | 12 +++- src/bin/psql/copy.c | 15 +++-- src/bin/psql/startup.c | 7 ++- src/interfaces/ecpg/ecpglib/execute.c | 27 +++++++-- src/interfaces/ecpg/preproc/ecpg.c | 6 +- src/interfaces/ecpg/preproc/type.c | 76 ++++++++++++++++++------- src/interfaces/ecpg/preproc/variable.c | 5 +- src/interfaces/libpq/fe-connect.c | 2 +- src/interfaces/libpq/fe-exec.c | 28 ++++++++- src/test/regress/pg_regress.c | 27 ++++++++- 16 files changed, 290 insertions(+), 73 deletions(-) diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 71f9747300..56ad6ab424 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -371,7 +371,7 @@ secure_write(Port *port, void *ptr, size_t len) * A handshake can fail, so be prepared to retry it, but only * a few times. */ - for (retries = 0; retries++;) + for (retries = 0;; retries++) { if (SSL_do_handshake(port->ssl) > 0) break; /* done */ diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index fb449a8820..fa460ca82e 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -1158,8 +1158,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) * Only do it if the worker is not working to protect against Xid * wraparound. */ - if ((autovac != NULL) && - (autovac_pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) && + if ((autovac_pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) && !(autovac_pgxact->vacuumFlags & PROC_VACUUM_FOR_WRAPAROUND)) { int pid = autovac->pid; diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 94151acb48..71248ee1bc 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -1293,7 +1293,12 @@ setup_config(void) snprintf(path, sizeof(path), "%s/postgresql.conf", pg_data); writefile(path, conflines); - chmod(path, S_IRUSR | S_IWUSR); + if (chmod(path, S_IRUSR | S_IWUSR) != 0) + { + fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), + progname, path, strerror(errno)); + exit_nicely(); + } /* * create the automatic configuration file to store the configuration @@ -1308,7 +1313,12 @@ setup_config(void) sprintf(path, "%s/%s", pg_data, PG_AUTOCONF_FILENAME); writefile(path, autoconflines); - chmod(path, S_IRUSR | S_IWUSR); + if (chmod(path, S_IRUSR | S_IWUSR) != 0) + { + fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), + progname, path, strerror(errno)); + exit_nicely(); + } free(conflines); @@ -1387,7 +1397,12 @@ setup_config(void) snprintf(path, sizeof(path), "%s/pg_hba.conf", pg_data); writefile(path, conflines); - chmod(path, S_IRUSR | S_IWUSR); + if (chmod(path, S_IRUSR | S_IWUSR) != 0) + { + fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), + progname, path, strerror(errno)); + exit_nicely(); + } free(conflines); @@ -1398,7 +1413,12 @@ setup_config(void) snprintf(path, sizeof(path), "%s/pg_ident.conf", pg_data); writefile(path, conflines); - chmod(path, S_IRUSR | S_IWUSR); + if (chmod(path, S_IRUSR | S_IWUSR) != 0) + { + fprintf(stderr, _("%s: could not change permissions of \"%s\": %s\n"), + progname, path, strerror(errno)); + exit_nicely(); + } free(conflines); @@ -1957,8 +1977,14 @@ setup_collation(void) * only, so this doesn't clash with "en_US" for LATIN1, say. */ if (normalize_locale_name(alias, localebuf)) + { + char *quoted_alias = escape_quotes(alias); + PG_CMD_PRINTF3("INSERT INTO tmp_pg_collation VALUES (E'%s', E'%s', %d);\n", - escape_quotes(alias), quoted_locale, enc); + quoted_alias, quoted_locale, enc); + free(quoted_alias); + } + free(quoted_locale); } /* Add an SQL-standard name */ diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 8a4a9193bf..e5229c0b78 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -154,6 +154,7 @@ static int CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, static pgpid_t get_pgpid(void); static char **readfile(const char *path); +static void free_readfile(char **optlines); static int start_postmaster(void); static void read_post_opts(void); @@ -369,6 +370,24 @@ readfile(const char *path) } +/* + * Free memory allocated for optlines through readfile() + */ +void +free_readfile(char **optlines) +{ + int i = 0; + + if (!optlines) + return; + + while (optlines[i++]) + free(optlines[i]); + + free(optlines); + + return; +} /* * start/test/stop routines @@ -572,6 +591,13 @@ test_postmaster_connection(bool do_checkpoint) } } } + + /* + * Free the results of readfile. + * + * This is safe to call even if optlines is NULL. + */ + free_readfile(optlines); } /* If we have a connection string, ping the server */ @@ -708,6 +734,9 @@ read_post_opts(void) if (exec_path == NULL) exec_path = optline; } + + /* Free the results of readfile. */ + free_readfile(optlines); } } } @@ -1201,8 +1230,13 @@ do_status(void) optlines = readfile(postopts_file); if (optlines != NULL) + { for (; *optlines != NULL; optlines++) fputs(*optlines, stdout); + + /* Free the results of readfile */ + free_readfile(optlines); + } return; } } diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 2b36e45533..46699a2d14 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -1947,8 +1947,10 @@ _discoverArchiveFormat(ArchiveHandle *AH) else AH->offSize = AH->intSize; - if ((AH->format = fgetc(fh)) == EOF) + if ((byteread = fgetc(fh)) == EOF) exit_horribly(modulename, "could not read input file: %s\n", strerror(errno)); + + AH->format = byteread; AH->lookahead[AH->lookaheadLen++] = AH->format; } else diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 4fabc1d4e5..770e97dce9 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -234,9 +234,9 @@ static char *format_function_arguments_old(Archive *fout, char **argnames); static char *format_function_signature(Archive *fout, FuncInfo *finfo, bool honor_quotes); -static const char *convertRegProcReference(Archive *fout, +static char *convertRegProcReference(Archive *fout, const char *proc); -static const char *convertOperatorReference(Archive *fout, const char *opr); +static char *convertOperatorReference(Archive *fout, const char *opr); static const char *convertTSFunction(Archive *fout, Oid funcOid); static Oid findLastBuiltinOid_V71(Archive *fout, const char *); static Oid findLastBuiltinOid_V70(Archive *fout); @@ -10246,6 +10246,8 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) char *oprjoin; char *oprcanmerge; char *oprcanhash; + char *oprregproc; + char *oprref; /* Skip if not to be dumped */ if (!oprinfo->dobj.dump || dataOnly) @@ -10352,8 +10354,12 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) oprcanmerge = PQgetvalue(res, 0, i_oprcanmerge); oprcanhash = PQgetvalue(res, 0, i_oprcanhash); - appendPQExpBuffer(details, " PROCEDURE = %s", - convertRegProcReference(fout, oprcode)); + oprregproc = convertRegProcReference(fout, oprcode); + if (oprregproc) + { + appendPQExpBuffer(details, " PROCEDURE = %s", oprregproc); + free(oprregproc); + } appendPQExpBuffer(oprid, "%s (", oprinfo->dobj.name); @@ -10388,13 +10394,19 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) else appendPQExpBufferStr(oprid, ", NONE)"); - name = convertOperatorReference(fout, oprcom); - if (name) - appendPQExpBuffer(details, ",\n COMMUTATOR = %s", name); + oprref = convertOperatorReference(fout, oprcom); + if (oprref) + { + appendPQExpBuffer(details, ",\n COMMUTATOR = %s", oprref); + free(oprref); + } - name = convertOperatorReference(fout, oprnegate); - if (name) - appendPQExpBuffer(details, ",\n NEGATOR = %s", name); + oprref = convertOperatorReference(fout, oprnegate); + if (oprref) + { + appendPQExpBuffer(details, ",\n NEGATOR = %s", oprref); + free(oprref); + } if (strcmp(oprcanmerge, "t") == 0) appendPQExpBufferStr(details, ",\n MERGES"); @@ -10402,13 +10414,19 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) if (strcmp(oprcanhash, "t") == 0) appendPQExpBufferStr(details, ",\n HASHES"); - name = convertRegProcReference(fout, oprrest); - if (name) - appendPQExpBuffer(details, ",\n RESTRICT = %s", name); + oprregproc = convertRegProcReference(fout, oprrest); + if (oprregproc) + { + appendPQExpBuffer(details, ",\n RESTRICT = %s", oprregproc); + free(oprregproc); + } - name = convertRegProcReference(fout, oprjoin); - if (name) - appendPQExpBuffer(details, ",\n JOIN = %s", name); + oprregproc = convertRegProcReference(fout, oprjoin); + if (oprregproc) + { + appendPQExpBuffer(details, ",\n JOIN = %s", oprregproc); + free(oprregproc); + } /* * DROP must be fully qualified in case same name appears in pg_catalog @@ -10453,12 +10471,13 @@ dumpOpr(Archive *fout, OprInfo *oprinfo) /* * Convert a function reference obtained from pg_operator * - * Returns what to print, or NULL if function references is InvalidOid + * Returns allocated string of what to print, or NULL if function references + * is InvalidOid. Returned string is expected to be free'd by the caller. * * In 7.3 the input is a REGPROCEDURE display; we have to strip the * argument-types part. In prior versions, the input is a REGPROC display. */ -static const char * +static char * convertRegProcReference(Archive *fout, const char *proc) { /* In all cases "-" means a null reference */ @@ -10488,20 +10507,21 @@ convertRegProcReference(Archive *fout, const char *proc) } /* REGPROC before 7.3 does not quote its result */ - return fmtId(proc); + return pg_strdup(fmtId(proc)); } /* * Convert an operator cross-reference obtained from pg_operator * - * Returns what to print, or NULL to print nothing + * Returns an allocated string of what to print, or NULL to print nothing. + * Caller is responsible for free'ing result string. * * In 7.3 and up the input is a REGOPERATOR display; we have to strip the * argument-types part, and add OPERATOR() decoration if the name is * schema-qualified. In older versions, the input is just a numeric OID, * which we search our operator list for. */ -static const char * +static char * convertOperatorReference(Archive *fout, const char *opr) { OprInfo *oprInfo; @@ -10549,7 +10569,7 @@ convertOperatorReference(Archive *fout, const char *opr) opr); return NULL; } - return oprInfo->dobj.name; + return pg_strdup(oprInfo->dobj.name); } /* @@ -11522,6 +11542,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo) const char *aggtransfn; const char *aggfinalfn; const char *aggsortop; + char *aggsortconvop; bool hypothetical; const char *aggtranstype; const char *aggtransspace; @@ -11665,6 +11686,12 @@ dumpAgg(Archive *fout, AggInfo *agginfo) { write_msg(NULL, "WARNING: aggregate function %s could not be dumped correctly for this database version; ignored\n", aggsig); + + if (aggfullsig) + free(aggfullsig); + + free(aggsig); + return; } @@ -11709,11 +11736,12 @@ dumpAgg(Archive *fout, AggInfo *agginfo) aggfinalfn); } - aggsortop = convertOperatorReference(fout, aggsortop); - if (aggsortop) + aggsortconvop = convertOperatorReference(fout, aggsortop); + if (aggsortconvop) { appendPQExpBuffer(details, ",\n SORTOP = %s", - aggsortop); + aggsortconvop); + free(aggsortconvop); } if (hypothetical) @@ -12413,6 +12441,7 @@ dumpUserMappings(Archive *fout, destroyPQExpBuffer(query); destroyPQExpBuffer(delq); + destroyPQExpBuffer(tag); destroyPQExpBuffer(q); } diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 6a963b5d6f..fd64ba824f 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -2027,14 +2027,20 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf, if (fwrite(query_buf->data, 1, ql, stream) != ql) { psql_error("%s: %s\n", fname, strerror(errno)); - fclose(stream); - remove(fname); + + if (fclose(stream) != 0) + psql_error("%s: %s\n", fname, strerror(errno)); + + if (remove(fname) != 0) + psql_error("%s: %s\n", fname, strerror(errno)); + error = true; } else if (fclose(stream) != 0) { psql_error("%s: %s\n", fname, strerror(errno)); - remove(fname); + if (remove(fname) != 0) + psql_error("%s: %s\n", fname, strerror(errno)); error = true; } } diff --git a/src/bin/psql/copy.c b/src/bin/psql/copy.c index dbb2255db6..9e815b136e 100644 --- a/src/bin/psql/copy.c +++ b/src/bin/psql/copy.c @@ -345,13 +345,20 @@ do_copy(const char *args) if (!options->program) { + int result; + /* make sure the specified file is not a directory */ - fstat(fileno(copystream), &st); - if (S_ISDIR(st.st_mode)) - { - fclose(copystream); + if ((result = fstat(fileno(copystream), &st)) < 0) + psql_error("could not stat file: %s\n", + strerror(errno)); + + if (result == 0 && S_ISDIR(st.st_mode)) psql_error("%s: cannot copy from/to a directory\n", options->file); + + if (result < 0 || S_ISDIR(st.st_mode)) + { + fclose(copystream); free_copy_options(options); return false; } diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 106199273f..d5f1c0d408 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -603,7 +603,12 @@ process_psqlrc(char *argv0) char etc_path[MAXPGPATH]; char *envrc = getenv("PSQLRC"); - find_my_exec(argv0, my_exec_path); + if (find_my_exec(argv0, my_exec_path) < 0) + { + fprintf(stderr, _("%s: could not find own program executable\n"), argv0); + exit(EXIT_FAILURE); + } + get_etc_path(my_exec_path, etc_path); snprintf(rc_file, MAXPGPATH, "%s/%s", etc_path, SYSPSQLRC); diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index 8e165d97ac..a90fb41483 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -855,14 +855,22 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari for (element = 0; element < var->arrsize; element++) { + int result; + nval = PGTYPESnumeric_new(); if (!nval) return false; if (var->type == ECPGt_numeric) - PGTYPESnumeric_copy((numeric *) ((var + var->offset * element)->value), nval); + result = PGTYPESnumeric_copy((numeric *) ((var + var->offset * element)->value), nval); else - PGTYPESnumeric_from_decimal((decimal *) ((var + var->offset * element)->value), nval); + result = PGTYPESnumeric_from_decimal((decimal *) ((var + var->offset * element)->value), nval); + + if (result != 0) + { + PGTYPESnumeric_free(nval); + return false; + } str = PGTYPESnumeric_to_asc(nval, nval->dscale); slen = strlen(str); @@ -882,14 +890,22 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari } else { + int result; + nval = PGTYPESnumeric_new(); if (!nval) return false; if (var->type == ECPGt_numeric) - PGTYPESnumeric_copy((numeric *) (var->value), nval); + result = PGTYPESnumeric_copy((numeric *) (var->value), nval); else - PGTYPESnumeric_from_decimal((decimal *) (var->value), nval); + result = PGTYPESnumeric_from_decimal((decimal *) (var->value), nval); + + if (result != 0) + { + PGTYPESnumeric_free(nval); + return false; + } str = PGTYPESnumeric_to_asc(nval, nval->dscale); slen = strlen(str); @@ -1026,7 +1042,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari { str = quote_postgres(PGTYPEStimestamp_to_asc(*(timestamp *) ((var + var->offset * element)->value)), quote, lineno); if (!str) + { + ecpg_free(mallocedval); return false; + } slen = strlen(str); diff --git a/src/interfaces/ecpg/preproc/ecpg.c b/src/interfaces/ecpg/preproc/ecpg.c index cd110f6b48..339ca8821a 100644 --- a/src/interfaces/ecpg/preproc/ecpg.c +++ b/src/interfaces/ecpg/preproc/ecpg.c @@ -138,7 +138,11 @@ main(int argc, char *const argv[]) progname = get_progname(argv[0]); - find_my_exec(argv[0], my_exec_path); + if (find_my_exec(argv[0], my_exec_path) < 0) + { + fprintf(stderr, _("%s: could not locate my own executable path\n"), argv[0]); + return (ILLEGAL_OPTION); + } output_filename = NULL; while ((c = getopt_long(argc, argv, "vcio:I:tD:dC:r:h?", ecpg_options, NULL)) != -1) diff --git a/src/interfaces/ecpg/preproc/type.c b/src/interfaces/ecpg/preproc/type.c index c24b4c2fbc..2982cb6204 100644 --- a/src/interfaces/ecpg/preproc/type.c +++ b/src/interfaces/ecpg/preproc/type.c @@ -308,7 +308,11 @@ ECPGdump_a_type(FILE *o, const char *name, struct ECPGtype * type, const int bra if (ind_type != NULL) { if (ind_type->type == ECPGt_NO_INDICATOR) - ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, mm_strdup("-1"), NULL, ind_prefix, 0); + { + char *str_neg_one = mm_strdup("-1"); + ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, str_neg_one, NULL, ind_prefix, 0); + free(str_neg_one); + } else { ECPGdump_a_simple(o, ind_name, ind_type->u.element->type, @@ -318,37 +322,71 @@ ECPGdump_a_type(FILE *o, const char *name, struct ECPGtype * type, const int bra } break; case ECPGt_struct: - if (indicator_set && ind_type->type != ECPGt_struct) - mmfatal(INDICATOR_NOT_STRUCT, "indicator for struct has to be a struct"); + { + char *str_one = mm_strdup("1"); - ECPGdump_a_struct(o, name, ind_name, mm_strdup("1"), type, ind_type, prefix, ind_prefix); + if (indicator_set && ind_type->type != ECPGt_struct) + mmfatal(INDICATOR_NOT_STRUCT, "indicator for struct has to be a struct"); + + ECPGdump_a_struct(o, name, ind_name, str_one, type, ind_type, prefix, ind_prefix); + free(str_one); + } break; case ECPGt_union: /* cannot dump a complete union */ base_yyerror("type of union has to be specified"); break; case ECPGt_char_variable: - if (indicator_set && (ind_type->type == ECPGt_struct || ind_type->type == ECPGt_array)) - mmfatal(INDICATOR_NOT_SIMPLE, "indicator for simple data type has to be simple"); + { + /* Allocate for each, as there are code-paths where the values get stomped on. */ + char *str_varchar_one = mm_strdup("1"); + char *str_arr_one = mm_strdup("1"); + char *str_neg_one = mm_strdup("-1"); - ECPGdump_a_simple(o, name, type->type, mm_strdup("1"), (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : mm_strdup("1"), struct_sizeof, prefix, 0); - if (ind_type != NULL) - ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : mm_strdup("-1"), ind_struct_sizeof, ind_prefix, 0); + if (indicator_set && (ind_type->type == ECPGt_struct || ind_type->type == ECPGt_array)) + mmfatal(INDICATOR_NOT_SIMPLE, "indicator for simple data type has to be simple"); + + ECPGdump_a_simple(o, name, type->type, str_varchar_one, (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : str_arr_one, struct_sizeof, prefix, 0); + if (ind_type != NULL) + ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : str_neg_one, ind_struct_sizeof, ind_prefix, 0); + + free(str_varchar_one); + free(str_arr_one); + free(str_neg_one); + } break; case ECPGt_descriptor: - if (indicator_set && (ind_type->type == ECPGt_struct || ind_type->type == ECPGt_array)) - mmfatal(INDICATOR_NOT_SIMPLE, "indicator for simple data type has to be simple"); + { + /* Allocate for each, as there are code-paths where the values get stomped on. */ + char *str_neg_one = mm_strdup("-1"); + char *ind_type_neg_one = mm_strdup("-1"); - ECPGdump_a_simple(o, name, type->type, NULL, mm_strdup("-1"), NULL, prefix, 0); - if (ind_type != NULL) - ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, mm_strdup("-1"), NULL, ind_prefix, 0); + if (indicator_set && (ind_type->type == ECPGt_struct || ind_type->type == ECPGt_array)) + mmfatal(INDICATOR_NOT_SIMPLE, "indicator for simple data type has to be simple"); + + ECPGdump_a_simple(o, name, type->type, NULL, str_neg_one, NULL, prefix, 0); + if (ind_type != NULL) + ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, ind_type_neg_one, NULL, ind_prefix, 0); + + free(str_neg_one); + free(ind_type_neg_one); + } break; default: - if (indicator_set && (ind_type->type == ECPGt_struct || ind_type->type == ECPGt_array)) - mmfatal(INDICATOR_NOT_SIMPLE, "indicator for simple data type has to be simple"); + { + /* Allocate for each, as there are code-paths where the values get stomped on. */ + char *str_neg_one = mm_strdup("-1"); + char *ind_type_neg_one = mm_strdup("-1"); - ECPGdump_a_simple(o, name, type->type, type->size, (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : mm_strdup("-1"), struct_sizeof, prefix, type->counter); - if (ind_type != NULL) - ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : mm_strdup("-1"), ind_struct_sizeof, ind_prefix, 0); + if (indicator_set && (ind_type->type == ECPGt_struct || ind_type->type == ECPGt_array)) + mmfatal(INDICATOR_NOT_SIMPLE, "indicator for simple data type has to be simple"); + + ECPGdump_a_simple(o, name, type->type, type->size, (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : str_neg_one, struct_sizeof, prefix, type->counter); + if (ind_type != NULL) + ECPGdump_a_simple(o, ind_name, ind_type->type, ind_type->size, (arr_str_siz && strcmp(arr_str_siz, "0") != 0) ? arr_str_siz : ind_type_neg_one, ind_struct_sizeof, ind_prefix, 0); + + free(str_neg_one); + free(ind_type_neg_one); + } break; } } diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c index cc923a797b..d762286b94 100644 --- a/src/interfaces/ecpg/preproc/variable.c +++ b/src/interfaces/ecpg/preproc/variable.c @@ -437,6 +437,7 @@ remove_variable_from_list(struct arguments ** list, struct variable * var) void dump_variables(struct arguments * list, int mode) { + char *str_zero = mm_strdup("0"); if (list == NULL) return; @@ -450,11 +451,13 @@ dump_variables(struct arguments * list, int mode) /* Then the current element and its indicator */ ECPGdump_a_type(yyout, list->variable->name, list->variable->type, list->variable->brace_level, list->indicator->name, list->indicator->type, list->indicator->brace_level, - NULL, NULL, mm_strdup("0"), NULL, NULL); + NULL, NULL, str_zero, NULL, NULL); /* Then release the list element. */ if (mode != 0) free(list); + + free(str_zero); } void diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 35672a747b..1f0eeaf4c6 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2876,7 +2876,7 @@ closePGconn(PGconn *conn) */ pqPutMsgStart('X', false, conn); pqPutMsgEnd(conn); - pqFlush(conn); + (void) pqFlush(conn); } /* diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index ecf9c21311..6f74108bd6 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -2724,7 +2724,8 @@ PQfnumber(const PGresult *res, const char *field_name) { char *field_case; bool in_quotes; - char *iptr; + bool all_lower = true; + const char *iptr; char *optr; int i; @@ -2740,6 +2741,28 @@ PQfnumber(const PGresult *res, const char *field_name) res->attDescs == NULL) return -1; + /* + * Check if we can avoid the strdup() and related work because the + * passed-in string wouldn't be changed before we do the check anyway. + */ + for (iptr = field_name; *iptr; iptr++) + { + char c = *iptr; + + if (c == '"' || c != pg_tolower((unsigned char) c)) + { + all_lower = false; + break; + } + } + + if (all_lower) + for (i = 0; i < res->numAttributes; i++) + if (strcmp(field_name, res->attDescs[i].name) == 0) + return i; + + /* Fall through to the normal check if that didn't work out. */ + /* * Note: this code will not reject partially quoted strings, eg * foo"BAR"foo will become fooBARfoo when it probably ought to be an error @@ -2883,7 +2906,7 @@ PQoidStatus(const PGresult *res) size_t len; - if (!res || !res->cmdStatus || strncmp(res->cmdStatus, "INSERT ", 7) != 0) + if (!res || strncmp(res->cmdStatus, "INSERT ", 7) != 0) return ""; len = strspn(res->cmdStatus + 7, "0123456789"); @@ -2907,7 +2930,6 @@ PQoidValue(const PGresult *res) unsigned long result; if (!res || - !res->cmdStatus || strncmp(res->cmdStatus, "INSERT ", 7) != 0 || res->cmdStatus[7] < '0' || res->cmdStatus[7] > '9') diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 3a492449fb..541ce8b33d 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -450,7 +450,12 @@ convert_sourcefiles_in(char *source_subdir, char *dest_dir, char *dest_subdir, c * Windows. See pgsql-hackers discussion of 2008-01-18. */ if (directory_exists(testtablespace)) - rmtree(testtablespace, true); + if (!rmtree(testtablespace, true)) + { + fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\": %s\n"), + progname, testtablespace, strerror(errno)); + exit(2); + } make_directory(testtablespace); #endif @@ -1152,6 +1157,9 @@ get_alternative_expectfile(const char *expectfile, int i) char *tmp = (char *) malloc(ssize); char *s = (char *) malloc(ssize); + if (!tmp || !s) + return NULL; + strcpy(tmp, expectfile); last_dot = strrchr(tmp, '.'); if (!last_dot) @@ -1258,8 +1266,18 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul char *alt_expectfile; alt_expectfile = get_alternative_expectfile(expectfile, i); + if (!alt_expectfile) + { + fprintf(stderr, _("Unable to check secondary comparison files: %s\n"), + strerror(errno)); + exit(2); + } + if (!file_exists(alt_expectfile)) + { + free(alt_expectfile); continue; + } snprintf(cmd, sizeof(cmd), SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE, @@ -1268,6 +1286,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul if (run_diff(cmd, diff) == 0) { unlink(diff); + free(alt_expectfile); return false; } @@ -2105,7 +2124,11 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc if (directory_exists(temp_install)) { header(_("removing existing temp installation")); - rmtree(temp_install, true); + if (!rmtree(temp_install, true)) + { + fprintf(stderr, _("\n%s: could not remove temp installation \"%s\": %s\n"), progname, temp_install, strerror(errno)); + exit(2); + } } header(_("creating temporary installation"));