public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v1] Change pg_attribute_noreturn() to pg_noreturn, move to before return type.
10+ messages / 5 participants
[nested] [flat]

* [PATCH v1] Change pg_attribute_noreturn() to pg_noreturn, move to before return type.
@ 2019-11-12 21:55 Andres Freund <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Andres Freund @ 2019-11-12 21:55 UTC (permalink / raw)

---
 contrib/cube/cubedata.h                      |  2 +-
 contrib/dblink/dblink.c                      |  6 ++----
 contrib/pgcrypto/px.h                        |  2 +-
 contrib/seg/segdata.h                        |  2 +-
 src/backend/postmaster/autovacuum.c          |  4 ++--
 src/backend/postmaster/pgarch.c              |  2 +-
 src/backend/postmaster/pgstat.c              |  2 +-
 src/backend/postmaster/postmaster.c          |  4 ++--
 src/backend/postmaster/syslogger.c           |  2 +-
 src/backend/replication/logical/tablesync.c  |  3 +--
 src/backend/replication/walsender.c          |  2 +-
 src/backend/utils/adt/json.c                 |  4 ++--
 src/backend/utils/adt/ri_triggers.c          |  8 ++++----
 src/backend/utils/fmgr/dfmgr.c               |  4 ++--
 src/backend/utils/misc/guc.c                 |  3 +--
 src/bin/pg_dump/pg_backup_utils.h            |  2 +-
 src/bin/pg_upgrade/pg_upgrade.h              |  2 +-
 src/bin/pgbench/pgbench.h                    | 12 ++++++------
 src/include/bootstrap/bootstrap.h            |  4 ++--
 src/include/c.h                              | 10 +++++-----
 src/include/mb/pg_wchar.h                    |  6 +++---
 src/include/parser/parse_relation.h          |  6 +++---
 src/include/parser/scanner.h                 |  2 +-
 src/include/pgstat.h                         |  2 +-
 src/include/postmaster/autovacuum.h          |  4 ++--
 src/include/postmaster/bgworker_internals.h  |  2 +-
 src/include/postmaster/bgwriter.h            |  4 ++--
 src/include/postmaster/pgarch.h              |  2 +-
 src/include/postmaster/postmaster.h          |  4 ++--
 src/include/postmaster/startup.h             |  2 +-
 src/include/postmaster/syslogger.h           |  2 +-
 src/include/postmaster/walwriter.h           |  2 +-
 src/include/replication/walreceiver.h        |  2 +-
 src/include/replication/walsender_private.h  |  2 +-
 src/include/storage/ipc.h                    |  2 +-
 src/include/storage/lock.h                   |  2 +-
 src/include/tcop/tcopprot.h                  | 10 +++++-----
 src/include/utils/datetime.h                 |  4 ++--
 src/include/utils/elog.h                     |  8 ++++----
 src/include/utils/help_config.h              |  2 +-
 src/interfaces/ecpg/preproc/preproc_extern.h |  2 +-
 src/pl/plpgsql/src/plpgsql.h                 |  2 +-
 src/test/modules/test_shm_mq/test_shm_mq.h   |  2 +-
 src/test/modules/worker_spi/worker_spi.c     |  2 +-
 src/timezone/zic.c                           |  4 ++--
 45 files changed, 79 insertions(+), 83 deletions(-)

diff --git a/contrib/cube/cubedata.h b/contrib/cube/cubedata.h
index dbe7d4f7429..178fb46f217 100644
--- a/contrib/cube/cubedata.h
+++ b/contrib/cube/cubedata.h
@@ -61,7 +61,7 @@ typedef struct NDBOX
 
 /* in cubescan.l */
 extern int	cube_yylex(void);
-extern void cube_yyerror(NDBOX **result, const char *message) pg_attribute_noreturn();
+extern pg_noreturn void cube_yyerror(NDBOX **result, const char *message);
 extern void cube_scanner_init(const char *str);
 extern void cube_scanner_finish(void);
 
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 7e225589a9b..371b89383d9 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -152,8 +152,7 @@ xpstrdup(const char *in)
 	return pstrdup(in);
 }
 
-static void
-pg_attribute_noreturn()
+static pg_noreturn void
 dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
 {
 	char	   *msg = pchomp(PQerrorMessage(conn));
@@ -163,8 +162,7 @@ dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
 	elog(ERROR, "%s: %s", p2, msg);
 }
 
