From 2caf331cd4402d10440ea7cfad8875c192277273 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Wed, 9 Apr 2025 13:32:24 +0300 Subject: [PATCH v2 1/2] Use 'void *' for arbitrary buffers, 'uint8 *' for byte arrays A 'void *' argument suggests that the caller might pass an arbitrary struct, which is appropriate for functions like libc's read/write, or pq_sendbytes(). 'uint8 *' is more appropriate for byte arrays that have no structure, like the cancellation keys or SCRAM tokens. Change a few places to follow that convention. Discussion: https://www.postgresql.org/message-id/61be9e31-7b7d-49d5-bc11-721800d89d64@eisentraut.org --- src/backend/libpq/auth-scram.c | 2 +- src/backend/storage/ipc/procsignal.c | 6 +++--- src/backend/utils/init/globals.c | 2 +- src/common/base64.c | 16 ++++++++-------- src/common/scram-common.c | 10 +++++----- src/include/common/base64.h | 4 ++-- src/include/common/scram-common.h | 4 ++-- src/include/libpq/pqcomm.h | 2 +- src/include/miscadmin.h | 2 +- src/include/storage/procsignal.h | 4 ++-- src/interfaces/libpq/fe-auth-scram.c | 12 ++++++------ src/interfaces/libpq/fe-cancel.c | 2 +- src/interfaces/libpq/fe-misc.c | 10 +++++----- src/interfaces/libpq/fe-protocol3.c | 6 +++--- src/interfaces/libpq/libpq-int.h | 12 ++++++------ 15 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 26dd241efa9..f80333bb533 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -484,7 +484,7 @@ pg_be_scram_build_secret(const char *password) { char *prep_password; pg_saslprep_rc rc; - char saltbuf[SCRAM_DEFAULT_SALT_LEN]; + uint8 saltbuf[SCRAM_DEFAULT_SALT_LEN]; char *result; const char *errstr = NULL; diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c index a3c2cd12277..f9dafbcaf22 100644 --- a/src/backend/storage/ipc/procsignal.c +++ b/src/backend/storage/ipc/procsignal.c @@ -64,7 +64,7 @@ typedef struct { pg_atomic_uint32 pss_pid; int pss_cancel_key_len; /* 0 means no cancellation is possible */ - char pss_cancel_key[MAX_CANCEL_KEY_LENGTH]; + uint8 pss_cancel_key[MAX_CANCEL_KEY_LENGTH]; volatile sig_atomic_t pss_signalFlags[NUM_PROCSIGNALS]; slock_t pss_mutex; /* protects the above fields */ @@ -162,7 +162,7 @@ ProcSignalShmemInit(void) * Register the current process in the ProcSignal array */ void -ProcSignalInit(char *cancel_key, int cancel_key_len) +ProcSignalInit(const uint8 *cancel_key, int cancel_key_len) { ProcSignalSlot *slot; uint64 barrier_generation; @@ -728,7 +728,7 @@ procsignal_sigusr1_handler(SIGNAL_ARGS) * fields in the ProcSignal slots. */ void -SendCancelRequest(int backendPID, char *cancel_key, int cancel_key_len) +SendCancelRequest(int backendPID, const uint8 *cancel_key, int cancel_key_len) { Assert(backendPID != 0); diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 1847e7c85d3..92b0446b80c 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -50,7 +50,7 @@ pg_time_t MyStartTime; TimestampTz MyStartTimestamp; struct ClientSocket *MyClientSocket; struct Port *MyProcPort; -char MyCancelKey[MAX_CANCEL_KEY_LENGTH]; +uint8 MyCancelKey[MAX_CANCEL_KEY_LENGTH]; int MyCancelKeyLength = 0; int MyPMChildSlot; diff --git a/src/common/base64.c b/src/common/base64.c index 6028f413472..240461fe1e6 100644 --- a/src/common/base64.c +++ b/src/common/base64.c @@ -41,15 +41,15 @@ static const int8 b64lookup[128] = { /* * pg_b64_encode * - * Encode into base64 the given string. Returns the length of the encoded - * string, and -1 in the event of an error with the result buffer zeroed - * for safety. + * Encode the 'src' byte array into base64. Returns the length of the encoded + * string, and -1 in the event of an error with the result buffer zeroed for + * safety. */ int -pg_b64_encode(const char *src, int len, char *dst, int dstlen) +pg_b64_encode(const uint8 *src, int len, char *dst, int dstlen) { char *p; - const char *s, + const uint8 *s, *end = src + len; int pos = 2; uint32 buf = 0; @@ -59,7 +59,7 @@ pg_b64_encode(const char *src, int len, char *dst, int dstlen) while (s < end) { - buf |= (unsigned char) *s << (pos << 3); + buf |= *s << (pos << 3); pos--; s++; @@ -113,11 +113,11 @@ error: * buffer zeroed for safety. */ int -pg_b64_decode(const char *src, int len, char *dst, int dstlen) +pg_b64_decode(const char *src, int len, uint8 *dst, int dstlen) { const char *srcend = src + len, *s = src; - char *p = dst; + uint8 *p = dst; char c; int b = 0; uint32 buf = 0; diff --git a/src/common/scram-common.c b/src/common/scram-common.c index bfd2a340016..e47a6ebbaab 100644 --- a/src/common/scram-common.c +++ b/src/common/scram-common.c @@ -37,7 +37,7 @@ int scram_SaltedPassword(const char *password, pg_cryptohash_type hash_type, int key_length, - const char *salt, int saltlen, int iterations, + const uint8 *salt, int saltlen, int iterations, uint8 *result, const char **errstr) { int password_len = strlen(password); @@ -62,7 +62,7 @@ scram_SaltedPassword(const char *password, /* First iteration */ if (pg_hmac_init(hmac_ctx, (uint8 *) password, password_len) < 0 || - pg_hmac_update(hmac_ctx, (uint8 *) salt, saltlen) < 0 || + pg_hmac_update(hmac_ctx, salt, saltlen) < 0 || pg_hmac_update(hmac_ctx, (uint8 *) &one, sizeof(uint32)) < 0 || pg_hmac_final(hmac_ctx, Ui_prev, key_length) < 0) { @@ -207,7 +207,7 @@ scram_ServerKey(const uint8 *salted_password, */ char * scram_build_secret(pg_cryptohash_type hash_type, int key_length, - const char *salt, int saltlen, int iterations, + const uint8 *salt, int saltlen, int iterations, const char *password, const char **errstr) { uint8 salted_password[SCRAM_MAX_KEY_LEN]; @@ -290,7 +290,7 @@ scram_build_secret(pg_cryptohash_type hash_type, int key_length, *(p++) = '$'; /* stored key */ - encoded_result = pg_b64_encode((char *) stored_key, key_length, p, + encoded_result = pg_b64_encode(stored_key, key_length, p, encoded_stored_len); if (encoded_result < 0) { @@ -307,7 +307,7 @@ scram_build_secret(pg_cryptohash_type hash_type, int key_length, *(p++) = ':'; /* server key */ - encoded_result = pg_b64_encode((char *) server_key, key_length, p, + encoded_result = pg_b64_encode(server_key, key_length, p, encoded_server_len); if (encoded_result < 0) { diff --git a/src/include/common/base64.h b/src/include/common/base64.h index 3f74aa301f0..66cb57b017f 100644 --- a/src/include/common/base64.h +++ b/src/include/common/base64.h @@ -11,8 +11,8 @@ #define BASE64_H /* base 64 */ -pg_nodiscard extern int pg_b64_encode(const char *src, int len, char *dst, int dstlen); -pg_nodiscard extern int pg_b64_decode(const char *src, int len, char *dst, int dstlen); +pg_nodiscard extern int pg_b64_encode(const uint8 *src, int len, char *dst, int dstlen); +pg_nodiscard extern int pg_b64_decode(const char *src, int len, uint8 *dst, int dstlen); extern int pg_b64_enc_len(int srclen); extern int pg_b64_dec_len(int srclen); diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h index 46df94ef1d4..5ce055e0e27 100644 --- a/src/include/common/scram-common.h +++ b/src/include/common/scram-common.h @@ -51,7 +51,7 @@ extern int scram_SaltedPassword(const char *password, pg_cryptohash_type hash_type, int key_length, - const char *salt, int saltlen, int iterations, + const uint8 *salt, int saltlen, int iterations, uint8 *result, const char **errstr); extern int scram_H(const uint8 *input, pg_cryptohash_type hash_type, int key_length, uint8 *result, @@ -64,7 +64,7 @@ extern int scram_ServerKey(const uint8 *salted_password, uint8 *result, const char **errstr); extern char *scram_build_secret(pg_cryptohash_type hash_type, int key_length, - const char *salt, int saltlen, int iterations, + const uint8 *salt, int saltlen, int iterations, const char *password, const char **errstr); #endif /* SCRAM_COMMON_H */ diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index d11069cf8dc..f04ca135653 100644 --- a/src/include/libpq/pqcomm.h +++ b/src/include/libpq/pqcomm.h @@ -141,7 +141,7 @@ typedef struct CancelRequestPacket /* Note that each field is stored in network byte order! */ MsgType cancelRequestCode; /* code to identify a cancel request */ uint32 backendPID; /* PID of client's backend */ - char cancelAuthCode[FLEXIBLE_ARRAY_MEMBER]; /* secret key to + uint8 cancelAuthCode[FLEXIBLE_ARRAY_MEMBER]; /* secret key to * authorize cancel */ } CancelRequestPacket; diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 72f5655fb34..258e5624ad0 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -192,7 +192,7 @@ extern PGDLLIMPORT pg_time_t MyStartTime; extern PGDLLIMPORT TimestampTz MyStartTimestamp; extern PGDLLIMPORT struct Port *MyProcPort; extern PGDLLIMPORT struct Latch *MyLatch; -extern PGDLLIMPORT char MyCancelKey[]; +extern PGDLLIMPORT uint8 MyCancelKey[]; extern PGDLLIMPORT int MyCancelKeyLength; extern PGDLLIMPORT int MyPMChildSlot; diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h index cfe14631445..345d5a0ecb1 100644 --- a/src/include/storage/procsignal.h +++ b/src/include/storage/procsignal.h @@ -73,10 +73,10 @@ typedef enum extern Size ProcSignalShmemSize(void); extern void ProcSignalShmemInit(void); -extern void ProcSignalInit(char *cancel_key, int cancel_key_len); +extern void ProcSignalInit(const uint8 *cancel_key, int cancel_key_len); extern int SendProcSignal(pid_t pid, ProcSignalReason reason, ProcNumber procNumber); -extern void SendCancelRequest(int backendPID, char *cancel_key, int cancel_key_len); +extern void SendCancelRequest(int backendPID, const uint8 *cancel_key, int cancel_key_len); extern uint64 EmitProcSignalBarrier(ProcSignalBarrierType type); extern void WaitForProcSignalBarrier(uint64 generation); diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index fe18615197f..3babbc8d522 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -70,7 +70,7 @@ typedef struct /* These come from the server-first message */ char *server_first_message; - char *salt; + uint8 *salt; int saltlen; int iterations; char *nonce; @@ -350,7 +350,7 @@ static char * build_client_first_message(fe_scram_state *state) { PGconn *conn = state->conn; - char raw_nonce[SCRAM_RAW_NONCE_LEN + 1]; + uint8 raw_nonce[SCRAM_RAW_NONCE_LEN + 1]; char *result; int channel_info_len; int encoded_len; @@ -477,7 +477,7 @@ build_client_final_message(fe_scram_state *state) char *cbind_data = NULL; size_t cbind_data_len = 0; size_t cbind_header_len; - char *cbind_input; + uint8 *cbind_input; size_t cbind_input_len; int encoded_cbind_len; @@ -574,7 +574,7 @@ build_client_final_message(fe_scram_state *state) encoded_len = pg_b64_enc_len(state->key_length); if (!enlargePQExpBuffer(&buf, encoded_len)) goto oom_error; - encoded_len = pg_b64_encode((char *) client_proof, + encoded_len = pg_b64_encode(client_proof, state->key_length, buf.data + buf.len, encoded_len); @@ -694,7 +694,7 @@ read_server_final_message(fe_scram_state *state, char *input) { PGconn *conn = state->conn; char *encoded_server_signature; - char *decoded_server_signature; + uint8 *decoded_server_signature; int server_signature_len; state->server_final_message = strdup(input); @@ -916,7 +916,7 @@ pg_fe_scram_build_secret(const char *password, int iterations, const char **errs { char *prep_password; pg_saslprep_rc rc; - char saltbuf[SCRAM_DEFAULT_SALT_LEN]; + uint8 saltbuf[SCRAM_DEFAULT_SALT_LEN]; char *result; /* diff --git a/src/interfaces/libpq/fe-cancel.c b/src/interfaces/libpq/fe-cancel.c index e84e64bf2a7..d51a2d2f70a 100644 --- a/src/interfaces/libpq/fe-cancel.c +++ b/src/interfaces/libpq/fe-cancel.c @@ -463,7 +463,7 @@ PQsendCancelRequest(PGconn *cancelConn) memset(&req, 0, offsetof(CancelRequestPacket, cancelAuthCode)); req.cancelRequestCode = (MsgType) pg_hton32(CANCEL_REQUEST_CODE); req.backendPID = pg_hton32(cancelConn->be_pid); - if (pqPutnchar((char *) &req, offsetof(CancelRequestPacket, cancelAuthCode), cancelConn)) + if (pqPutnchar(&req, offsetof(CancelRequestPacket, cancelAuthCode), cancelConn)) return STATUS_ERROR; if (pqPutnchar(cancelConn->be_cancel_key, cancelConn->be_cancel_key_len, cancelConn)) return STATUS_ERROR; diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index d78445c70af..c74fe404bbe 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -67,7 +67,7 @@ PQlibVersion(void) /* - * pqGetc: get 1 character from the connection + * pqGetc: read 1 character from the connection * * All these routines return 0 on success, EOF on error. * Note that for the Get routines, EOF only means there is not enough @@ -100,7 +100,7 @@ pqPutc(char c, PGconn *conn) /* * pqGets[_append]: - * get a null-terminated string from the connection, + * read a null-terminated string from the connection, * and store it in an expansible PQExpBuffer. * If we run out of memory, all of the string is still read, * but the excess characters are silently discarded. @@ -159,10 +159,10 @@ pqPuts(const char *s, PGconn *conn) /* * pqGetnchar: - * get a string of exactly len bytes in buffer s, no null termination + * read exactly len bytes in buffer s, no null termination */ int -pqGetnchar(char *s, size_t len, PGconn *conn) +pqGetnchar(void *s, size_t len, PGconn *conn) { if (len > (size_t) (conn->inEnd - conn->inCursor)) return EOF; @@ -199,7 +199,7 @@ pqSkipnchar(size_t len, PGconn *conn) * write exactly len bytes to the current message */ int -pqPutnchar(const char *s, size_t len, PGconn *conn) +pqPutnchar(const void *s, size_t len, PGconn *conn) { if (pqPutMsgBytes(s, len, conn)) return EOF; diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index d85910f41fc..289d1beca75 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -1532,7 +1532,7 @@ getParameterStatus(PGconn *conn) static int getBackendKeyData(PGconn *conn, int msgLength) { - uint8 cancel_key_len; + int cancel_key_len; if (conn->be_cancel_key) { @@ -2121,7 +2121,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid, } else { - if (pqPutnchar((char *) args[i].u.ptr, args[i].len, conn)) + if (pqPutnchar(args[i].u.ptr, args[i].len, conn)) return NULL; } } @@ -2215,7 +2215,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid, } else { - if (pqGetnchar((char *) result_buf, + if (pqGetnchar(result_buf, *actual_result_len, conn)) continue; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 9369c217fb5..a6cfd7f5c9d 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -539,16 +539,16 @@ struct pg_conn * tried host */ bool send_appname; /* okay to send application_name? */ size_t scram_client_key_len; - void *scram_client_key_binary; /* binary SCRAM client key */ + uint8 *scram_client_key_binary; /* binary SCRAM client key */ size_t scram_server_key_len; - void *scram_server_key_binary; /* binary SCRAM server key */ + uint8 *scram_server_key_binary; /* binary SCRAM server key */ ProtocolVersion min_pversion; /* protocol version to request */ ProtocolVersion max_pversion; /* protocol version to request */ /* Miscellaneous stuff */ int be_pid; /* PID of backend --- needed for cancels */ - char *be_cancel_key; /* query cancellation key and its length */ - uint16 be_cancel_key_len; + int be_cancel_key_len; + uint8 *be_cancel_key; /* query cancellation key */ pgParameterStatus *pstatus; /* ParameterStatus data */ int client_encoding; /* encoding id */ bool std_strings; /* standard_conforming_strings */ @@ -787,9 +787,9 @@ extern int pqPutc(char c, PGconn *conn); extern int pqGets(PQExpBuffer buf, PGconn *conn); extern int pqGets_append(PQExpBuffer buf, PGconn *conn); extern int pqPuts(const char *s, PGconn *conn); -extern int pqGetnchar(char *s, size_t len, PGconn *conn); +extern int pqGetnchar(void *s, size_t len, PGconn *conn); extern int pqSkipnchar(size_t len, PGconn *conn); -extern int pqPutnchar(const char *s, size_t len, PGconn *conn); +extern int pqPutnchar(const void *s, size_t len, PGconn *conn); extern int pqGetInt(int *result, size_t bytes, PGconn *conn); extern int pqPutInt(int value, size_t bytes, PGconn *conn); extern int pqPutMsgStart(char msg_type, PGconn *conn); -- 2.39.5