-static void
-pg_attribute_noreturn()
+static pg_noreturn void
 dblink_conn_not_avail(const char *conname)
 {
 	if (conname)
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 0d4722a04a0..a3f3a182a66 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -184,7 +184,7 @@ int			px_find_hmac(const char *name, PX_HMAC **res);
 int			px_find_cipher(const char *name, PX_Cipher **res);
 int			px_find_combo(const char *name, PX_Combo **res);
 
-void		px_THROW_ERROR(int err) pg_attribute_noreturn();
+pg_noreturn void px_THROW_ERROR(int err);
 const char *px_strerror(int err);
 
 const char *px_resolve_alias(const PX_Alias *aliases, const char *name);
diff --git a/contrib/seg/segdata.h b/contrib/seg/segdata.h
index 9488bf3a81a..a5d8ac3f591 100644
--- a/contrib/seg/segdata.h
+++ b/contrib/seg/segdata.h
@@ -16,7 +16,7 @@ extern int	significant_digits(const char *str);
 
 /* in segscan.l */
 extern int	seg_yylex(void);
-extern void seg_yyerror(SEG *result, const char *message) pg_attribute_noreturn();
+extern pg_noreturn void seg_yyerror(SEG *result, const char *message);
 extern void seg_scanner_init(const char *str);
 extern void seg_scanner_finish(void);
 
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c1dd8168caf..a266bdeb1ae 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -307,8 +307,8 @@ int			AutovacuumLauncherPid = 0;
 static pid_t avlauncher_forkexec(void);
 static pid_t avworker_forkexec(void);
 #endif
-NON_EXEC_STATIC void AutoVacWorkerMain(int argc, char *argv[]) pg_attribute_noreturn();
-NON_EXEC_STATIC void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn();
+NON_EXEC_STATIC pg_noreturn void AutoVacWorkerMain(int argc, char *argv[]);
+NON_EXEC_STATIC pg_noreturn void AutoVacLauncherMain(int argc, char *argv[]);
 
 static Oid	do_start_worker(void);
 static void launcher_determine_sleep(bool canlaunch, bool recursing,
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index f84f882c4cb..5b8460e4d0f 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -96,7 +96,7 @@ static volatile sig_atomic_t ready_to_stop = false;
 static pid_t pgarch_forkexec(void);
 #endif
 
-NON_EXEC_STATIC void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
+NON_EXEC_STATIC pg_noreturn void PgArchiverMain(int argc, char *argv[]);
 static void pgarch_exit(SIGNAL_ARGS);
 static void ArchSigHupHandler(SIGNAL_ARGS);
 static void ArchSigTermHandler(SIGNAL_ARGS);
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 011076c3e3e..c0641ce6123 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -284,7 +284,7 @@ static instr_time total_func_time;
 static pid_t pgstat_forkexec(void);
 #endif
 
-NON_EXEC_STATIC void PgstatCollectorMain(int argc, char *argv[]) pg_attribute_noreturn();
+NON_EXEC_STATIC pg_noreturn void PgstatCollectorMain(int argc, char *argv[]);
 static void pgstat_exit(SIGNAL_ARGS);
 static void pgstat_beshutdown_hook(int code, Datum arg);
 static void pgstat_sighup_handler(SIGNAL_ARGS);
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 5f303591652..1f5c6ab403e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -400,8 +400,8 @@ static void LogChildExit(int lev, const char *procname,
 						 int pid, int exitstatus);
 static void PostmasterStateMachine(void);
 static void BackendInitialize(Port *port);
-static void BackendRun(Port *port) pg_attribute_noreturn();
-static void ExitPostmaster(int status) pg_attribute_noreturn();
+static pg_noreturn void BackendRun(Port *port);
+static pg_noreturn void ExitPostmaster(int status);
 static int	ServerLoop(void);
 static int	BackendStartup(Port *port);
 static int	ProcessStartupPacket(Port *port, bool secure_done);
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index bb2baff7631..cda31f10546 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -137,7 +137,7 @@ static volatile sig_atomic_t rotation_requested = false;
 static pid_t syslogger_forkexec(void);
 static void syslogger_parseArgs(int argc, char *argv[]);
 #endif
-NON_EXEC_STATIC void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn();
+NON_EXEC_STATIC pg_noreturn void SysLoggerMain(int argc, char *argv[]);
 static void process_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
 static void flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
 static FILE *logfile_open(const char *filename, const char *mode,
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 7881079e96b..a3302e12663 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -117,8 +117,7 @@ StringInfo	copybuf = NULL;
 /*
  * Exit routine for synchronization worker.
  */
-static void
-pg_attribute_noreturn()
+static pg_noreturn void
 finish_sync_worker(void)
 {
 	/*
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 7f5671504f7..6ab7966818f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -222,7 +222,7 @@ typedef void (*WalSndSendDataCallback) (void);
 static void WalSndLoop(WalSndSendDataCallback send_data);
 static void InitWalSenderSlot(void);
 static void WalSndKill(int code, Datum arg);
-static void WalSndShutdown(void) pg_attribute_noreturn();
+static pg_noreturn void WalSndShutdown(void);
 static void XLogSendPhysical(void);
 static void XLogSendLogical(void);
 static void WalSndDone(WalSndSendDataCallback send_data);
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index d4ba3bd87db..fd03c0315c5 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -84,8 +84,8 @@ static void parse_object_field(JsonLexContext *lex, JsonSemAction *sem);
 static void parse_object(JsonLexContext *lex, JsonSemAction *sem);
 static void parse_array_element(JsonLexContext *lex, JsonSemAction *sem);
 static void parse_array(JsonLexContext *lex, JsonSemAction *sem);
-static void report_parse_error(JsonParseContext ctx, JsonLexContext *lex) pg_attribute_noreturn();
-static void report_invalid_token(JsonLexContext *lex) pg_attribute_noreturn();
+static pg_noreturn void report_parse_error(JsonParseContext ctx, JsonLexContext *lex);
+static pg_noreturn void report_invalid_token(JsonLexContext *lex);
 static int	report_json_context(JsonLexContext *lex);
 static char *extract_mb_char(char *s);
 static void composite_to_json(Datum composite, StringInfo result,
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index caf27f8bcb4..bd94f22507e 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -218,10 +218,10 @@ static bool ri_PerformCheck(const RI_ConstraintInfo *riinfo,
 static void ri_ExtractValues(Relation rel, TupleTableSlot *slot,
 							 const RI_ConstraintInfo *riinfo, bool rel_is_pk,
 							 Datum *vals, char *nulls);
-static void ri_ReportViolation(const RI_ConstraintInfo *riinfo,
-							   Relation pk_rel, Relation fk_rel,
-							   TupleTableSlot *violatorslot, TupleDesc tupdesc,
-							   int queryno, bool partgone) pg_attribute_noreturn();
+static pg_noreturn void ri_ReportViolation(const RI_ConstraintInfo *riinfo,
+										   Relation pk_rel, Relation fk_rel,
+										   TupleTableSlot *violatorslot, TupleDesc tupdesc,
+										   int queryno, bool partgone);
 
 
 /*
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index be684786d64..88c4978f348 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -77,8 +77,8 @@ static DynamicFileList *file_tail = NULL;
 char	   *Dynamic_library_path;
 
 static void *internal_load_library(const char *libname);
-static void incompatible_module_error(const char *libname,
-									  const Pg_magic_struct *module_magic_data) pg_attribute_noreturn();
+static pg_noreturn void incompatible_module_error(const char *libname,
+												  const Pg_magic_struct *module_magic_data);
 static void internal_unload_library(const char *libname);
 static bool file_exists(const char *name);
 static char *expand_dynamic_library_name(const char *name);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 994bf37477a..92e84bda2ba 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11624,8 +11624,7 @@ assign_recovery_target_timeline(const char *newval, void *extra)
  * string.
  */
 
-static void
-pg_attribute_noreturn()
+static pg_noreturn void
 error_multiple_recovery_targets(void)
 {
 	ereport(ERROR,
diff --git a/src/bin/pg_dump/pg_backup_utils.h b/src/bin/pg_dump/pg_backup_utils.h
index 619c9fe08c8..c5c81b41a49 100644
--- a/src/bin/pg_dump/pg_backup_utils.h
+++ b/src/bin/pg_dump/pg_backup_utils.h
@@ -31,7 +31,7 @@ extern const char *progname;
 
 extern void set_dump_section(const char *arg, int *dumpSections);
 extern void on_exit_nicely(on_exit_nicely_callback function, void *arg);
-extern void exit_nicely(int code) pg_attribute_noreturn();
+extern pg_noreturn void exit_nicely(int code);
 
 #define fatal(...) do { pg_log_error(__VA_ARGS__); exit_nicely(1); } while(0)
 
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 729f86aa32d..6150883a4ed 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -434,7 +434,7 @@ int			get_user_info(char **user_name_p);
 void		check_ok(void);
 void		report_status(eLogType type, const char *fmt,...) pg_attribute_printf(2, 3);
 void		pg_log(eLogType type, const char *fmt,...) pg_attribute_printf(2, 3);
-void		pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2) pg_attribute_noreturn();
+pg_noreturn void pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2);
 void		end_progress_output(void);
 void		prep_status(const char *fmt,...) pg_attribute_printf(1, 2);
 void		check_ok(void);
diff --git a/src/bin/pgbench/pgbench.h b/src/bin/pgbench/pgbench.h
index c4a1e298e06..3759528e44d 100644
--- a/src/bin/pgbench/pgbench.h
+++ b/src/bin/pgbench/pgbench.h
@@ -141,9 +141,9 @@ extern PgBenchExpr *expr_parse_result;
 
 extern int	expr_yyparse(yyscan_t yyscanner);
 extern int	expr_yylex(union YYSTYPE *lvalp, yyscan_t yyscanner);
-extern void expr_yyerror(yyscan_t yyscanner, const char *str) pg_attribute_noreturn();
-extern void expr_yyerror_more(yyscan_t yyscanner, const char *str,
-							  const char *more) pg_attribute_noreturn();
+extern pg_noreturn void expr_yyerror(yyscan_t yyscanner, const char *str);
+extern pg_noreturn void expr_yyerror_more(yyscan_t yyscanner, const char *str,
+										  const char *more);
 extern bool expr_lex_one_word(PsqlScanState state, PQExpBuffer word_buf,
 							  int *offset);
 extern yyscan_t expr_scanner_init(PsqlScanState state,
@@ -156,9 +156,9 @@ extern char *expr_scanner_get_substring(PsqlScanState state,
 										bool chomp);
 extern int	expr_scanner_get_lineno(PsqlScanState state, int offset);
 
-extern void syntax_error(const char *source, int lineno, const char *line,
-						 const char *cmd, const char *msg,
-						 const char *more, int col) pg_attribute_noreturn();
+extern pg_noreturn void syntax_error(const char *source, int lineno, const char *line,
+									 const char *cmd, const char *msg,
+									 const char *more, int col);
 
 extern bool strtoint64(const char *str, bool errorOK, int64 *pi);
 extern bool strtodouble(const char *str, bool errorOK, double *pd);
diff --git a/src/include/bootstrap/bootstrap.h b/src/include/bootstrap/bootstrap.h
index 85705602cb3..151d0ab3557 100644
--- a/src/include/bootstrap/bootstrap.h
+++ b/src/include/bootstrap/bootstrap.h
@@ -32,7 +32,7 @@ extern Form_pg_attribute attrtypes[MAXATTR];
 extern int	numattr;
 
 
-extern void AuxiliaryProcessMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern pg_noreturn void AuxiliaryProcessMain(int argc, char *argv[]);
 
 extern void closerel(char *name);
 extern void boot_openrel(char *name);
@@ -57,6 +57,6 @@ extern void boot_get_type_io_data(Oid typid,
 extern int	boot_yyparse(void);
 
 extern int	boot_yylex(void);
-extern void boot_yyerror(const char *str) pg_attribute_noreturn();
+extern pg_noreturn void boot_yyerror(const char *str);
 
 #endif							/* BOOTSTRAP_H */
diff --git a/src/include/c.h b/src/include/c.h
index c95acd35f7f..d5000557c0e 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -135,7 +135,7 @@
 /* GCC, Sunpro and XLC support aligned, packed and noreturn */
 #if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__IBMC__)
 #define pg_attribute_aligned(a) __attribute__((aligned(a)))
-#define pg_attribute_noreturn() __attribute__((noreturn))
+#define pg_noreturn __attribute__((noreturn))
 #define pg_attribute_packed() __attribute__((packed))
 #define HAVE_PG_ATTRIBUTE_NORETURN 1
 #else
@@ -144,7 +144,7 @@
  * affect code functionality; they *must* be implemented by the compiler
  * if they are to be used.
  */
-#define pg_attribute_noreturn()
+#define pg_noreturn
 #endif
 
 /*
@@ -813,9 +813,9 @@ typedef NameData *Name;
  * we should declare it as long as !FRONTEND.
  */
 #ifndef FRONTEND
-extern void ExceptionalCondition(const char *conditionName,
-								 const char *errorType,
-								 const char *fileName, int lineNumber) pg_attribute_noreturn();
+extern pg_noreturn void ExceptionalCondition(const char *conditionName,
+											 const char *errorType,
+											 const char *fileName, int lineNumber);
 #endif
 
 /*
diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h
index 3e3e6c470e9..05169fc87de 100644
--- a/src/include/mb/pg_wchar.h
+++ b/src/include/mb/pg_wchar.h
@@ -608,9 +608,9 @@ extern void check_encoding_conversion_args(int src_encoding,
 										   int expected_src_encoding,
 										   int expected_dest_encoding);
 
-extern void report_invalid_encoding(int encoding, const char *mbstr, int len) pg_attribute_noreturn();
-extern void report_untranslatable_char(int src_encoding, int dest_encoding,
-									   const char *mbstr, int len) pg_attribute_noreturn();
+extern pg_noreturn void report_invalid_encoding(int encoding, const char *mbstr, int len);
+extern pg_noreturn void report_untranslatable_char(int src_encoding, int dest_encoding,
+												   const char *mbstr, int len);
 
 extern void local2local(const unsigned char *l, unsigned char *p, int len,
 						int src_encoding, int dest_encoding, const unsigned char *tab);
diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h
index f7e078172de..4859cce8d1e 100644
--- a/src/include/parser/parse_relation.h
+++ b/src/include/parser/parse_relation.h
@@ -116,9 +116,9 @@ extern bool isLockedRefname(ParseState *pstate, const char *refname);
 extern void addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte,
 						  bool addToJoinList,
 						  bool addToRelNameSpace, bool addToVarNameSpace);
-extern void errorMissingRTE(ParseState *pstate, RangeVar *relation) pg_attribute_noreturn();
-extern void errorMissingColumn(ParseState *pstate,
-							   const char *relname, const char *colname, int location) pg_attribute_noreturn();
+extern pg_noreturn void errorMissingRTE(ParseState *pstate, RangeVar *relation);
+extern pg_noreturn void errorMissingColumn(ParseState *pstate,
+										   const char *relname, const char *colname, int location);
 extern void expandRTE(RangeTblEntry *rte, int rtindex, int sublevels_up,
 					  int location, bool include_dropped,
 					  List **colnames, List **colvars);
diff --git a/src/include/parser/scanner.h b/src/include/parser/scanner.h
index 731a2bd264a..c10d42bf35a 100644
--- a/src/include/parser/scanner.h
+++ b/src/include/parser/scanner.h
@@ -128,6 +128,6 @@ extern void scanner_finish(core_yyscan_t yyscanner);
 extern int	core_yylex(core_YYSTYPE *lvalp, YYLTYPE *llocp,
 					   core_yyscan_t yyscanner);
 extern int	scanner_errposition(int location, core_yyscan_t yyscanner);
-extern void scanner_yyerror(const char *message, core_yyscan_t yyscanner) pg_attribute_noreturn();
+extern pg_noreturn void scanner_yyerror(const char *message, core_yyscan_t yyscanner);
 
 #endif							/* SCANNER_H */
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index fe076d823db..58453faaaec 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -1240,7 +1240,7 @@ extern void pgstat_reset_all(void);
 extern void allow_immediate_pgstat_restart(void);
 
 #ifdef EXEC_BACKEND
-extern void PgstatCollectorMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern pg_noreturn void PgstatCollectorMain(int argc, char *argv[]);
 #endif
 
 
diff --git a/src/include/postmaster/autovacuum.h b/src/include/postmaster/autovacuum.h
index 8451e5d9e27..663ac67d04c 100644
--- a/src/include/postmaster/autovacuum.h
+++ b/src/include/postmaster/autovacuum.h
@@ -65,8 +65,8 @@ extern void AutoVacWorkerFailed(void);
 extern void AutoVacuumUpdateDelay(void);
 
 #ifdef EXEC_BACKEND
-extern void AutoVacLauncherMain(int argc, char *argv[]) pg_attribute_noreturn();
-extern void AutoVacWorkerMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern pg_noreturn void AutoVacLauncherMain(int argc, char *argv[]);
+extern pg_noreturn void AutoVacWorkerMain(int argc, char *argv[]);
 extern void AutovacuumWorkerIAm(void);
 extern void AutovacuumLauncherIAm(void);
 #endif
diff --git a/src/include/postmaster/bgworker_internals.h b/src/include/postmaster/bgworker_internals.h
index 0809e0af538..db1358578cb 100644
--- a/src/include/postmaster/bgworker_internals.h
+++ b/src/include/postmaster/bgworker_internals.h
@@ -54,7 +54,7 @@ extern void BackgroundWorkerStopNotifications(pid_t pid);
 extern void ResetBackgroundWorkerCrashTimes(void);
 
 /* Function to start a background worker, called from postmaster.c */
-extern void StartBackgroundWorker(void) pg_attribute_noreturn();
+extern pg_noreturn void StartBackgroundWorker(void);
 
 #ifdef EXEC_BACKEND
 extern BackgroundWorker *BackgroundWorkerEntry(int slotno);
diff --git a/src/include/postmaster/bgwriter.h b/src/include/postmaster/bgwriter.h
index 630366f49ef..327bc4e784a 100644
--- a/src/include/postmaster/bgwriter.h
+++ b/src/include/postmaster/bgwriter.h
@@ -27,8 +27,8 @@ extern int	CheckPointTimeout;
 extern int	CheckPointWarning;
 extern double CheckPointCompletionTarget;
 
-extern void BackgroundWriterMain(void) pg_attribute_noreturn();
-extern void CheckpointerMain(void) pg_attribute_noreturn();
+extern pg_noreturn void BackgroundWriterMain(void);
+extern pg_noreturn void CheckpointerMain(void);
 
 extern void RequestCheckpoint(int flags);
 extern void CheckpointWriteDelay(int flags, double progress);
diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h
index 2474eac26a5..d6f50650d5b 100644
--- a/src/include/postmaster/pgarch.h
+++ b/src/include/postmaster/pgarch.h
@@ -33,7 +33,7 @@
 extern int	pgarch_start(void);
 
 #ifdef EXEC_BACKEND
-extern void PgArchiverMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern pg_noreturn void PgArchiverMain(int argc, char *argv[]);
 #endif
 
 #endif							/* _PGARCH_H */
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index b692d8be110..7f7ef1c58cf 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -46,7 +46,7 @@ extern int	postmaster_alive_fds[2];
 
 extern PGDLLIMPORT const char *progname;
 
-extern void PostmasterMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern pg_noreturn void PostmasterMain(int argc, char *argv[]);
 extern void ClosePostmasterPorts(bool am_syslogger);
 extern void InitProcessGlobals(void);
 
@@ -56,7 +56,7 @@ extern bool PostmasterMarkPIDForWorkerNotify(int);
 
 #ifdef EXEC_BACKEND
 extern pid_t postmaster_forkexec(int argc, char *argv[]);
-extern void SubPostmasterMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern pg_noreturn void SubPostmasterMain(int argc, char *argv[]);
 
 extern Size ShmemBackendArraySize(void);
 extern void ShmemBackendArrayAllocation(void);
diff --git a/src/include/postmaster/startup.h b/src/include/postmaster/startup.h
index 7b3a089932e..771e1a1126b 100644
--- a/src/include/postmaster/startup.h
+++ b/src/include/postmaster/startup.h
@@ -13,7 +13,7 @@
 #define _STARTUP_H
 
 extern void HandleStartupProcInterrupts(void);
-extern void StartupProcessMain(void) pg_attribute_noreturn();
+extern pg_noreturn void StartupProcessMain(void);
 extern void PreRestoreCommand(void);
 extern void PostRestoreCommand(void);
 extern bool IsPromoteTriggered(void);
diff --git a/src/include/postmaster/syslogger.h b/src/include/postmaster/syslogger.h
index 3a611045736..17e0a22ec7b 100644
--- a/src/include/postmaster/syslogger.h
+++ b/src/include/postmaster/syslogger.h
@@ -84,7 +84,7 @@ extern int	SysLogger_Start(void);
 extern void write_syslogger_file(const char *buffer, int count, int dest);
 
 #ifdef EXEC_BACKEND
-extern void SysLoggerMain(int argc, char *argv[]) pg_attribute_noreturn();
+extern pg_noreturn void SysLoggerMain(int argc, char *argv[]);
 #endif
 
 extern bool CheckLogrotateSignal(void);
diff --git a/src/include/postmaster/walwriter.h b/src/include/postmaster/walwriter.h
index ae267d2411e..df73343ef9d 100644
--- a/src/include/postmaster/walwriter.h
+++ b/src/include/postmaster/walwriter.h
@@ -16,6 +16,6 @@
 extern int	WalWriterDelay;
 extern int	WalWriterFlushAfter;
 
-extern void WalWriterMain(void) pg_attribute_noreturn();
+extern pg_noreturn void WalWriterMain(void);
 
 #endif							/* _WALWRITER_H */
diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index e12a9349660..47a0f5a22ed 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -300,7 +300,7 @@ walrcv_clear_result(WalRcvExecResult *walres)
 }
 
 /* prototypes for functions in walreceiver.c */
-extern void WalReceiverMain(void) pg_attribute_noreturn();
+extern pg_noreturn void WalReceiverMain(void);
 extern void ProcessWalRcvInterrupts(void);
 
 /* prototypes for functions in walreceiverfuncs.c */
diff --git a/src/include/replication/walsender_private.h b/src/include/replication/walsender_private.h
index 0dd6d1cf808..2eeee014933 100644
--- a/src/include/replication/walsender_private.h
+++ b/src/include/replication/walsender_private.h
@@ -120,7 +120,7 @@ extern void WalSndSetState(WalSndState state);
  */
 extern int	replication_yyparse(void);
 extern int	replication_yylex(void);
-extern void replication_yyerror(const char *str) pg_attribute_noreturn();
+extern pg_noreturn void replication_yyerror(const char *str);
 extern void replication_scanner_init(const char *query_string);
 extern void replication_scanner_finish(void);
 
diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h
index d113813df32..cd75c79825b 100644
--- a/src/include/storage/ipc.h
+++ b/src/include/storage/ipc.h
@@ -65,7 +65,7 @@ typedef void (*shmem_startup_hook_type) (void);
 extern PGDLLIMPORT bool proc_exit_inprogress;
 extern PGDLLIMPORT bool shmem_exit_inprogress;
 
-extern void proc_exit(int code) pg_attribute_noreturn();
+extern pg_noreturn void proc_exit(int code);
 extern void shmem_exit(int code);
 extern void on_proc_exit(pg_on_exit_callback function, Datum arg);
 extern void on_shmem_exit(pg_on_exit_callback function, Datum arg);
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 6efb7a96481..4bf0aa43f0e 100644
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -575,7 +575,7 @@ extern void lock_twophase_standby_recover(TransactionId xid, uint16 info,
 
 extern DeadLockState DeadLockCheck(PGPROC *proc);
 extern PGPROC *GetBlockingAutoVacuumPgproc(void);
-extern void DeadLockReport(void) pg_attribute_noreturn();
+extern pg_noreturn void DeadLockReport(void);
 extern void RememberSimpleDeadLock(PGPROC *proc1,
 								   LOCKMODE lockmode,
 								   LOCK *lock,
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index ec21f7e45c5..b04d02b198c 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -61,9 +61,9 @@ extern bool check_max_stack_depth(int *newval, void **extra, GucSource source);
 extern void assign_max_stack_depth(int newval, void *extra);
 
 extern void die(SIGNAL_ARGS);
-extern void quickdie(SIGNAL_ARGS) pg_attribute_noreturn();
+extern pg_noreturn void quickdie(SIGNAL_ARGS);
 extern void StatementCancelHandler(SIGNAL_ARGS);
-extern void FloatExceptionHandler(SIGNAL_ARGS) pg_attribute_noreturn();
+extern pg_noreturn void FloatExceptionHandler(SIGNAL_ARGS);
 extern void RecoveryConflictInterrupt(ProcSignalReason reason); /* called from SIGUSR1
 																 * handler */
 extern void ProcessClientReadInterrupt(bool blocked);
@@ -71,9 +71,9 @@ extern void ProcessClientWriteInterrupt(bool blocked);
 
 extern void process_postgres_switches(int argc, char *argv[],
 									  GucContext ctx, const char **dbname);
-extern void PostgresMain(int argc, char *argv[],
-						 const char *dbname,
-						 const char *username) pg_attribute_noreturn();
+extern pg_noreturn void PostgresMain(int argc, char *argv[],
+									 const char *dbname,
+									 const char *username);
 extern long get_stack_depth_rlimit(void);
 extern void ResetUsage(void);
 extern void ShowUsage(const char *title);
diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h
index 5ebf336071b..eba5f3c9a80 100644
--- a/src/include/utils/datetime.h
+++ b/src/include/utils/datetime.h
@@ -304,8 +304,8 @@ extern int	DecodeInterval(char **field, int *ftype, int nf, int range,
 extern int	DecodeISO8601Interval(char *str,
 								  int *dtype, struct pg_tm *tm, fsec_t *fsec);
 
-extern void DateTimeParseError(int dterr, const char *str,
-							   const char *datatype) pg_attribute_noreturn();
+extern pg_noreturn void DateTimeParseError(int dterr, const char *str,
+										   const char *datatype);
 
 extern int	DetermineTimeZoneOffset(struct pg_tm *tm, pg_tz *tzp);
 extern int	DetermineTimeZoneAbbrevOffset(struct pg_tm *tm, const char *abbr, pg_tz *tzp);
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 9ab4b5470b7..9c3569e2719 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -353,8 +353,8 @@ extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
 	} while (0)
 
 /*
- * Some compilers understand pg_attribute_noreturn(); for other compilers,
- * insert pg_unreachable() so that the compiler gets the point.
+ * Some compilers understand pg_noreturn for other compilers, insert
+ * pg_unreachable() so that the compiler gets the point.
  */
 #ifdef HAVE_PG_ATTRIBUTE_NORETURN
 #define PG_RE_THROW()  \
@@ -414,9 +414,9 @@ extern void EmitErrorReport(void);
 extern ErrorData *CopyErrorData(void);
 extern void FreeErrorData(ErrorData *edata);
 extern void FlushErrorState(void);
-extern void ReThrowError(ErrorData *edata) pg_attribute_noreturn();
+extern pg_noreturn void ReThrowError(ErrorData *edata);
 extern void ThrowErrorData(ErrorData *edata);
-extern void pg_re_throw(void) pg_attribute_noreturn();
+extern pg_noreturn void pg_re_throw(void);
 
 extern char *GetErrorContextStack(void);
 
diff --git a/src/include/utils/help_config.h b/src/include/utils/help_config.h
index f0eace59f50..7858973968f 100644
--- a/src/include/utils/help_config.h
+++ b/src/include/utils/help_config.h
@@ -12,6 +12,6 @@
 #ifndef HELP_CONFIG_H
 #define HELP_CONFIG_H 1
 
-extern void GucInfoMain(void) pg_attribute_noreturn();
+extern pg_noreturn void GucInfoMain(void);
 
 #endif
diff --git a/src/interfaces/ecpg/preproc/preproc_extern.h b/src/interfaces/ecpg/preproc/preproc_extern.h
index 323fd5c3c1a..2d83e3fefb9 100644
--- a/src/interfaces/ecpg/preproc/preproc_extern.h
+++ b/src/interfaces/ecpg/preproc/preproc_extern.h
@@ -76,7 +76,7 @@ extern void base_yyerror(const char *);
 extern void *mm_alloc(size_t);
 extern char *mm_strdup(const char *);
 extern void mmerror(int errorcode, enum errortype type, const char *error,...) pg_attribute_printf(3, 4);
-extern void mmfatal(int errorcode, const char *error,...) pg_attribute_printf(2, 3) pg_attribute_noreturn();
+extern pg_noreturn void mmfatal(int errorcode, const char *error,...) pg_attribute_printf(2, 3);
 extern void output_get_descr_header(char *);
 extern void output_get_descr(char *, char *);
 extern void output_set_descr_header(char *);
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index f66b2baa70a..52e53c796e9 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1318,7 +1318,7 @@ extern int	plpgsql_peek(void);
 extern void plpgsql_peek2(int *tok1_p, int *tok2_p, int *tok1_loc,
 						  int *tok2_loc);
 extern int	plpgsql_scanner_errposition(int location);
-extern void plpgsql_yyerror(const char *message) pg_attribute_noreturn();
+extern pg_noreturn void plpgsql_yyerror(const char *message);
 extern int	plpgsql_location_to_lineno(int location);
 extern int	plpgsql_latest_lineno(void);
 extern void plpgsql_scanner_init(const char *str);
diff --git a/src/test/modules/test_shm_mq/test_shm_mq.h b/src/test/modules/test_shm_mq/test_shm_mq.h
index 4c0722297e9..3ff8b5f17d8 100644
--- a/src/test/modules/test_shm_mq/test_shm_mq.h
+++ b/src/test/modules/test_shm_mq/test_shm_mq.h
@@ -40,6 +40,6 @@ extern void test_shm_mq_setup(int64 queue_size, int32 nworkers,
 							  shm_mq_handle **input);
 
 /* Main entrypoint for a worker. */
-extern void test_shm_mq_main(Datum) pg_attribute_noreturn();
+extern pg_noreturn void test_shm_mq_main(Datum);
 
 #endif
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index bc9ef64a508..35583a70e72 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -46,7 +46,7 @@ PG_MODULE_MAGIC;
 PG_FUNCTION_INFO_V1(worker_spi_launch);
 
 void		_PG_init(void);
-void		worker_spi_main(Datum) pg_attribute_noreturn();
+pg_noreturn void worker_spi_main(Datum);
 
 /* flags set by signal handlers */
 static volatile sig_atomic_t got_sighup = false;
diff --git a/src/timezone/zic.c b/src/timezone/zic.c
index c27fb456d05..31f317d9cac 100644
--- a/src/timezone/zic.c
+++ b/src/timezone/zic.c
@@ -118,11 +118,11 @@ struct zone
 
 extern int	link(const char *fromname, const char *toname);
 
-static void memory_exhausted(const char *msg) pg_attribute_noreturn();
+static pg_noreturn void memory_exhausted(const char *msg);
 static void verror(const char *string, va_list args) pg_attribute_printf(1, 0);
 static void error(const char *string,...) pg_attribute_printf(1, 2);
 static void warning(const char *string,...) pg_attribute_printf(1, 2);
-static void usage(FILE *stream, int status) pg_attribute_noreturn();
+static pg_noreturn void usage(FILE *stream, int status);
 static void addtt(zic_t starttime, int type);
 static int	addtype(zic_t, char const *, bool, bool, bool);
 static void leapadd(zic_t, bool, int, int);
-- 
2.23.0.385.gbc12974a89


--k5olgh2lgulcruiy--





^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* [PATCH 06/18] One fewer (not one less)
@ 2021-01-18 20:37 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Justin Pryzby @ 2021-01-18 20:37 UTC (permalink / raw)

---
 contrib/pageinspect/heapfuncs.c            | 2 +-
 doc/src/sgml/charset.sgml                  | 2 +-
 doc/src/sgml/ref/create_type.sgml          | 2 +-
 doc/src/sgml/rules.sgml                    | 2 +-
 src/backend/access/common/heaptuple.c      | 4 ++--
 src/backend/commands/analyze.c             | 2 +-
 src/backend/utils/adt/jsonpath_exec.c      | 2 +-
 src/backend/utils/adt/selfuncs.c           | 2 +-
 src/backend/utils/cache/catcache.c         | 2 +-
 src/backend/utils/misc/sampling.c          | 2 +-
 src/backend/utils/mmgr/freepage.c          | 2 +-
 src/bin/pgbench/pgbench.c                  | 2 +-
 src/include/pg_config_manual.h             | 4 ++--
 src/interfaces/ecpg/include/sqlda-native.h | 2 +-
 src/test/regress/expected/geometry.out     | 2 +-
 src/test/regress/sql/geometry.sql          | 2 +-
 16 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 9abcee32af..f6760eb31e 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -338,7 +338,7 @@ tuple_data_split_internal(Oid relid, char *tupdata,
 		attr = TupleDescAttr(tupdesc, i);
 
 		/*
-		 * Tuple header can specify less attributes than tuple descriptor as
+		 * Tuple header can specify fewer attributes than tuple descriptor as
 		 * ALTER TABLE ADD COLUMN without DEFAULT keyword does not actually
 		 * change tuples in pages, so attributes with numbers greater than
 		 * (t_infomask2 & HEAP_NATTS_MASK) should be treated as NULL.
diff --git a/doc/src/sgml/charset.sgml b/doc/src/sgml/charset.sgml
index cebc09ef91..1b00e543a6 100644
--- a/doc/src/sgml/charset.sgml
+++ b/doc/src/sgml/charset.sgml
@@ -619,7 +619,7 @@ SELECT * FROM test1 ORDER BY a || b COLLATE "fr_FR";
     name such as <literal>de_DE</literal> can be considered unique
     within a given database even though it would not be unique globally.
     Use of the stripped collation names is recommended, since it will
-    make one less thing you need to change if you decide to change to
+    make one fewer thing you need to change if you decide to change to
     another database encoding.  Note however that the <literal>default</literal>,
     <literal>C</literal>, and <literal>POSIX</literal> collations can be used regardless of
     the database encoding.
diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml
index 0b24a55505..693423e524 100644
--- a/doc/src/sgml/ref/create_type.sgml
+++ b/doc/src/sgml/ref/create_type.sgml
@@ -867,7 +867,7 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
    Before <productname>PostgreSQL</productname> version 8.3, the name of
    a generated array type was always exactly the element type's name with one
    underscore character (<literal>_</literal>) prepended.  (Type names were
-   therefore restricted in length to one less character than other names.)
+   therefore restricted in length to one fewer character than other names.)
    While this is still usually the case, the array type name may vary from
    this in case of maximum-length names or collisions with user type names
    that begin with underscore.  Writing code that depends on this convention
diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
index e81addcfa9..aa172d102b 100644
--- a/doc/src/sgml/rules.sgml
+++ b/doc/src/sgml/rules.sgml
@@ -1266,7 +1266,7 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
 <para>
     The query trees generated from rule actions are thrown into the
     rewrite system again, and maybe more rules get applied resulting
-    in more or less query trees.
+    in additional or fewer query trees.
     So a rule's actions must have either a different
     command type or a different result relation than the rule itself is
     on, otherwise this recursive process will end up in an infinite loop.
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 24a27e387d..0b56b0fa5a 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -719,11 +719,11 @@ heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest)
 }
 
 /*
- * Expand a tuple which has less attributes than required. For each attribute
+ * Expand a tuple which has fewer attributes than required. For each attribute
  * not present in the sourceTuple, if there is a missing value that will be
  * used. Otherwise the attribute will be set to NULL.
  *
- * The source tuple must have less attributes than the required number.
+ * The source tuple must have fewer attributes than the required number.
  *
  * Only one of targetHeapTuple and targetMinimalTuple may be supplied. The
  * other argument must be NULL.
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 7295cf0215..64908ac39c 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1003,7 +1003,7 @@ examine_attribute(Relation onerel, int attnum, Node *index_expr)
  * As of May 2004 we use a new two-stage method:  Stage one selects up
  * to targrows random blocks (or all blocks, if there aren't so many).
  * Stage two scans these blocks and uses the Vitter algorithm to create
- * a random sample of targrows rows (or less, if there are less in the
+ * a random sample of targrows rows (or fewer, if there are fewer in the
  * sample of blocks).  The two stages are executed simultaneously: each
  * block is processed as soon as stage one returns its number and while
  * the rows are read stage two controls which ones are to be inserted
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 4d185c27b4..078aaef539 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -263,7 +263,7 @@ static int	compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
  *		implement @? and @@ operators, which in turn are intended to have an
  *		index support.  Thus, it's desirable to make it easier to achieve
  *		consistency between index scan results and sequential scan results.
- *		So, we throw as less errors as possible.  Regarding this function,
+ *		So, we throw as few errors as possible.  Regarding this function,
  *		such behavior also matches behavior of JSON_EXISTS() clause of
  *		SQL/JSON.  Regarding jsonb_path_match(), this function doesn't have
  *		an analogy in SQL/JSON, so we define its behavior on our own.
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 47ca4ddbb5..52314d3aa1 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -645,7 +645,7 @@ scalarineqsel(PlannerInfo *root, Oid operator, bool isgt, bool iseq,
 
 			/*
 			 * The calculation so far gave us a selectivity for the "<=" case.
-			 * We'll have one less tuple for "<" and one additional tuple for
+			 * We'll have one fewer tuple for "<" and one additional tuple for
 			 * ">=", the latter of which we'll reverse the selectivity for
 			 * below, so we can simply subtract one tuple for both cases.  The
 			 * cases that need this adjustment can be identified by iseq being
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index fa2b49c676..55c9445898 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -1497,7 +1497,7 @@ GetCatCacheHashValue(CatCache *cache,
  *		It doesn't make any sense to specify all of the cache's key columns
  *		here: since the key is unique, there could be at most one match, so
  *		you ought to use SearchCatCache() instead.  Hence this function takes
- *		one less Datum argument than SearchCatCache() does.
+ *		one fewer Datum argument than SearchCatCache() does.
  *
  *		The caller must not modify the list object or the pointed-to tuples,
  *		and must call ReleaseCatCacheList() when done with the list.
diff --git a/src/backend/utils/misc/sampling.c b/src/backend/utils/misc/sampling.c
index 0c327e823f..7348b86682 100644
--- a/src/backend/utils/misc/sampling.c
+++ b/src/backend/utils/misc/sampling.c
@@ -42,7 +42,7 @@ BlockSampler_Init(BlockSampler bs, BlockNumber nblocks, int samplesize,
 	bs->N = nblocks;			/* measured table size */
 
 	/*
-	 * If we decide to reduce samplesize for tables that have less or not much
+	 * If we decide to reduce samplesize for tables that have fewer or not much
 	 * more than samplesize blocks, here is the place to do it.
 	 */
 	bs->n = samplesize;
diff --git a/src/backend/utils/mmgr/freepage.c b/src/backend/utils/mmgr/freepage.c
index e4ee1aab97..10a1effb74 100644
--- a/src/backend/utils/mmgr/freepage.c
+++ b/src/backend/utils/mmgr/freepage.c
@@ -495,7 +495,7 @@ FreePageManagerDump(FreePageManager *fpm)
  * if we search the parent page for the first key greater than or equal to
  * the first key on the current page, the downlink to this page will be either
  * the exact index returned by the search (if the first key decreased)
- * or one less (if the first key increased).
+ * or one fewer (if the first key increased).
  */
 static void
 FreePageBtreeAdjustAncestorKeys(FreePageManager *fpm, FreePageBtree *btp)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index a4a3f40048..627a244fb7 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -6458,7 +6458,7 @@ threadRun(void *arg)
 
 			/*
 			 * If advanceConnectionState changed client to finished state,
-			 * that's one less client that remains.
+			 * that's one fewer client that remains.
 			 */
 			if (st->state == CSTATE_FINISHED || st->state == CSTATE_ABORTED)
 				remains--;
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index d27c8601fa..e3d2e751ea 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -21,7 +21,7 @@
 
 /*
  * Maximum length for identifiers (e.g. table names, column names,
- * function names).  Names actually are limited to one less byte than this,
+ * function names).  Names actually are limited to one fewer byte than this,
  * because the length must include a trailing zero byte.
  *
  * Changing this requires an initdb.
@@ -87,7 +87,7 @@
 
 /*
  * MAXPGPATH: standard size of a pathname buffer in PostgreSQL (hence,
- * maximum usable pathname length is one less).
+ * maximum usable pathname length is one fewer).
  *
  * We'd use a standard system header symbol for this, if there weren't
  * so many to choose from: MAXPATHLEN, MAX_PATH, PATH_MAX are all
diff --git a/src/interfaces/ecpg/include/sqlda-native.h b/src/interfaces/ecpg/include/sqlda-native.h
index 67d3c7b4e4..9e73f1f1b1 100644
--- a/src/interfaces/ecpg/include/sqlda-native.h
+++ b/src/interfaces/ecpg/include/sqlda-native.h
@@ -7,7 +7,7 @@
 
 /*
  * Maximum length for identifiers (e.g. table names, column names,
- * function names).  Names actually are limited to one less byte than this,
+ * function names).  Names actually are limited to one fewer byte than this,
  * because the length must include a trailing zero byte.
  *
  * This should be at least as much as NAMEDATALEN of the database the
diff --git a/src/test/regress/expected/geometry.out b/src/test/regress/expected/geometry.out
index 84f7eabb66..9799cfbdbd 100644
--- a/src/test/regress/expected/geometry.out
+++ b/src/test/regress/expected/geometry.out
@@ -4325,7 +4325,7 @@ SELECT f1, polygon(8, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
  <(100,1),115>  | ((-15,1),(18.6827201635,82.3172798365),(100,116),(181.317279836,82.3172798365),(215,1),(181.317279836,-80.3172798365),(100,-114),(18.6827201635,-80.3172798365))
 (6 rows)
 
--- Too less points error
+-- Too few points error
 SELECT f1, polygon(1, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 ERROR:  must request at least 2 points
 -- Zero radius error
diff --git a/src/test/regress/sql/geometry.sql b/src/test/regress/sql/geometry.sql
index 96df0ab05a..b0ab6d03ec 100644
--- a/src/test/regress/sql/geometry.sql
+++ b/src/test/regress/sql/geometry.sql
@@ -424,7 +424,7 @@ SELECT f1, f1::polygon FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 -- To polygon with less points
 SELECT f1, polygon(8, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 
--- Too less points error
+-- Too few points error
 SELECT f1, polygon(1, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 
 -- Zero radius error
-- 
2.17.0


--lc9FT7cWel8HagAv
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0007-function-comment-get_am_name.patch"



^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
@ 2025-03-11 16:58 Bernd Helmle <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Bernd Helmle @ 2025-03-11 16:58 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; Japin Li <[email protected]>; +Cc: pgsql-hackers

Hi,

> I definitely like that passlib have documented their thought process
> thoroughly.
> 

Please find attached v4 of this patch. I added the following changes:

- Check for non-supported characters in the salt like passlib does.
- Check for reserved tokens when parsing the salt string (i find this
very strict, but it covers the cases Japin Li pointed out).

I didn't implement being more strict when it comes to the salt length:
the salt might be provided from external sources and the password hash
is just used for validating within the database.

When hashing the password within postgres, users always should use
gen_salt(), which generates a reasonable salt string. 

Maybe, in case of empty salts, we should issue a WARNING instead of
erroring out and put additional documentation on how to use it right.

> I think using their strict mode is good on principle, but if we're
> going
> to do that, then the salt string should not be used verbatim, but
> instead base64-decoded first to get the actual salt bytes, like they
> do.
> Does this break compabitibility with other systems?  Are
> passlib-generated password hashes incompatible with, say, "openssl
> passwd" which you (Bernd) mentioned at the beginning of the thread?
> Maybe if the password hashes are no longer compatible, then we should
> ditch the idea of restricting salts to base64 chars and accept the
> whole
> range of bytes, like Drepper.
> 
> But in any case ISTM we should reject, as they suggest, the use of
> less
> than 4 bytes of salt (and should perhaps settle for a default of 16,
> as
> passlib suggests).  I suppose this is why passlib returns NULL with
> empty salt.  What we should do in that case IMO is ereport(ERROR). 
> 


Hmm, i didn't understand that passlib does decode them first, i thought
they use it encoded... at least, in our current form we're pretty much
compatible with Drepper, passlib and OpenSSL, as far as i tested:

Empty salt
----------

# Drepper reference code

shacrypt password "" sha512crypt
$6$$bLTg4cpho8PIUrjfsE7qlU08Qx2UEfw..xOc6I1wpGVtyVYToGrr7BzRdAAnEr5lYFr
1Z9WcCf1xNZ1HG9qFW1

# Patch

SELECT crypt('password', '$6$$');
                                           crypt                      
───────────────────────────────────────────────────────────────────────
─────────────────────
$6$$bLTg4cpho8PIUrjfsE7qlU08Qx2UEfw..xOc6I1wpGVtyVYToGrr7BzRdAAnEr5lYFr1
Z9WcCf1xNZ1HG9qFW1
(1 row)

# Passlib

from passlib.hash import sha256_crypt,sha512_crypt
hash = sha512_crypt.using(salt='').using(rounds=5000).hash("password")
print(hash)
$6$$bLTg4cpho8PIUrjfsE7qlU08Qx2UEfw..xOc6I1wpGVtyVYToGrr7BzRdAAnEr5lYFr
1Z9WcCf1xNZ1HG9qFW1

# OpenSSL

echo password | openssl passwd -6 -stdin -salt ''             
<NULL>

Short salt
----------

# Drepper reference code

./shacrypt password abcdef sha512crypt
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH
1SXexabbdNXXOO11F7sdyurk/

# Patch

SELECT crypt('password', '$6$abcdef$');
                                              crypt                   
───────────────────────────────────────────────────────────────────────
───────────────────────────
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH1
SXexabbdNXXOO11F7sdyurk/
(1 row)

# Passlib

from passlib.hash import sha256_crypt,sha512_crypt
hash =
sha512_crypt.using(salt='abcdef').using(rounds=5000).hash("password")
print(hash)
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH
1SXexabbdNXXOO11F7sdyurk/

# OpenSSL

echo password | openssl passwd -6 -stdin -salt 'abcdef'
$6$abcdef$2yYFwtar3.rlLovG0bXbxXMjZe7Z/1EhQ0gEs0nXalsCW04p2iy6unkvbrBFH
1SXexabbdNXXOO11F7sdyurk/

Salt > MAXLEN(16 Bytes)
-----------------------

# Drepper reference code
(truncates salt)

shacrypt password abcdefghijklmnopqrstuvwxyz sha512crypt
$6$abcdefghijklmnop$0aenUFHf897F9u0tURIHOeACWajSuVGa7jgJGyq.DKZm/WXl/IZ
FvPbneFydBjomEOgM.Sh1m0L3KsS1.H5b//

# Patch
(truncates salt)

SELECT crypt('password', '$6$abcdefghijklmnopqrstuvwxyz$');
                                                   crypt              
───────────────────────────────────────────────────────────────────────
─────────────────────────────────────
$6$abcdefghijklmnop$0aenUFHf897F9u0tURIHOeACWajSuVGa7jgJGyq.DKZm/WXl/IZF
vPbneFydBjomEOgM.Sh1m0L3KsS1.H5b//
(1 row)

# Passlib

Errors out with

ValueError: salt too large (sha512_crypt requires <= 16 chars)

# OpenSSL
(truncates salt)

echo password | openssl passwd -6 -stdin -salt
'abcdefghijklmnopqrstuvwxyz'
$6$abcdefghijklmnop$0aenUFHf897F9u0tURIHOeACWajSuVGa7jgJGyq.DKZm/WXl/IZ
FvPbneFydBjomEOgM.Sh1m0L3KsS1.H5b//

Salt with "invalid" character
-----------------------------

# Drepper reference code

./shacrypt password abcdefghi%jklmno sha512crypt
$6$abcdefghi%jklmno$LMN/V1pW97IoK0rWSDVqCo9EYd6zpqP0TdTX9.cxFAFqsdSMWQM
jehkmMtDzL36VBKeG6dg.kFAQKoFvZpK0G.

# Patch
current version V4 (attached)

Errors out:

SELECT crypt('password', '$6$abcdefghi%jklmno$');
FEHLER:  invalid character in salt string: "%"
Time: 0,217 ms

# Passlib

Errors out

ValueError: invalid characters in sha512_crypt salt

# OpenSSL

echo password | openssl passwd -6 -stdin -salt 'abcdefghi%jklmno'     
$6$abcdefghi%jklmno$LMN/V1pW97IoK0rWSDVqCo9EYd6zpqP0TdTX9.cxFAFqsdSMWQM
jehkmMtDzL36VBKeG6dg.kFAQKoFvZpK0G.


So the hashes we produce are identical, but with being more strict we
differ in the handling of the provided salt.

	Bernd




Attachments:

  [text/x-patch] 0001-Add-modern-SHA-2-based-password-hashes-to-pgcrypto-v4.patch (43.3K, ../../[email protected]/2-0001-Add-modern-SHA-2-based-password-hashes-to-pgcrypto-v4.patch)
  download | inline diff:
From dccaa0c9cea257edb7350d0ff3ab7f1752383278 Mon Sep 17 00:00:00 2001
From: Bernd Helmle <Bernd Helmle [email protected]>
Date: Tue, 11 Mar 2025 17:31:39 +0100
Subject: [PATCH] Add modern SHA-2 based password hashes to pgcrypto.

This adapts the publicly available reference implementation on
https://www.akkadia.org/drepper/SHA-crypt.txt and adds the new
hash algorithms sha256crypt and sha512crypt to crypt() and gen_salt()
respectively.
---
 contrib/pgcrypto/Makefile                    |   3 +-
 contrib/pgcrypto/crypt-gensalt.c             |  84 +++
 contrib/pgcrypto/crypt-sha.c                 | 694 +++++++++++++++++++
 contrib/pgcrypto/expected/crypt-shacrypt.out | 196 ++++++
 contrib/pgcrypto/meson.build                 |   2 +
 contrib/pgcrypto/px-crypt.c                  |  16 +
 contrib/pgcrypto/px-crypt.h                  |  31 +
 contrib/pgcrypto/sql/crypt-shacrypt.sql      |  99 +++
 doc/src/sgml/pgcrypto.sgml                   |  41 +-
 9 files changed, 1164 insertions(+), 2 deletions(-)
 create mode 100644 contrib/pgcrypto/crypt-sha.c
 create mode 100644 contrib/pgcrypto/expected/crypt-shacrypt.out
 create mode 100644 contrib/pgcrypto/sql/crypt-shacrypt.sql

diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index 11c4455dd05..69afa375011 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -11,6 +11,7 @@ OBJS = \
 	crypt-des.o \
 	crypt-gensalt.o \
 	crypt-md5.o \
+	crypt-sha.o \
 	mbuf.o \
 	openssl.o \
 	pgcrypto.o \
@@ -43,7 +44,7 @@ REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
 	sha2 des 3des cast5 \
 	crypt-des crypt-md5 crypt-blowfish crypt-xdes \
 	pgp-armor pgp-decrypt pgp-encrypt pgp-encrypt-md5 $(CF_PGP_TESTS) \
-	pgp-pubkey-decrypt pgp-pubkey-encrypt pgp-info
+	pgp-pubkey-decrypt pgp-pubkey-encrypt pgp-info crypt-shacrypt
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
diff --git a/contrib/pgcrypto/crypt-gensalt.c b/contrib/pgcrypto/crypt-gensalt.c
index 740f3612532..b9aded897d1 100644
--- a/contrib/pgcrypto/crypt-gensalt.c
+++ b/contrib/pgcrypto/crypt-gensalt.c
@@ -185,3 +185,87 @@ _crypt_gensalt_blowfish_rn(unsigned long count,
 
 	return output;
 }
+
+static char *
+_crypt_gensalt_sha(unsigned long count,
+				   const char *input, int size, char *output, int output_size)
+{
+	char * s_ptr = output;
+	unsigned int result_bufsize = PX_SHACRYPT_SALT_BUF_LEN;
+	int rc;
+
+	/* output buffer must be allocated with PX_MAX_SALT_LEN bytes */
+	if (PX_MAX_SALT_LEN < result_bufsize)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("invalid size of salt")));
+	}
+
+	/*
+	 * Care must be taken to not exceed the buffer size allocated for
+	 * the input character buffer.
+	 */
+	if ((PX_SHACRYPT_SALT_MAX_LEN != size)
+		|| (output_size < size))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INTERNAL_ERROR),
+				 errmsg("invalid length of salt buffer")));
+	}
+
+	/* Skip magic bytes, set by callers */
+	s_ptr += 3;
+	if ((rc = pg_snprintf(s_ptr, 18, "rounds=%ld$", count)) <= 0)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INTERNAL_ERROR),
+				 errmsg("cannot format salt string")));
+	}
+
+	/* s_ptr should now be positioned at the start of the salt string */
+	s_ptr += rc;
+
+	/*
+	 * Normalize salt string
+	 *
+	 * size of input buffer was checked above to
+	 * not exceed PX_SHACRYPT_SALT_LEN_MAX.
+	 */
+	for (int i = 0; i < size; i++)
+	{
+		*s_ptr = _crypt_itoa64[input[i] & 0x3f];
+		s_ptr++;
+	}
+
+	/* We're done */
+	return output;
+}
+
+char *
+_crypt_gensalt_sha512_rn(unsigned long count,
+						 char const *input, int size,
+						 char *output, int output_size)
+{
+	memset(output, 0, output_size);
+	/* set magic byte for sha512crypt */
+	output[0] = '$';
+	output[1] = '6';
+	output[2] = '$';
+
+	return _crypt_gensalt_sha(count, input, size, output, output_size);
+}
+
+char *
+_crypt_gensalt_sha256_rn(unsigned long count,
+						 const char *input, int size,
+						 char *output, int output_size)
+{
+	memset(output, 0, output_size);
+	/* set magic byte for sha256crypt */
+	output[0] = '$';
+	output[1] = '5';
+	output[2] = '$';
+
+	return _crypt_gensalt_sha(count, input, size, output, output_size);
+}
\ No newline at end of file
diff --git a/contrib/pgcrypto/crypt-sha.c b/contrib/pgcrypto/crypt-sha.c
new file mode 100644
index 00000000000..ec51e865a5e
--- /dev/null
+++ b/contrib/pgcrypto/crypt-sha.c
@@ -0,0 +1,694 @@
+/*
+ * contrib/pgcrypto/crypt-sha.c
+ *
+ * This implements shacrypt password hash functions and follows the
+ * public available reference implementation from
+ *
+ * https://www.akkadia.org/drepper/SHA-crypt.txt
+ *
+ * This code is public domain.
+ *
+ * Please see the inline comments for details about the algorithm.
+ *
+ * Basically the following code implements password hashing with sha256 and
+ * sha512 digest via OpenSSL. Additionally, an extended salt generation (see
+ * crypt-gensalt.c for details) is provided, which generates a salt suitable
+ * for either sha256crypt and sha512crypt password hash generation.
+ *
+ * Official identifiers for suitable password hashes used in salts are
+ * 5 : sha256crypt and
+ * 6 : sha512crypt
+ *
+ * The hashing code below supports and uses salt length up to 16 bytes. Longer
+ * input is possible, but any additional byte of the input is disregarded.
+ * gen_salt(), when called with a sha256crypt or sha512crypt identifier will
+ * always generate a 16 byte long salt string.
+ *
+ * Output is compatible with any sha256crypt and sha512crypt output
+ * generated by e.g. OpenSSL or libc crypt().
+ *
+ * The described algorithm uses default computing rounds of 5000. Currently,
+ * even when no specific rounds specification is used, we always explicitly
+ * print out the rounds option flag with the final hash password string.
+ *
+ * The length of the specific password hash (without magic bytes and salt
+ * string) is:
+ *
+ * sha256crypt: 43 bytes and
+ * sha512crypt: 86 bytes.
+ *
+ * Overall hashed password length is:
+ *
+ * sha256crypt: 80 bytes and
+ * sha512crypt: 123 bytes
+ *
+ */
+#include "postgres.h"
+#include "miscadmin.h"
+
+#include "px-crypt.h"
+#include "px.h"
+
+typedef enum
+{
+	PGCRYPTO_SHA256CRYPT = 0,
+	PGCRYPTO_SHA512CRYPT = 1,
+	PGCRYPTO_SHA_UNKOWN
+}			PGCRYPTO_SHA_t;
+
+static unsigned char _crypt_itoa64[64 + 1] =
+"./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
+
+/*
+ * Modern UNIX password, based on SHA crypt hashes
+ */
+char *
+px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstlen)
+{
+	static const char rounds_prefix[] = "rounds=";
+	static char *magic_bytes[2] = {"$5$", "$6$"};
+
+	/* Used to create the password hash string */
+	StringInfo	out_buf = NULL;
+
+	PGCRYPTO_SHA_t type = PGCRYPTO_SHA_UNKOWN;
+	PX_MD	   *digestA = NULL;
+	PX_MD	   *digestB = NULL;
+	int			err;
+
+	const char *dec_salt_binary;	/* pointer into the real salt string */
+	StringInfo decoded_salt = NULL; /* decoded salt string */
+
+	unsigned char sha_buf[PX_SHACRYPT_DIGEST_MAX_LEN];
+	unsigned char sha_buf_tmp[PX_SHACRYPT_DIGEST_MAX_LEN];	/* temporary buffer for
+																 * digests */
+
+	char		rounds_custom = 0;
+	char	   *p_bytes = NULL;
+	char	   *s_bytes = NULL;
+	char	   *cp = NULL;
+	const char *fp = NULL;				/* intermediate pointer within salt string */
+	const char *ep = NULL;				/* holds pointer to the end of the salt string */
+
+	size_t		buf_size = 0;	/* buffer size for sha256crypt/sha512crypt */
+	unsigned int block;			/* number of bytes processed */
+	unsigned long rounds = PX_SHACRYPT_ROUNDS_DEFAULT;
+
+	unsigned	len, salt_len = 0;
+
+	/* Init result buffer */
+	out_buf = makeStringInfoExt(PX_SHACRYPT_BUF_LEN);
+	decoded_salt = makeStringInfoExt(PX_SHACRYPT_SALT_MAX_LEN);
+
+	/* Sanity checks */
+	if (!passwd)
+		return NULL;
+
+	if (pw == NULL)
+	{
+		elog(ERROR, "null value for password rejected");
+	}
+
+	if (salt == NULL)
+	{
+		elog(ERROR, "null value for salt rejected");
+	}
+
+	/*
+	 * Make sure result buffers are large enough.
+	 */
+	if (dstlen < PX_SHACRYPT_BUF_LEN)
+	{
+		elog(ERROR, "insufficient result buffer size to encrypt password");
+	}
+
+	/* Init contents of buffers properly */
+	memset(&sha_buf, '\0', sizeof(sha_buf));
+	memset(&sha_buf_tmp, '\0', sizeof(sha_buf_tmp));
+
+	/*
+	 * Decode the salt string. We need to know how many rounds and which
+	 * digest we have to use to hash the password.
+	 */
+	len = strlen(pw);
+	dec_salt_binary = salt;
+
+	/*
+	 * Analyze and prepare the salt string
+	 *
+	 * The magic string should be specified in the first three bytes of the
+	 * salt string. But do some sanity checks before.
+	 */
+	if (strlen(dec_salt_binary) < 3)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("invalid salt")));
+	}
+
+	/*
+	 * Check format of magic bytes. These should define either 5=sha256crypt
+	 * or 6=sha512crypt in the second byte, enclosed by ascii dollar signs.
+	 */
+	if ((dec_salt_binary[0] != '$')
+		&& (dec_salt_binary[2] != '$'))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("invalid format of salt")),
+				errhint("magic byte format for shacrypt is either \"$5$\" or \"$6$\""));
+	}
+
+	/*
+	 * Check magic byte for supported shacrypt digest.
+	 *
+	 * We're just interested in the very first 3 bytes of the salt string,
+	 * since this defines the digest length to use.
+	 */
+	if (strncmp(dec_salt_binary, magic_bytes[0], strlen(magic_bytes[0])) == 0)
+	{
+		type = PGCRYPTO_SHA256CRYPT;
+		dec_salt_binary += strlen(magic_bytes[0]);
+	}
+	else if (strncmp(dec_salt_binary, magic_bytes[1], strlen(magic_bytes[1])) == 0)
+	{
+		type = PGCRYPTO_SHA512CRYPT;
+		dec_salt_binary += strlen(magic_bytes[1]);
+	}
+
+	/*
+	 * dec_salt_binary pointer is positioned after the magic bytes now
+	 *
+	 * We extract any options in the following code branch. The only optional
+	 * setting we need to take care of is the "rounds" option. Note that the
+	 * salt generator already checked for invalid settings before, but we need
+	 * to do it here again to protect against injection of wrong values when
+	 * called without the generator.
+	 *
+	 * If there is any garbage added after the magic byte and the options/salt
+	 * string, we don't treat this special: This is just absorbed as part of
+	 * the salt with up to PX_SHACRYPT_SALT_LEN_MAX.
+	 *
+	 * Unknown magic byte is handled later below
+	 */
+	if (strncmp(dec_salt_binary,
+				rounds_prefix, sizeof(rounds_prefix) - 1) == 0)
+	{
+
+		const char *num = dec_salt_binary + sizeof(rounds_prefix) - 1;
+		char	   *endp;
+		long		srounds = strtoul(num, &endp, 10);
+
+		if (*endp == '$')
+		{
+			dec_salt_binary = endp + 1;
+
+			/*
+			 * We violate supported lower or upper bound of rounds, but in
+			 * this case we change this value to the supported lower or upper
+			 * value. We don't do this silently and print a NOTICE in such a
+			 * case.
+			 *
+			 * Note that a salt string generated with gen_salt() would never
+			 * generated such a salt string, since it would error out.
+			 *
+			 * But Drepper's upstream reference implementation supports this
+			 * when passing the salt string directly, so we maintain
+			 * compatibility here.
+			 */
+			if (srounds > PX_SHACRYPT_ROUNDS_MAX)
+			{
+				ereport(NOTICE,
+						errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+						errmsg("rounds=%ld exceeds maximum supported value (%ld), using %ld instead",
+							   srounds, PX_SHACRYPT_ROUNDS_MAX,
+							   PX_SHACRYPT_ROUNDS_MAX));
+				srounds = PX_SHACRYPT_ROUNDS_MAX;
+			}
+			else if (srounds < PX_SHACRYPT_ROUNDS_MIN)
+			{
+				ereport(NOTICE,
+						errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+						errmsg("rounds=%ld is below supported value (%ld), using %ld instead",
+							   srounds, PX_SHACRYPT_ROUNDS_MIN,
+							   PX_SHACRYPT_ROUNDS_MIN));
+				srounds = PX_SHACRYPT_ROUNDS_MIN;
+			}
+
+			rounds = (unsigned long) srounds;
+			rounds_custom = 1;
+		}
+		else
+		{
+			ereport(ERROR,
+					errcode(ERRCODE_SYNTAX_ERROR),
+					errmsg("could not parse salt options"));
+		}
+
+	}
+
+	/*
+	 * Choose the correct digest length and add the magic bytes to the result
+	 * buffer. Also handle possible invalid magic byte we've extracted above.
+	 */
+	switch (type)
+	{
+		case PGCRYPTO_SHA256CRYPT:
+			{
+				/* Two PX_MD objects required */
+				err = px_find_digest("sha256", &digestA);
+				if (err)
+					goto error;
+
+				err = px_find_digest("sha256", &digestB);
+				if (err)
+					goto error;
+
+				/* digest buffer length is 32 for sha256 */
+				buf_size = 32;
+
+				appendStringInfoString(out_buf, magic_bytes[0]);
+				break;
+			}
+
+		case PGCRYPTO_SHA512CRYPT:
+			{
+				/* Two PX_MD objects required */
+				err = px_find_digest("sha512", &digestA);
+				if (err)
+					goto error;
+
+				err = px_find_digest("sha512", &digestB);
+				if (err)
+					goto error;
+
+				buf_size = PX_SHACRYPT_DIGEST_MAX_LEN;
+
+				appendStringInfoString(out_buf, magic_bytes[1]);
+				break;
+			}
+
+		case PGCRYPTO_SHA_UNKOWN:
+			elog(ERROR, "unknown crypt identifier \"%c\"", salt[1]);
+	}
+
+	if (rounds_custom > 0)
+	{
+		appendStringInfo(out_buf, "rounds=%lu$", rounds);
+	}
+
+	/*
+	 * We need the real decoded salt string from salt input, this is every
+	 * character before the last '$' in the preamble. Append every
+	 * compatible character up to PX_SHACRYPT_SALT_MAX_LEN to the result buffer.
+	 * Note that depending on the input, there might be no '$' marker after
+	 * the salt, when there is no password hash attached at the end.
+	 *
+	 * We try hard to recognize mistakes, but since we might get an input
+	 * string which might also have the password hash after the salt string
+	 * section we give up as soon we reach the end of the input or if there
+	 * are any bytes consumed for the salt string until we reach the
+	 * first '$' marker thereafter.
+	 */
+	for (ep = dec_salt_binary;
+		 *ep && ep < (dec_salt_binary + PX_SHACRYPT_SALT_MAX_LEN);
+		 ep++)
+	{
+		/*
+		 * Filter out any string which shouldn't be here.
+		 *
+		 * First check for accidentally embedded magic strings here. We don't
+		 * support '$' in salt strings anyways and seeing a magic byte trying
+		 * to identify shacrypt hashes might indicate that something went
+		 * wrong when generating this salt string. Note that we later check for
+		 * non-supported literals anyways, but any '$' here confuses us
+		 * at this point.
+		 */
+		fp = strstr(dec_salt_binary, magic_bytes[0]);
+
+		if (fp != NULL)
+		{
+			elog(ERROR, "bogus magic byte found in salt string");
+		}
+
+		fp = strstr(dec_salt_binary, magic_bytes[1]);
+
+		if (fp != NULL)
+		{
+			elog(ERROR, "bogus magic byte found in salt string");
+		}
+
+		/*
+		 * This looks very strict, but we assume the caller did something
+		 * wrong when we see a "rounds=" option here.
+		 */
+		fp = strstr(dec_salt_binary, rounds_prefix);
+
+		if (fp != NULL)
+		{
+			elog(ERROR, "invalid rounds option specified in salt string");
+		}
+
+		if (*ep != '$')
+		{
+			if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
+			{
+				appendStringInfoCharMacro(decoded_salt, *ep);
+			}
+			else
+			{
+				elog(ERROR, "invalid character in salt string: \"%c\"", *ep);
+			}
+		}
+		else
+		{
+			/* We encountered a '$' marker. Check if we already absorbed
+			 * some bytes from input. If true, we are optimistic and
+			 * terminate at this stage. Of not, we try further.
+			 *
+			 * If we already consumed enough bytes for the salt string, everything
+			 * that is after this marker is considered to be part of an
+			 * optionally specified password hash and ignored.
+			 */
+			if (decoded_salt->len > 0)
+				break;
+		}
+
+	}
+
+	salt_len = decoded_salt->len;
+	appendStringInfoString(out_buf, decoded_salt->data);
+	elog(DEBUG1, "using salt \"%s\", salt len = %d, rounds = %lu",
+		 decoded_salt->data, decoded_salt->len, rounds);
+
+	/*
+	 * Sanity check:
+	 *
+	 * At this point the salt string buffer must not exceed expected size
+	 */
+	if (out_buf->len > (3 + 17 * rounds_custom + salt_len))
+	{
+		elog(ERROR, "unexpected length of salt string");
+	}
+
+	/*-
+	 * 1. Start digest A
+	 * 2. Add the password string to digest A
+	 * 3. Add the salt to digest A
+	 */
+	px_md_update(digestA, (const unsigned char *) pw, len);
+	px_md_update(digestA, (const unsigned char *) decoded_salt->data, salt_len);
+
+	/*-
+	 * 4. Create digest B
+	 * 5. Add password to digest B
+	 * 6. Add the salt string to digest B
+	 * 7. Add the password again to digest B
+	 * 8. Finalize digest B
+	 */
+	px_md_update(digestB, (const unsigned char *) pw, len);
+	px_md_update(digestB, (const unsigned char *) dec_salt_binary, salt_len);
+	px_md_update(digestB, (const unsigned char *) pw, len);
+	px_md_finish(digestB, sha_buf);
+
+	/*-
+	 * 9. For each block of (excluding the NULL byte), add
+	 *    digest B to digest A.
+	 */
+	for (block = len; block > buf_size; block -= buf_size)
+	{
+		px_md_update(digestA, sha_buf, buf_size);
+	}
+
+	/*-
+	 * 10 For the remaining N bytes of the password string, add
+	 * the first N bytes of digest B to A */
+	px_md_update(digestA, sha_buf, block);
+
+	/*-
+	 * 11 For each bit of the binary representation of the length of the
+	 * password string up to and including the highest 1-digit, starting
+	 * from to lowest bit position (numeric value 1)
+	 *
+	 * a) for a 1-digit add digest B (sha_buf) to digest A
+	 * b) for a 0-digit add the password string
+	 */
+
+	block = len;
+	while (block)
+	{
+		px_md_update(digestA,
+					 (block & 1) ? sha_buf : (const unsigned char *) pw,
+					 (block & 1) ? buf_size : len);
+
+		/* right shift to next byte */
+		block >>= 1;
+	}
+
+	/* 12 Finalize digest A */
+	px_md_finish(digestA, sha_buf);
+
+	/* 13 Start digest DP */
+	px_md_reset(digestB);
+
+	/*-
+	 * 14 Add every byte of the password string (excluding trailing NULL)
+	 * to the digest DP
+	 */
+	for (block = len; block > 0; block--)
+	{
+		px_md_update(digestB, (const unsigned char *) pw, len);
+	}
+
+	/* 15 Finalize digest DP */
+	px_md_finish(digestB, sha_buf_tmp);
+
+	/*-
+	 * 16 produce byte sequence P with same length as password.
+	 *
+	 *     a) for each block of 32 or 64 bytes of length of the password
+	 *        string the entire digest DP is used
+	 *     b) for the remaining N (up to  31 or 63) bytes use the
+	 *         first N bytes of digest DP
+	 */
+	if ((p_bytes = palloc0(len)) == NULL)
+	{
+		goto error;
+	}
+
+	/* N step of 16, copy over the bytes from password */
+	for (cp = p_bytes, block = len; block > buf_size; block -= buf_size, cp += buf_size)
+		memcpy(cp, sha_buf_tmp, buf_size);
+	memcpy(cp, sha_buf_tmp, block);
+
+	/*
+	 * 17 Start digest DS
+	 */
+	px_md_reset(digestB);
+
+	/*-
+	 * 18 Repeat the following 16+A[0] times, where A[0] represents the first
+	 *    byte in digest A interpreted as an 8-bit unsigned value
+	 *    add the salt to digest DS
+	 */
+	for (block = 16 + sha_buf[0]; block > 0; block--)
+	{
+		px_md_update(digestB, (const unsigned char *) dec_salt_binary, salt_len);
+	}
+
+	/*
+	 * 19 Finalize digest DS
+	 */
+	px_md_finish(digestB, sha_buf_tmp);
+
+	/*-
+	 * 20 Produce byte sequence S of the same length as the salt string where
+	 *
+	 * a) for each block of 32 or 64 bytes of length of the salt string the
+	 *     entire digest DS is used
+	 *
+	 * b) for the remaining N (up to  31 or 63) bytes use the first N
+	 *    bytes of digest DS
+	 */
+	if ((s_bytes = palloc0(salt_len)) == NULL)
+		goto error;
+
+	for (cp = s_bytes, block = salt_len; block > buf_size; block -= buf_size, cp += buf_size)
+	{
+		memcpy(cp, sha_buf_tmp, buf_size);
+	}
+	memcpy(cp, sha_buf_tmp, block);
+
+	/* Make sure we don't leave something important behind */
+	px_memset(&sha_buf_tmp, 0, sizeof sha_buf);
+
+	/*-
+	 * 21 Repeat a loop according to the number specified in the rounds=<N>
+	 *    specification in the salt (or the default value if none is
+	 *    present).  Each round is numbered, starting with 0 and up to N-1.
+	 *
+	 *    The loop uses a digest as input.  In the first round it is the
+	 *    digest produced in step 12.  In the latter steps it is the digest
+	 *    produced in step 21.h of the previous round.  The following text
+	 *    uses the notation "digest A/B" to describe this behavior.
+	 */
+	for (block = 0; block < rounds; block++)
+	{
+
+		/*
+		 * Make it possible to abort in case large values for "rounds" are
+		 * specified.
+		 */
+		CHECK_FOR_INTERRUPTS();
+
+		/* a) start digest B */
+		px_md_reset(digestB);
+
+		/*
+		 * b) for odd round numbers add the byte sequense P to digest B c) for
+		 * even round numbers add digest A/B
+		 */
+		px_md_update(digestB,
+					 (block & 1) ? (const unsigned char *) p_bytes : sha_buf,
+					 (block & 1) ? len : buf_size);
+
+		/* d) for all round numbers not divisible by 3 add the byte sequence S */
+		if (block % 3)
+		{
+			px_md_update(digestB, (const unsigned char *) s_bytes, salt_len);
+		}
+
+		/* e) for all round numbers not divisible by 7 add the byte sequence P */
+		if (block % 7)
+		{
+			px_md_update(digestB, (const unsigned char *) p_bytes, len);
+		}
+
+		/*
+		 * f) for odd round numbers add digest A/C g) for even round numbers
+		 * add the byte sequence P
+		 */
+		px_md_update(digestB,
+					 (block & 1) ? sha_buf : (const unsigned char *) p_bytes,
+					 (block & 1) ? buf_size : len);
+
+		/* h) finish digest C. */
+		px_md_finish(digestB, sha_buf);
+
+	}
+
+	px_md_free(digestA);
+	px_md_free(digestB);
+
+	digestA = NULL;
+	digestB = NULL;
+
+	pfree(s_bytes);
+	pfree(p_bytes);
+
+	s_bytes = NULL;
+	p_bytes = NULL;
+
+	/* prepare final result buffer */
+	appendStringInfoCharMacro(out_buf, '$');
+
+#define b64_from_24bit(B2, B1, B0, N)                      \
+	do {                                                    \
+		unsigned int w = ((B2) << 16) | ((B1) << 8) | (B0); \
+		int i = (N);                                        \
+		while (i-- > 0)                                     \
+		{                                                   \
+			appendStringInfoCharMacro(out_buf, _crypt_itoa64[w & 0x3f]); \
+			w >>= 6;                                        \
+		}                                                   \
+	} while (0)
+
+	switch (type)
+	{
+		case PGCRYPTO_SHA256CRYPT:
+			{
+				b64_from_24bit(sha_buf[0], sha_buf[10], sha_buf[20], 4);
+				b64_from_24bit(sha_buf[21], sha_buf[1], sha_buf[11], 4);
+				b64_from_24bit(sha_buf[12], sha_buf[22], sha_buf[2], 4);
+				b64_from_24bit(sha_buf[3], sha_buf[13], sha_buf[23], 4);
+				b64_from_24bit(sha_buf[24], sha_buf[4], sha_buf[14], 4);
+				b64_from_24bit(sha_buf[15], sha_buf[25], sha_buf[5], 4);
+				b64_from_24bit(sha_buf[6], sha_buf[16], sha_buf[26], 4);
+				b64_from_24bit(sha_buf[27], sha_buf[7], sha_buf[17], 4);
+				b64_from_24bit(sha_buf[18], sha_buf[28], sha_buf[8], 4);
+				b64_from_24bit(sha_buf[9], sha_buf[19], sha_buf[29], 4);
+				b64_from_24bit(0, sha_buf[31], sha_buf[30], 3);
+
+				break;
+			}
+
+		case PGCRYPTO_SHA512CRYPT:
+			{
+				b64_from_24bit(sha_buf[0], sha_buf[21], sha_buf[42], 4);
+				b64_from_24bit(sha_buf[22], sha_buf[43], sha_buf[1], 4);
+				b64_from_24bit(sha_buf[44], sha_buf[2], sha_buf[23], 4);
+				b64_from_24bit(sha_buf[3], sha_buf[24], sha_buf[45], 4);
+				b64_from_24bit(sha_buf[25], sha_buf[46], sha_buf[4], 4);
+				b64_from_24bit(sha_buf[47], sha_buf[5], sha_buf[26], 4);
+				b64_from_24bit(sha_buf[6], sha_buf[27], sha_buf[48], 4);
+				b64_from_24bit(sha_buf[28], sha_buf[49], sha_buf[7], 4);
+				b64_from_24bit(sha_buf[50], sha_buf[8], sha_buf[29], 4);
+				b64_from_24bit(sha_buf[9], sha_buf[30], sha_buf[51], 4);
+				b64_from_24bit(sha_buf[31], sha_buf[52], sha_buf[10], 4);
+				b64_from_24bit(sha_buf[53], sha_buf[11], sha_buf[32], 4);
+				b64_from_24bit(sha_buf[12], sha_buf[33], sha_buf[54], 4);
+				b64_from_24bit(sha_buf[34], sha_buf[55], sha_buf[13], 4);
+				b64_from_24bit(sha_buf[56], sha_buf[14], sha_buf[35], 4);
+				b64_from_24bit(sha_buf[15], sha_buf[36], sha_buf[57], 4);
+				b64_from_24bit(sha_buf[37], sha_buf[58], sha_buf[16], 4);
+				b64_from_24bit(sha_buf[59], sha_buf[17], sha_buf[38], 4);
+				b64_from_24bit(sha_buf[18], sha_buf[39], sha_buf[60], 4);
+				b64_from_24bit(sha_buf[40], sha_buf[61], sha_buf[19], 4);
+				b64_from_24bit(sha_buf[62], sha_buf[20], sha_buf[41], 4);
+				b64_from_24bit(0, 0, sha_buf[63], 2);
+
+				break;
+			}
+
+		case PGCRYPTO_SHA_UNKOWN:
+			/* we shouldn't land here ... */
+			elog(ERROR, "unsupported digest length");
+
+	}
+
+	*cp = '\0';
+
+	/*
+	 * Copy over result to specified buffer.
+	 *
+	 * The passwd character buffer should have at least PX_SHACRYPT_BUF_LEN
+	 * allocated, since we checked above if dstlen is smaller than
+	 * PX_SHACRYPT_BUF_LEN (which also includes the NULL byte).
+	 *
+	 * In that case we would have failed above already.
+	 */
+	memcpy(passwd, out_buf->data, out_buf->len);
+
+	/* make sure nothing important is left behind */
+	px_memset(&sha_buf, 0, sizeof sha_buf);
+	destroyStringInfo(out_buf);
+	destroyStringInfo(decoded_salt);
+
+	/* ...and we're done */
+	return passwd;
+
+error:
+	if (digestA != NULL)
+		px_md_free(digestA);
+
+	if (digestB != NULL)
+		px_md_free(digestB);
+
+	if (out_buf != NULL)
+		destroyStringInfo(out_buf);
+
+	ereport(ERROR,
+			(errcode(ERRCODE_INTERNAL_ERROR),
+			 errmsg("cannot create encrypted password")));
+	return NULL;				/* keep compiler quiet */
+}
diff --git a/contrib/pgcrypto/expected/crypt-shacrypt.out b/contrib/pgcrypto/expected/crypt-shacrypt.out
new file mode 100644
index 00000000000..f3b76d5e913
--- /dev/null
+++ b/contrib/pgcrypto/expected/crypt-shacrypt.out
@@ -0,0 +1,196 @@
+--
+-- crypt() and gensalt: sha256crypt, sha512crypt
+--
+-- $5$ is sha256crypt
+SELECT crypt('', '$5$Szzz0yzz');
+                          crypt                          
+---------------------------------------------------------
+ $5$Szzz0yzz$cA.ZFZKqblRYjdsbrWtVTYa/qSwPQnt2uh0LBtyYAAD
+(1 row)
+
+SELECT crypt('foox', '$5$Szzz0yzz');
+                          crypt                          
+---------------------------------------------------------
+ $5$Szzz0yzz$7hI0rUWkO2QdBkzamh.vP.MIPlbZiwSvu2smhSi6064
+(1 row)
+
+CREATE TABLE ctest (data text, res text, salt text);
+INSERT INTO ctest VALUES ('password', '', '');
+-- generate a salt for sha256crypt, default rounds
+UPDATE ctest SET salt = gen_salt('sha256crypt');
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+ worked 
+--------
+ t
+(1 row)
+
+-- generate a salt for sha256crypt, rounds 9999
+UPDATE ctest SET salt = gen_salt('sha256crypt', 9999);
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+ worked 
+--------
+ t
+(1 row)
+
+-- should fail, below supported minimum rounds value
+UPDATE ctest SET salt = gen_salt('sha256crypt', 10);
+ERROR:  gen_salt: Incorrect number of rounds
+-- should fail, exceeds supported maximum rounds value
+UPDATE ctest SET salt = gen_salt('sha256crypt', 1000000000);
+ERROR:  gen_salt: Incorrect number of rounds
+TRUNCATE ctest;
+-- $6$ is sha512crypt
+SELECT crypt('', '$6$Szzz0yzz');
+                                               crypt                                                
+----------------------------------------------------------------------------------------------------
+ $6$Szzz0yzz$EGj.JLAovFyAtCJx3YD1DXD1yTXoO9gv4qgLyHBsJJ1lkpnLB8ZPHekm1qXjJCOBc/8thCuHpxNN8Y5xzRYU5.
+(1 row)
+
+SELECT crypt('foox', '$6$Szzz0yzz');
+                                               crypt                                                
+----------------------------------------------------------------------------------------------------
+ $6$Szzz0yzz$KqDw1Y8kze.VFapkvTc9Y5fbqzltjeRz1aPGC/pkHRhFQZ2aM6PmZpXQjcD7AOH88Bq0CSD.VlmymQzcBMEUl0
+(1 row)
+
+INSERT INTO ctest VALUES ('password', '', '');
+-- generate a salt for sha512crypt, default rounds
+UPDATE ctest SET salt = gen_salt('sha512crypt');
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+ worked 
+--------
+ t
+(1 row)
+
+-- generate a salt for sha512crypt, rounds 9999
+UPDATE ctest SET salt = gen_salt('sha512crypt', 9999);
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+ worked 
+--------
+ t
+(1 row)
+
+-- should fail, below supported minimum rounds value
+UPDATE ctest SET salt = gen_salt('sha512crypt', 10);
+ERROR:  gen_salt: Incorrect number of rounds
+-- should fail, exceeds supported maximum rounds value
+UPDATE ctest SET salt = gen_salt('sha512crypt', 1000000000);
+ERROR:  gen_salt: Incorrect number of rounds
+-- Extended tests taken from public domain code at
+-- https://www.akkadia.org/drepper/SHA-crypt.txt
+--
+-- We adapt the tests defined there to make sure we are compatible with the reference
+-- implementation.
+-- This tests sha256crypt (magic byte $5$ with salt and rounds)
+SELECT crypt('Hello world!', '$5$saltstring')
+           = '$5$saltstring$5B8vYYiY.CVt1RlTTf8KbXBH3hsxY/GNooZaBBGWEc5' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('Hello world!', '$5$rounds=10000$saltstringsaltstring')
+           = '$5$rounds=10000$saltstringsaltst$3xv.VbSHBb41AL9AvLeujZkZRBAwqFMz2.opqey6IcA' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('This is just a test', '$5$rounds=5000$toolongsaltstring')
+           = '$5$rounds=5000$toolongsaltstrin$Un/5jzAHMgOGZ5.mWJpuVolil07guHPvOW8mGRcvxa5' AS result;
+ result 
+--------
+ t
+(1 row)
+
+ SELECT crypt('a very much longer text to encrypt.  This one even stretches over more'
+     'than one line.', '$5$rounds=1400$anotherlongsaltstring')
+            = '$5$rounds=1400$anotherlongsalts$Rx.j8H.h8HjEDGomFU8bDkXm3XIUnzyxf12oP84Bnq1' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('we have a short salt string but not a short password', '$5$rounds=77777$short')
+           = '$5$rounds=77777$short$JiO1O3ZpDAxGJeaDIuqCoEFysAe1mZNJRs3pw0KQRd/' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('a short string', '$5$rounds=123456$asaltof16chars..')
+           = '$5$rounds=123456$asaltof16chars..$gP3VQ/6X7UUEW3HkBn2w1/Ptq2jxPyzV/cZKmF/wJvD' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('the minimum number is still observed', '$5$rounds=10$roundstoolow')
+           = '$5$rounds=1000$roundstoolow$yfvwcWrQ8l/K0DAWyuPMDNHpIVlTQebY9l/gL972bIC' AS result;
+NOTICE:  rounds=10 is below supported value (1000), using 1000 instead
+ result 
+--------
+ t
+(1 row)
+
+-- The following tests sha512crypt (magic byte $6$ with salt and rounds)
+SELECT crypt('Hello world!', '$6$saltstring')
+           = '$6$saltstring$svn8UoSVapNtMuq1ukKS4tPQd8iKwSMHWjl/O817G3uBnIFNjnQJuesI68u4OTLiBFdcbYEdFCoEOfaS35inz1' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('Hello world!', '$6$rounds=10000$saltstringsaltstring')
+           = '$6$rounds=10000$saltstringsaltst$OW1/O6BYHV6BcXZu8QVeXbDWra3Oeqh0sbHbbMCVNSnCM/UrjmM0Dp8vOuZeHBy/YTBmSK6H9qs/y3RnOaw5v.' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('This is just a test', '$6$rounds=5000$toolongsaltstring')
+           = '$6$rounds=5000$toolongsaltstrin$lQ8jolhgVRVhY4b5pZKaysCLi0QBxGoNeKQzQ3glMhwllF7oGDZxUhx1yxdYcz/e1JSbq3y6JMxxl8audkUEm0' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('a very much longer text to encrypt.  This one even stretches over more'
+                 'than one line.', '$6$rounds=1400$anotherlongsaltstring')
+           = '$6$rounds=1400$anotherlongsalts$POfYwTEok97VWcjxIiSOjiykti.o/pQs.wPvMxQ6Fm7I6IoYN3CmLs66x9t0oSwbtEW7o7UmJEiDwGqd8p4ur1' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('we have a short salt string but not a short password', '$6$rounds=77777$short')
+           = '$6$rounds=77777$short$WuQyW2YR.hBNpjjRhpYD/ifIw05xdfeEyQoMxIXbkvr0gge1a1x3yRULJ5CCaUeOxFmtlcGZelFl5CxtgfiAc0' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('a short string', '$6$rounds=123456$asaltof16chars..')
+           = '$6$rounds=123456$asaltof16chars..$BtCwjqMJGx5hrJhZywWvt0RLE8uZ4oPwcelCjmw2kSYu.Ec6ycULevoBK25fs2xXgMNrCzIMVcgEJAstJeonj1' AS result;
+ result 
+--------
+ t
+(1 row)
+
+SELECT crypt('the minimum number is still observed', '$6$rounds=10$roundstoolow')
+           = '$6$rounds=1000$roundstoolow$kUMsbe306n21p9R.FRkW3IGn.S9NPN0x50YhH1xhLsPuWGsUSklZt58jaTfF4ZEQpyUNGc0dqbpBYYBaHHrsX.' AS result;
+NOTICE:  rounds=10 is below supported value (1000), using 1000 instead
+ result 
+--------
+ t
+(1 row)
+
+-- cleanup
+DROP TABLE ctest;
diff --git a/contrib/pgcrypto/meson.build b/contrib/pgcrypto/meson.build
index 7a4e8e76d64..7d5ef9b6d32 100644
--- a/contrib/pgcrypto/meson.build
+++ b/contrib/pgcrypto/meson.build
@@ -9,6 +9,7 @@ pgcrypto_sources = files(
   'crypt-des.c',
   'crypt-gensalt.c',
   'crypt-md5.c',
+  'crypt-sha.c',
   'mbuf.c',
   'pgcrypto.c',
   'pgp-armor.c',
@@ -52,6 +53,7 @@ pgcrypto_regress = [
   'pgp-pubkey-decrypt',
   'pgp-pubkey-encrypt',
   'pgp-info',
+  'crypt-shacrypt'
 ]
 
 pgcrypto_openssl_sources = files(
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 96ce9384aff..df9875b9433 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -67,6 +67,16 @@ run_crypt_bf(const char *psw, const char *salt,
 	return res;
 }
 
+static char *
+run_crypt_sha(const char *psw, const char *salt,
+			  char *buf, unsigned len)
+{
+	char *res;
+
+	res = px_crypt_shacrypt(psw, salt, buf, len);
+	return res;
+}
+
 struct px_crypt_algo
 {
 	char	   *id;
@@ -81,6 +91,8 @@ static const struct px_crypt_algo
 	{"$2x$", 4, run_crypt_bf},
 	{"$2$", 3, NULL},			/* N/A */
 	{"$1$", 3, run_crypt_md5},
+	{"$5$", 3, run_crypt_sha},
+	{"$6$", 3, run_crypt_sha},
 	{"_", 1, run_crypt_des},
 	{"", 0, run_crypt_des},
 	{NULL, 0, NULL}
@@ -127,6 +139,10 @@ static struct generator gen_list[] = {
 	{"md5", _crypt_gensalt_md5_rn, 6, 0, 0, 0},
 	{"xdes", _crypt_gensalt_extended_rn, 3, PX_XDES_ROUNDS, 1, 0xFFFFFF},
 	{"bf", _crypt_gensalt_blowfish_rn, 16, PX_BF_ROUNDS, 4, 31},
+	{"sha256crypt", _crypt_gensalt_sha256_rn, PX_SHACRYPT_SALT_MAX_LEN,
+	 PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
+	{"sha512crypt", _crypt_gensalt_sha512_rn, PX_SHACRYPT_SALT_MAX_LEN,
+	 PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
 	{NULL, NULL, 0, 0, 0, 0}
 };
 
diff --git a/contrib/pgcrypto/px-crypt.h b/contrib/pgcrypto/px-crypt.h
index 54de8069655..9287c33efe1 100644
--- a/contrib/pgcrypto/px-crypt.h
+++ b/contrib/pgcrypto/px-crypt.h
@@ -45,6 +45,30 @@
 /* default for blowfish salt */
 #define PX_BF_ROUNDS		6
 
+/* Maximum salt string length of shacrypt.  */
+#define PX_SHACRYPT_SALT_MAX_LEN 16
+
+/* SHA buffer length */
+#define PX_SHACRYPT_DIGEST_MAX_LEN 64
+
+/* calculated buffer size of a buffer to store a shacrypt salt string */
+#define PX_SHACRYPT_SALT_BUF_LEN (3 + 7 + 10 + PX_SHACRYPT_SALT_MAX_LEN + 1)
+
+/*
+ * calculated buffer size of a buffer to store complete result of a shacrypt
+ * digest including salt
+ */
+#define PX_SHACRYPT_BUF_LEN (PX_SHACRYPT_SALT_BUF_LEN + 86 + 1)
+
+/* Default number of rounds of shacrypt if not explicitly specified.  */
+#define PX_SHACRYPT_ROUNDS_DEFAULT 5000l
+
+/* Minimum number of rounds of shacrypt.  */
+#define PX_SHACRYPT_ROUNDS_MIN 1000l
+
+/* Maximum number of rounds of shacrypt.  */
+#define PX_SHACRYPT_ROUNDS_MAX 999999999l
+
 /*
  * main interface
  */
@@ -64,6 +88,10 @@ char	   *_crypt_gensalt_md5_rn(unsigned long count,
 								  const char *input, int size, char *output, int output_size);
 char	   *_crypt_gensalt_blowfish_rn(unsigned long count,
 									   const char *input, int size, char *output, int output_size);
+char	   *_crypt_gensalt_sha256_rn(unsigned long count,
+									 const char *input, int size, char *output, int output_size);
+char	   *_crypt_gensalt_sha512_rn(unsigned long count,
+									 const char *input, int size, char *output, int output_size);
 
 /* disable 'extended DES crypt' */
 /* #define DISABLE_XDES */
@@ -79,4 +107,7 @@ char	   *px_crypt_des(const char *key, const char *setting);
 char	   *px_crypt_md5(const char *pw, const char *salt,
 						 char *passwd, unsigned dstlen);
 
+/* crypt-sha.c */
+char       *px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstlen);
+
 #endif							/* _PX_CRYPT_H */
diff --git a/contrib/pgcrypto/sql/crypt-shacrypt.sql b/contrib/pgcrypto/sql/crypt-shacrypt.sql
new file mode 100644
index 00000000000..3ee826f61aa
--- /dev/null
+++ b/contrib/pgcrypto/sql/crypt-shacrypt.sql
@@ -0,0 +1,99 @@
+--
+-- crypt() and gensalt: sha256crypt, sha512crypt
+--
+
+-- $5$ is sha256crypt
+SELECT crypt('', '$5$Szzz0yzz');
+
+SELECT crypt('foox', '$5$Szzz0yzz');
+
+CREATE TABLE ctest (data text, res text, salt text);
+INSERT INTO ctest VALUES ('password', '', '');
+
+-- generate a salt for sha256crypt, default rounds
+UPDATE ctest SET salt = gen_salt('sha256crypt');
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+
+-- generate a salt for sha256crypt, rounds 9999
+UPDATE ctest SET salt = gen_salt('sha256crypt', 9999);
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+
+-- should fail, below supported minimum rounds value
+UPDATE ctest SET salt = gen_salt('sha256crypt', 10);
+
+-- should fail, exceeds supported maximum rounds value
+UPDATE ctest SET salt = gen_salt('sha256crypt', 1000000000);
+
+TRUNCATE ctest;
+
+-- $6$ is sha512crypt
+SELECT crypt('', '$6$Szzz0yzz');
+
+SELECT crypt('foox', '$6$Szzz0yzz');
+
+INSERT INTO ctest VALUES ('password', '', '');
+
+-- generate a salt for sha512crypt, default rounds
+UPDATE ctest SET salt = gen_salt('sha512crypt');
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+
+-- generate a salt for sha512crypt, rounds 9999
+UPDATE ctest SET salt = gen_salt('sha512crypt', 9999);
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+
+-- should fail, below supported minimum rounds value
+UPDATE ctest SET salt = gen_salt('sha512crypt', 10);
+
+-- should fail, exceeds supported maximum rounds value
+UPDATE ctest SET salt = gen_salt('sha512crypt', 1000000000);
+
+-- Extended tests taken from public domain code at
+-- https://www.akkadia.org/drepper/SHA-crypt.txt
+--
+-- We adapt the tests defined there to make sure we are compatible with the reference
+-- implementation.
+
+-- This tests sha256crypt (magic byte $5$ with salt and rounds)
+SELECT crypt('Hello world!', '$5$saltstring')
+           = '$5$saltstring$5B8vYYiY.CVt1RlTTf8KbXBH3hsxY/GNooZaBBGWEc5' AS result;
+SELECT crypt('Hello world!', '$5$rounds=10000$saltstringsaltstring')
+           = '$5$rounds=10000$saltstringsaltst$3xv.VbSHBb41AL9AvLeujZkZRBAwqFMz2.opqey6IcA' AS result;
+SELECT crypt('This is just a test', '$5$rounds=5000$toolongsaltstring')
+           = '$5$rounds=5000$toolongsaltstrin$Un/5jzAHMgOGZ5.mWJpuVolil07guHPvOW8mGRcvxa5' AS result;
+ SELECT crypt('a very much longer text to encrypt.  This one even stretches over more'
+     'than one line.', '$5$rounds=1400$anotherlongsaltstring')
+            = '$5$rounds=1400$anotherlongsalts$Rx.j8H.h8HjEDGomFU8bDkXm3XIUnzyxf12oP84Bnq1' AS result;
+SELECT crypt('we have a short salt string but not a short password', '$5$rounds=77777$short')
+           = '$5$rounds=77777$short$JiO1O3ZpDAxGJeaDIuqCoEFysAe1mZNJRs3pw0KQRd/' AS result;
+SELECT crypt('a short string', '$5$rounds=123456$asaltof16chars..')
+           = '$5$rounds=123456$asaltof16chars..$gP3VQ/6X7UUEW3HkBn2w1/Ptq2jxPyzV/cZKmF/wJvD' AS result;
+SELECT crypt('the minimum number is still observed', '$5$rounds=10$roundstoolow')
+           = '$5$rounds=1000$roundstoolow$yfvwcWrQ8l/K0DAWyuPMDNHpIVlTQebY9l/gL972bIC' AS result;
+
+-- The following tests sha512crypt (magic byte $6$ with salt and rounds)
+SELECT crypt('Hello world!', '$6$saltstring')
+           = '$6$saltstring$svn8UoSVapNtMuq1ukKS4tPQd8iKwSMHWjl/O817G3uBnIFNjnQJuesI68u4OTLiBFdcbYEdFCoEOfaS35inz1' AS result;
+SELECT crypt('Hello world!', '$6$rounds=10000$saltstringsaltstring')
+           = '$6$rounds=10000$saltstringsaltst$OW1/O6BYHV6BcXZu8QVeXbDWra3Oeqh0sbHbbMCVNSnCM/UrjmM0Dp8vOuZeHBy/YTBmSK6H9qs/y3RnOaw5v.' AS result;
+SELECT crypt('This is just a test', '$6$rounds=5000$toolongsaltstring')
+           = '$6$rounds=5000$toolongsaltstrin$lQ8jolhgVRVhY4b5pZKaysCLi0QBxGoNeKQzQ3glMhwllF7oGDZxUhx1yxdYcz/e1JSbq3y6JMxxl8audkUEm0' AS result;
+SELECT crypt('a very much longer text to encrypt.  This one even stretches over more'
+                 'than one line.', '$6$rounds=1400$anotherlongsaltstring')
+           = '$6$rounds=1400$anotherlongsalts$POfYwTEok97VWcjxIiSOjiykti.o/pQs.wPvMxQ6Fm7I6IoYN3CmLs66x9t0oSwbtEW7o7UmJEiDwGqd8p4ur1' AS result;
+SELECT crypt('we have a short salt string but not a short password', '$6$rounds=77777$short')
+           = '$6$rounds=77777$short$WuQyW2YR.hBNpjjRhpYD/ifIw05xdfeEyQoMxIXbkvr0gge1a1x3yRULJ5CCaUeOxFmtlcGZelFl5CxtgfiAc0' AS result;
+SELECT crypt('a short string', '$6$rounds=123456$asaltof16chars..')
+           = '$6$rounds=123456$asaltof16chars..$BtCwjqMJGx5hrJhZywWvt0RLE8uZ4oPwcelCjmw2kSYu.Ec6ycULevoBK25fs2xXgMNrCzIMVcgEJAstJeonj1' AS result;
+SELECT crypt('the minimum number is still observed', '$6$rounds=10$roundstoolow')
+           = '$6$rounds=1000$roundstoolow$kUMsbe306n21p9R.FRkW3IGn.S9NPN0x50YhH1xhLsPuWGsUSklZt58jaTfF4ZEQpyUNGc0dqbpBYYBaHHrsX.' AS result;
+
+-- cleanup
+DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index f87668dfaed..b567b5c16e8 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -189,6 +189,29 @@ hmac(data bytea, key bytea, type text) returns bytea
       <entry>13</entry>
       <entry>Original UNIX crypt</entry>
      </row>
+     <row>
+      <entry><literal>sha256crypt</literal></entry>
+      <entry>unlimited</entry>
+      <entry>yes</entry>
+      <entry>up to 32</entry>
+      <entry>80</entry>
+      <entry>Adapted from publicly available reference implementation
+       <ulink url="https://www.akkadia.org/drepper/SHA-crypt.txt">Unix crypt using SHA-256 and SHA-512
+       </ulink>
+      </entry>
+     </row>
+     <row>
+      <entry><literal>sha512crypt</literal></entry>
+      <entry>unlimited</entry>
+      <entry>yes</entry>
+      <entry>up to 32</entry>
+      <entry>123</entry>
+      <entry>Adapted from publicly available reference implementation
+       <ulink url="https://www.akkadia.org/drepper/SHA-crypt.txt">Unix crypt using SHA-256 and SHA-512
+       </ulink>
+      </entry>
+     </row>
+
     </tbody>
    </tgroup>
   </table>
@@ -245,7 +268,9 @@ gen_salt(type text [, iter_count integer ]) returns text
    <para>
     The <parameter>type</parameter> parameter specifies the hashing algorithm.
     The accepted types are: <literal>des</literal>, <literal>xdes</literal>,
-    <literal>md5</literal> and <literal>bf</literal>.
+    <literal>md5</literal>, <literal>bf</literal>, <literal>sha256crypt</literal> and
+    <literal>sha512crypt</literal>. The last two, <literal>sha256crypt</literal> and
+    <literal>sha512crypt</literal> are modern <literal>SHA-2</literal> based password hashes.
    </para>
 
    <para>
@@ -284,6 +309,12 @@ gen_salt(type text [, iter_count integer ]) returns text
        <entry>4</entry>
        <entry>31</entry>
       </row>
+      <row>
+       <entry><literal>sha256crypt, sha512crypt</literal></entry>
+       <entry>5000</entry>
+       <entry>1000</entry>
+       <entry>999999999</entry>
+      </row>
      </tbody>
     </tgroup>
    </table>
@@ -313,6 +344,14 @@ gen_salt(type text [, iter_count integer ]) returns text
     <function>gen_salt</function>.
    </para>
 
+   <para>
+   The default <parameter>iter_count</parameter> for <literal>sha256crypt</literal> and
+   <literal>sha512crypt</literal> of <literal>5000</literal> is considered too low for modern
+   hardware, but can be adjusted to generate stronger password hashes.
+   Otherwise both hashes, <literal>sha256crypt</literal> and <literal>sha512crypt</literal> are
+   considered safe.
+   </para>
+
    <table id="pgcrypto-hash-speed-table">
     <title>Hash Algorithm Speeds</title>
     <tgroup cols="5">
-- 
2.48.1



^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
@ 2025-04-05 17:22 Alvaro Herrera <[email protected]>
  2025-04-06 17:39 ` Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
  2025-04-06 19:43 ` Re: Modern SHA2- based password hashes for pgcrypto Tom Lane <[email protected]>
  2025-04-07 03:02 ` Re: Modern SHA2- based password hashes for pgcrypto Andres Freund <[email protected]>
  0 siblings, 3 replies; 10+ messages in thread

From: Alvaro Herrera @ 2025-04-05 17:22 UTC (permalink / raw)
  To: Bernd Helmle <[email protected]>; +Cc: Japin Li <[email protected]>; pgsql-hackers

Hello,

I have pushed this now, hoping it won't explode.

Thanks!

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"No es bueno caminar con un hombre muerto"






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
  2025-04-05 17:22 Re: Modern SHA2- based password hashes for pgcrypto Alvaro Herrera <[email protected]>
@ 2025-04-06 17:39 ` Bernd Helmle <[email protected]>
  2 siblings, 0 replies; 10+ messages in thread

From: Bernd Helmle @ 2025-04-06 17:39 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Japin Li <[email protected]>; pgsql-hackers

Am Samstag, dem 05.04.2025 um 19:22 +0200 schrieb Alvaro Herrera:
> Hello,
> 
> I have pushed this now, hoping it won't explode.
> 
> Thanks!

Very cool, i keep my fingers crossed.

Thanks,
	Bernd






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
  2025-04-05 17:22 Re: Modern SHA2- based password hashes for pgcrypto Alvaro Herrera <[email protected]>
@ 2025-04-06 19:43 ` Tom Lane <[email protected]>
  2025-04-07 07:03   ` Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
  2 siblings, 1 reply; 10+ messages in thread

From: Tom Lane @ 2025-04-06 19:43 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Bernd Helmle <[email protected]>; Japin Li <[email protected]>; pgsql-hackers

Alvaro Herrera <[email protected]> writes:
> I have pushed this now, hoping it won't explode.

mamba is not happy:

ccache cc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wshadow=compatible-local -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -g -O2 -Werror -fPIC -DPIC -fvisibility=hidden -I. -I. -I../../src/include   -I/usr/pkg/include/libxml2 -I/usr/pkg/include -I/usr/pkg/include  -I/usr/pkg/include  -c -o pgcrypto.o pgcrypto.c
In file included from /usr/include/ctype.h:100,
                 from ../../src/include/port.h:16,
                 from ../../src/include/c.h:1345,
                 from ../../src/include/postgres.h:48,
                 from crypt-sha.c:46:
crypt-sha.c: In function 'px_crypt_shacrypt':
crypt-sha.c:324:16: error: array subscript has type 'char' [-Werror=char-subscripts]
  324 |    if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
      |                ^
crypt-sha.c:324:32: error: array subscript has type 'char' [-Werror=char-subscripts]
  324 |    if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
      |                                ^
cc1: all warnings being treated as errors
gmake[1]: *** [<builtin>: crypt-sha.o] Error 1

What this is on about is that portable use of isalpha() or isdigit()
requires casting a "char" value to "unsigned char".  I was about to
make that simple change when I started to question if we actually
want to be using <ctype.h> here at all.  Do you REALLY intend that
any random non-ASCII letter is okay to include in the decoded_salt?
Are you okay with that filter being locale-dependent?

I'd be more comfortable with a check like

	if (strchr("...valid chars...", *ep) != NULL)

It looks like "_crypt_itoa64" might be directly usable as the
valid-chars string, too.  (BTW, why is _crypt_itoa64 not
marked const?)

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
  2025-04-05 17:22 Re: Modern SHA2- based password hashes for pgcrypto Alvaro Herrera <[email protected]>
  2025-04-06 19:43 ` Re: Modern SHA2- based password hashes for pgcrypto Tom Lane <[email protected]>
@ 2025-04-07 07:03   ` Bernd Helmle <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Bernd Helmle @ 2025-04-07 07:03 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: Japin Li <[email protected]>; pgsql-hackers

Am Sonntag, dem 06.04.2025 um 15:43 -0400 schrieb Tom Lane:
> What this is on about is that portable use of isalpha() or isdigit()
> requires casting a "char" value to "unsigned char".  I was about to
> make that simple change when I started to question if we actually
> want to be using <ctype.h> here at all.  Do you REALLY intend that
> any random non-ASCII letter is okay to include in the decoded_salt?
> Are you okay with that filter being locale-dependent?
> 
> I'd be more comfortable with a check like
> 
> 	if (strchr("...valid chars...", *ep) != NULL)
> 

Hmm, the idea was to allow a wider range of letters for the salt. This
came from my experiments that openssl allows more than just the ones
from _crypt_itoa64. But after reading this, i realized even isalpha()
isn't enough, since e.g. mulitbyte character wouldn't work anyways,
like openssl allows:

echo -n password | openssl passwd -5 -salt ÄÖÜ -stdin
$5$ÄÖÜ$NduBUgCdzvnW1lW8EMxW9DuxN6HmT0niS7H4ftRxuX0

So we would have to test for these cases, too. I looked again into
pyhon's passlib, and they don't accept any non-ASCII fancy characters,
neither.

So i think Tom has a point, we can restrict this to the characters from
_crypt_itoa64 and go that route. I am not sure about externally
generated hashes which are going to be stored in postgres and validated
later there. This can restrict the use case perhaps.

> It looks like "_crypt_itoa64" might be directly usable as the
> valid-chars string, too.  (BTW, why is _crypt_itoa64 not
> marked const?)

That's an oversight by me.

I can create a patch with the fixes (and the one Andres' reported) for
today.

Thanks

	Bernd







^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
  2025-04-05 17:22 Re: Modern SHA2- based password hashes for pgcrypto Alvaro Herrera <[email protected]>
@ 2025-04-07 03:02 ` Andres Freund <[email protected]>
  2025-04-07 07:09   ` Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
  2 siblings, 1 reply; 10+ messages in thread

From: Andres Freund @ 2025-04-07 03:02 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Bernd Helmle <[email protected]>; Japin Li <[email protected]>; pgsql-hackers

Hi,

On 2025-04-05 19:22:58 +0200, Alvaro Herrera wrote:
> I have pushed this now, hoping it won't explode.

I have a WIP patch that adds gcc specific allocator attributes for palloc et
al. Just rebased that. It warns on the new code as follows:

[1489/1804 42  82%] Compiling C object contrib/pgcrypto/pgcrypto.so.p/crypt-sha.c.o
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-sha.c: In function 'px_crypt_shacrypt':
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-sha.c:605:13: warning: pointer 'cp' may be used after 'pfree' [-Wuse-after-free]
  605 |         *cp = '\0';
      |         ~~~~^~~~~~
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-sha.c:533:9: note: call to 'pfree' here
  533 |         pfree(s_bytes);
      |         ^~~~~~~~~~~~~~

And it sure seems to have a point.  I'm surprised this isn't causing wider
issues...

Greetings,

Andres Freund





^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
  2025-04-05 17:22 Re: Modern SHA2- based password hashes for pgcrypto Alvaro Herrera <[email protected]>
  2025-04-07 03:02 ` Re: Modern SHA2- based password hashes for pgcrypto Andres Freund <[email protected]>
@ 2025-04-07 07:09   ` Bernd Helmle <[email protected]>
  2025-04-07 18:00     ` Re: Modern SHA2- based password hashes for pgcrypto Andres Freund <[email protected]>
  0 siblings, 1 reply; 10+ messages in thread

From: Bernd Helmle @ 2025-04-07 07:09 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; Alvaro Herrera <[email protected]>; +Cc: Japin Li <[email protected]>; pgsql-hackers

Am Sonntag, dem 06.04.2025 um 23:02 -0400 schrieb Andres Freund:
> Hi,
> 
> On 2025-04-05 19:22:58 +0200, Alvaro Herrera wrote:
> > I have pushed this now, hoping it won't explode.
> 
> I have a WIP patch that adds gcc specific allocator attributes for
> palloc et
> al. Just rebased that. It warns on the new code as follows:
> 
> [1489/1804 42  82%] Compiling C object
> contrib/pgcrypto/pgcrypto.so.p/crypt-sha.c.o
> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-
> sha.c: In function 'px_crypt_shacrypt':
> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-
> sha.c:605:13: warning: pointer 'cp' may be used after 'pfree' [-Wuse-
> after-free]
>   605 |         *cp = '\0';
>       |         ~~~~^~~~~~
> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-
> sha.c:533:9: note: call to 'pfree' here
>   533 |         pfree(s_bytes);
>       |         ^~~~~~~~~~~~~~
> 
> And it sure seems to have a point.  I'm surprised this isn't causing
> wider
> issues...

Indeed. I think this assignment is useless anyways, since s_bytes is
already allocated with palloc0. I must have overseen this one when
rearranging code...but yes, strange that it didn't cause drama.


Thanks
	Bernd







^ permalink  raw  reply  [nested|flat] 10+ messages in thread

* Re: Modern SHA2- based password hashes for pgcrypto
  2025-04-05 17:22 Re: Modern SHA2- based password hashes for pgcrypto Alvaro Herrera <[email protected]>
  2025-04-07 03:02 ` Re: Modern SHA2- based password hashes for pgcrypto Andres Freund <[email protected]>
  2025-04-07 07:09   ` Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
@ 2025-04-07 18:00     ` Andres Freund <[email protected]>
  0 siblings, 0 replies; 10+ messages in thread

From: Andres Freund @ 2025-04-07 18:00 UTC (permalink / raw)
  To: Bernd Helmle <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Japin Li <[email protected]>; pgsql-hackers

Hi,

On 2025-04-07 09:09:39 +0200, Bernd Helmle wrote:
> Am Sonntag, dem 06.04.2025 um 23:02 -0400 schrieb Andres Freund:
> > On 2025-04-05 19:22:58 +0200, Alvaro Herrera wrote:
> > > I have pushed this now, hoping it won't explode.
> > 
> > I have a WIP patch that adds gcc specific allocator attributes for
> > palloc et
> > al. Just rebased that. It warns on the new code as follows:
> > 
> > [1489/1804 42  82%] Compiling C object
> > contrib/pgcrypto/pgcrypto.so.p/crypt-sha.c.o
> > ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-
> > sha.c: In function 'px_crypt_shacrypt':
> > ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-
> > sha.c:605:13: warning: pointer 'cp' may be used after 'pfree' [-Wuse-
> > after-free]
> >   605 |         *cp = '\0';
> >       |         ~~~~^~~~~~
> > ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-
> > sha.c:533:9: note: call to 'pfree' here
> >   533 |         pfree(s_bytes);
> >       |         ^~~~~~~~~~~~~~
> > 
> > And it sure seems to have a point.  I'm surprised this isn't causing
> > wider
> > issues...
> 
> Indeed. I think this assignment is useless anyways, since s_bytes is
> already allocated with palloc0. I must have overseen this one when
> rearranging code...but yes, strange that it didn't cause drama.

Valgrind did find it actually, I just missed it due to other failures:

https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=skink&dt=2025-04-05%2020%3A52%3A38...

==2917549== VALGRINDERROR-BEGIN
==2917549== Invalid write of size 1
==2917549==    at 0x11D4AD9C: px_crypt_shacrypt (crypt-sha.c:605)
==2917549==    by 0x11D54B50: run_crypt_sha (px-crypt.c:76)
==2917549==    by 0x11D54BCB: px_crypt (px-crypt.c:119)
==2917549==    by 0x11D4BCBB: pg_crypt (pgcrypto.c:228)
==2917549==    by 0x3BBCEA: ExecInterpExpr (execExprInterp.c:1001)
==2917549==    by 0x3B790E: ExecInterpExprStillValid (execExprInterp.c:2299)
==2917549==    by 0x48FD40: ExecEvalExprSwitchContext (executor.h:466)
==2917549==    by 0x48FD40: evaluate_expr (clauses.c:5012)
==2917549==    by 0x48FF08: evaluate_function (clauses.c:4519)
==2917549==    by 0x492054: simplify_function (clauses.c:4108)
==2917549==    by 0x4901FA: eval_const_expressions_mutator (clauses.c:2593)
==2917549==    by 0x432CDE: expression_tree_mutator_impl (nodeFuncs.c:3486)
==2917549==    by 0x48FFB9: eval_const_expressions_mutator (clauses.c:3727)
==2917549==  Address 0x82fa740 is 912 bytes inside a recently re-allocated block of size 8,192 alloc'd
==2917549==    at 0x4844818: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2917549==    by 0x71EF75: AllocSetContextCreateInternal (aset.c:444)
==2917549==    by 0x3D17AE: CreateExprContextInternal (execUtils.c:260)
==2917549==    by 0x3D1CA8: CreateExprContext (execUtils.c:310)
==2917549==    by 0x3D1ED8: MakePerTupleExprContext (execUtils.c:462)
==2917549==    by 0x48FDBF: evaluate_expr (clauses.c:5013)
==2917549==    by 0x48FF08: evaluate_function (clauses.c:4519)
==2917549==    by 0x492054: simplify_function (clauses.c:4108)
==2917549==    by 0x4901FA: eval_const_expressions_mutator (clauses.c:2593)
==2917549==    by 0x432CDE: expression_tree_mutator_impl (nodeFuncs.c:3486)
==2917549==    by 0x48FFB9: eval_const_expressions_mutator (clauses.c:3727)
==2917549==    by 0x432EFD: expression_tree_mutator_impl (nodeFuncs.c:3572)
==2917549== 
==2917549== VALGRINDERROR-END
{
   <insert_a_suppression_name_here>
   Memcheck:Addr1
   fun:px_crypt_shacrypt
   fun:run_crypt_sha
   fun:px_crypt
   fun:pg_crypt
   fun:ExecInterpExpr
   fun:ExecInterpExprStillValid
   fun:ExecEvalExprSwitchContext
   fun:evaluate_expr
   fun:evaluate_function
   fun:simplify_function
   fun:eval_const_expressions_mutator
   fun:expression_tree_mutator_impl
   fun:eval_const_expressions_mutator
}
**2917549** Valgrind detected 1 error(s) during execution of "SELECT crypt('', '$5$Szzz0yzz');"

Greetings,

Andres






^ permalink  raw  reply  [nested|flat] 10+ messages in thread


end of thread, other threads:[~2025-04-07 18:00 UTC | newest]

Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 21:55 [PATCH v1] Change pg_attribute_noreturn() to pg_noreturn, move to before return type. Andres Freund <[email protected]>
2021-01-18 20:37 [PATCH 06/18] One fewer (not one less) Justin Pryzby <[email protected]>
2025-03-11 16:58 Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
2025-04-05 17:22 Re: Modern SHA2- based password hashes for pgcrypto Alvaro Herrera <[email protected]>
2025-04-06 17:39 ` Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
2025-04-06 19:43 ` Re: Modern SHA2- based password hashes for pgcrypto Tom Lane <[email protected]>
2025-04-07 07:03   ` Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
2025-04-07 03:02 ` Re: Modern SHA2- based password hashes for pgcrypto Andres Freund <[email protected]>
2025-04-07 07:09   ` Re: Modern SHA2- based password hashes for pgcrypto Bernd Helmle <[email protected]>
2025-04-07 18:00     ` Re: Modern SHA2- based password hashes for pgcrypto Andres Freund <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox