[pgpool-hackers: 4586] Re: Fix time_t warnings on OpenBSD
Martijn van Duren
pgpool at list.imperialat.at
Mon May 5 16:12:26 JST 2025
On Mon, 2025-05-05 at 14:12 +0900, Tatsuo Ishii wrote:
> > Hello all,
> >
> > On OpenBSD time_t is defined as __int64_t (which basically always
> > results into long long). This causes a lots of compile time warnings
> > since pgpool's code expects time_t to be a simple long in a lot of
> > places.
> >
> > POSIX defines time_t as "time_t shall be an integer type". So no
> > definition of signedness or size. Since it would be dumb to make it
> > unsigned (would be no pre-1970) and I know of no unsigned time_t
> > based system I reckon it's safe to assume time_t to be always signed.
> > For the size I would like to propose to always upcast to long long.
> > This way times can't get truncated and for the places where time_t
> > is actually used as a time and not a time diff this would allow pgpool
> > to keep working correctly post Y2038 on 64 bit clean time_t systems
> > post Y2038 (e.g. i386 OpenBSD).
> >
> > While here I also changed json_get_long_value_for_key to
> > json_get_llong_value_for_key and changed the parameter to long long.
> > This makes it more in line _json_value's integer, which is defined as
> > int64. This should also give 32 bit platforms proper retrieval of the
> > max value of an integer and may or may not solve some weird integer
> > overflow issues.
>
> Can we upcast using "int64_t" instead of "long long"? I am hesitate
> to use "long long" since its bit width may not be 64bit on certain
> systems.
I'm not opposed, although I dislike the PRIxYY format, and C99 specifies it
to be at least 64 bits in size, and having a bigger size here wouldn't hurt.
But if the choice is to go with the *int**_t route, maybe all
other instances of the custom fixed width integers should also be
changed? This format is POSIX-specified, so I see no reason to
keep these custom formats.
Diff below only compile tested on OpenBSD AMD64 with clang.
>
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
diff 0ac97e82e90b360632fc6d479ff331375fb619f7 7649a56611ed05d0a51caf4f39e620e1d08577cd
commit - 0ac97e82e90b360632fc6d479ff331375fb619f7
commit + 7649a56611ed05d0a51caf4f39e620e1d08577cd
blob - 43f29a595abff8f7883b7c6e0087a518fa7e30ff
blob + 585cba3898f9424b83fce47e086a780ef9bea235
--- src/include/pool_type.h
+++ src/include/pool_type.h
@@ -29,7 +29,9 @@
#include "config.h"
#include <sys/types.h>
#include <sys/socket.h>
+#include <inttypes.h>
#include <stddef.h>
+#include <stdint.h>
#include "libpq-fe.h"
/* Define common boolean type. C++ and BEOS already has it so exclude them. */
#ifdef c_plusplus
blob - 67cc0255a789499bfa83abd0369af22436387221
blob + f321dad7d09568a0d74a95a443a2a74fd25d454f
--- src/include/utils/json.h
+++ src/include/utils/json.h
@@ -313,7 +313,7 @@ extern "C"
/* pgpool-II extensions */
json_value *json_get_value_for_key(json_value * source, const char *key);
int json_get_int_value_for_key(json_value * source, const char *key, int *value);
-int json_get_long_value_for_key(json_value * source, const char *key, long *value);
+int json_get_int64_value_for_key(json_value * source, const char *key, int64_t *value);
char *json_get_string_value_for_key(json_value * source, const char *key);
int json_get_bool_value_for_key(json_value * source, const char *key, bool *value);
blob - 34c5b9ac6615bd990c829b78478a901bd91ce431
blob + 9e2220ed9661809c05c1f7fb1ba25124219a0276
--- src/include/watchdog/wd_commands.h
+++ src/include/watchdog/wd_commands.h
@@ -52,7 +52,7 @@ typedef struct WDGenericData
char *stringVal;
int intVal;
bool boolVal;
- long longVal;
+ int64_t longVal;
} data;
} WDGenericData;
blob - 7b53999ee3342459cb5beb43007dd054b5f84822
blob + 0e8f7732a7aba734b20dc50f95e90ad4d2a1c1c4
--- src/include/watchdog/wd_json_data.h
+++ src/include/watchdog/wd_json_data.h
@@ -51,8 +51,8 @@ extern bool parse_node_status_json(char *json_data, in
extern bool parse_beacon_message_json(char *json_data, int data_len, int *state,
- long *seconds_since_node_startup,
- long *seconds_since_current_state,
+ int64_t *seconds_since_node_startup,
+ int64_t *seconds_since_current_state,
int *quorumStatus,
int *standbyNodesCount,
bool *escalated);
blob - 2060867322a0105103161e72c71d98e8886a4961
blob + 6044e6a5952833183f03324fa6e8214d1743c0af
--- src/main/pgpool_logger.c
+++ src/main/pgpool_logger.c
@@ -50,7 +50,7 @@
#include "main/pgpool_logger.h"
#define DEVNULL "/dev/null"
-typedef int64 pg_time_t;
+typedef int64_t pg_time_t;
/*
* We read() into a temp buffer twice as big as a chunk, so that any fragment
* left after processing can be moved down to the front and we'll still have
blob - de2658d5e2b657b75c37ecd3025fdaae4956d7e9
blob + 35620b9ab47f98d5c6dca795873fad8d2b2d03e9
--- src/pcp_con/pcp_worker.c
+++ src/pcp_con/pcp_worker.c
@@ -933,9 +933,9 @@ inform_node_info(PCP_CONNECTION * frontend, char *buf)
snprintf(standby_delay_by_time_str, sizeof(standby_delay_by_time_str), "%d", bi->standby_delay_by_time);
- snprintf(standby_delay_str, sizeof(standby_delay_str), UINT64_FORMAT, bi->standby_delay);
+ snprintf(standby_delay_str, sizeof(standby_delay_str), "%"PRId64, (int64_t)bi->standby_delay);
- snprintf(status_changed_time_str, sizeof(status_changed_time_str), UINT64_FORMAT, bi->status_changed_time);
+ snprintf(status_changed_time_str, sizeof(status_changed_time_str), "%"PRId64, (int64_t)bi->status_changed_time);
pcp_write(frontend, "i", 1);
wsize = htonl(sizeof(code) +
blob - d7e1857a42350065b98ff567a349003ec7102c3d
blob + 91d2fed711e4bb5a25d7dfb0f2eef590e6d5272a
--- src/protocol/pool_connection_pool.c
+++ src/protocol/pool_connection_pool.c
@@ -299,10 +299,10 @@ pool_create_cp(void)
ereport(DEBUG1,
(errmsg("creating connection pool"),
- errdetail("user: %s database: %s closetime: %ld",
+ errdetail("user: %s database: %s closetime: %"PRId64,
CONNECTION_SLOT(p, main_node_id)->sp->user,
CONNECTION_SLOT(p, main_node_id)->sp->database,
- CONNECTION_SLOT(p, main_node_id)->closetime)));
+ (int64_t)CONNECTION_SLOT(p, main_node_id)->closetime)));
if (CONNECTION_SLOT(p, main_node_id)->closetime < closetime)
{
@@ -363,7 +363,7 @@ pool_connection_pool_timer(POOL_CONNECTION_POOL * back
ereport(DEBUG1,
(errmsg("setting backend connection close timer"),
- errdetail("close time %ld", time(NULL))));
+ errdetail("close time %"PRId64, (int64_t)time(NULL))));
/* Set connection close time */
for (i = 0; i < NUM_BACKENDS; i++)
@@ -421,7 +421,7 @@ pool_backend_timer(void)
now = time(NULL);
ereport(DEBUG1,
- (errmsg("backend timer handler called at %ld", now)));
+ (errmsg("backend timer handler called at %"PRId64, (int64_t)now)));
for (i = 0; i < pool_config->max_pool; i++, p++)
{
@@ -439,8 +439,8 @@ pool_backend_timer(void)
ereport(DEBUG1,
(errmsg("backend timer handler called"),
- errdetail("expire time: %ld",
- MAIN_CONNECTION(p)->closetime + pool_config->connection_life_time)));
+ errdetail("expire time: %"PRId64,
+ (int64_t)(MAIN_CONNECTION(p)->closetime + pool_config->connection_life_time))));
if (now >= (MAIN_CONNECTION(p)->closetime + pool_config->connection_life_time))
{
blob - cc702074d38c1879dbb3ee77cd5af0e66518d642
blob + 580980595d3294a636a3d15218350a9bd4f256ca
--- src/query_cache/pool_memqcache.c
+++ src/query_cache/pool_memqcache.c
@@ -280,7 +280,7 @@ pool_commit_cache(POOL_CONNECTION_POOL * backend, char
memqcache_expire = pool_config->memqcache_expire;
ereport(DEBUG1,
(errmsg("committing SELECT results to cache storage"),
- errdetail("memqcache_expire = %ld", memqcache_expire)));
+ errdetail("memqcache_expire = %"PRId64, (int64_t)memqcache_expire)));
if (pool_is_shmem_cache())
{
@@ -390,7 +390,7 @@ pool_catalog_commit_cache(POOL_CONNECTION_POOL * backe
memqcache_expire = pool_config->relcache_expire;
ereport(DEBUG1,
(errmsg("committing relation cache to cache storage"),
- errdetail("memqcache_expire = %ld", memqcache_expire)));
+ errdetail("memqcache_expire = %"PRId64, (int64_t)memqcache_expire)));
if (pool_is_shmem_cache())
{
@@ -2887,8 +2887,8 @@ static POOL_CACHEID * pool_find_item_on_shmem_cache(PO
{
ereport(DEBUG1,
(errmsg("memcache finding item"),
- errdetail("cache expired: now: %ld timestamp: %ld",
- now, cih->timestamp + cih->expire)));
+ errdetail("cache expired: now: %"PRId64" timestamp: %"PRId64,
+ (int64_t)now, (int64_t)(cih->timestamp + cih->expire))));
pool_delete_item_shmem_cache(c);
return NULL;
}
blob - 319c8fdbfe62e0b6ed071b53dd12f6591ab53610
blob + fa420ccd5b778652c5223361794a36b9f1439b51
--- src/utils/json.c
+++ src/utils/json.c
@@ -1191,7 +1191,7 @@ json_get_int_value_for_key(json_value * source, const
}
int
-json_get_long_value_for_key(json_value * source, const char *key, long *value)
+json_get_int64_value_for_key(json_value * source, const char *key, int64_t *value)
{
json_value *jNode;
blob - 71f871bc4222600d13d217bb6fc868c8406c7863
blob + 97163aaa45c1eb675ea28330bcdff95561194a00
--- src/utils/pool_process_reporting.c
+++ src/utils/pool_process_reporting.c
@@ -2052,7 +2052,7 @@ get_health_check_stats(int *nrows)
/* status last changed */
t = bi->status_changed_time;
- ereport(LOG,(errmsg("status_changed_time %ld", t)));
+ ereport(LOG,(errmsg("status_changed_time %"PRId64, (int64_t)t)));
strftime(stats[i].last_status_change, POOLCONFIG_MAXDATELEN, "%F %T", localtime(&t));
snprintf(stats[i].total_count, POOLCONFIG_MAXLONGCOUNTLEN, UINT64_FORMAT, health_check_stats[i].total_count);
blob - 32362fc6baae4f85be6953ef6a1bd70f2600211d
blob + ccd51a49fe141d78f1536b7496aed81eebc135de
--- src/utils/pool_relcache.c
+++ src/utils/pool_relcache.c
@@ -187,7 +187,7 @@ pool_search_relcache(POOL_RELCACHE * relcache, POOL_CO
{
ereport(DEBUG1,
(errmsg("searching relcache"),
- errdetail("relcache for database:%s table:%s expired. now:%ld expiration time:%ld", dbname, table, now, relcache->cache[i].expire)));
+ errdetail("relcache for database:%s table:%s expired. now:%"PRId64" expiration time:%"PRId64, dbname, table, (int64_t)now, (int64_t)relcache->cache[i].expire)));
relcache->cache[i].refcnt = 0;
break;
blob - faab4e6e5b8cd07c20226f3ed81b6cff7901740d
blob + ca6467d30c2cb88e10897d173e94d37c2e147be8
--- src/watchdog/watchdog.c
+++ src/watchdog/watchdog.c
@@ -6656,8 +6656,8 @@ watchdog_state_machine_nw_isolation(WD_EVENTS event, W
static bool
beacon_message_received_from_node(WatchdogNode * wdNode, WDPacketData * pkt)
{
- long seconds_since_node_startup;
- long seconds_since_current_state;
+ int64_t seconds_since_node_startup;
+ int64_t seconds_since_current_state;
int quorum_status;
int standby_nodes_count;
bool escalated;
blob - 145955865d1941a877ee33999777b4943972d43d
blob + eff2f7ff81e7a8e46a77c3f2c93414b345444e65
--- src/watchdog/wd_commands.c
+++ src/watchdog/wd_commands.c
@@ -193,9 +193,9 @@ get_wd_runtime_variable_value(char* wd_authkey, char *
case VALUE_DATA_TYPE_LONG:
{
- long longVal;
+ int64_t longVal;
- if (json_get_long_value_for_key(root, WD_JSON_KEY_VALUE_DATA, &longVal))
+ if (json_get_int64_value_for_key(root, WD_JSON_KEY_VALUE_DATA, &longVal))
{
ereport(WARNING,
(errmsg("get runtime variable value from watchdog failed"),
blob - c13e12b6c1b6de597dd6bd21ff1a2ddd033fdf5c
blob + 55526cb7746a5c95b2d34fd9598c6b2f6e654331
--- src/watchdog/wd_heartbeat.c
+++ src/watchdog/wd_heartbeat.c
@@ -854,8 +854,8 @@ packet_to_string_hb(WdHbPacket * pkt, char *str, int m
{
int len;
- len = snprintf(str, maxlen, "tv_sec=%ld tv_usec=%ld from=%s",
- pkt->send_time.tv_sec, pkt->send_time.tv_usec, pkt->from);
+ len = snprintf(str, maxlen, "tv_sec=%"PRId64" tv_usec=%"PRId64" from=%s",
+ (int64_t)pkt->send_time.tv_sec, (int64_t)pkt->send_time.tv_usec, pkt->from);
return len;
}
blob - 474fc37a495ea541cb1cc8c7a05ed8161a607e29
blob + 5a86a8e66b46a28b7434806459d6ab9d598a0269
--- src/watchdog/wd_json_data.c
+++ src/watchdog/wd_json_data.c
@@ -530,6 +530,7 @@ get_watchdog_node_from_json(char *json_data, int data_
{
json_value *root = NULL;
char *ptr;
+ int64_t longVal;
WatchdogNode *wdNode = palloc0(sizeof(WatchdogNode));
root = json_parse(json_data, data_len);
@@ -537,19 +538,20 @@ get_watchdog_node_from_json(char *json_data, int data_
if (root == NULL || root->type != json_object)
goto ERROR_EXIT;
- if (json_get_long_value_for_key(root, "StartupTimeSecs", &wdNode->startup_time.tv_sec))
+ if (json_get_int64_value_for_key(root, "StartupTimeSecs", &longVal))
{
bool escalated;
- long seconds_since_node_startup;
- long seconds_since_current_state;
+ int64_t seconds_since_node_startup;
+ int64_t seconds_since_current_state;
struct timeval current_time;
+ wdNode->startup_time.tv_sec = longVal;
gettimeofday(¤t_time, NULL);
/* The new version does not have StartupTimeSecs Key */
- if (json_get_long_value_for_key(root, "SecondsSinceStartup", &seconds_since_node_startup))
+ if (json_get_int64_value_for_key(root, "SecondsSinceStartup", &seconds_since_node_startup))
goto ERROR_EXIT;
- if (json_get_long_value_for_key(root, "SecondsSinceCurrentState", &seconds_since_current_state))
+ if (json_get_int64_value_for_key(root, "SecondsSinceCurrentState", &seconds_since_current_state))
goto ERROR_EXIT;
if (json_get_bool_value_for_key(root, "Escalated", &escalated))
goto ERROR_EXIT;
@@ -640,8 +642,8 @@ ERROR_EXIT:
bool
parse_beacon_message_json(char *json_data, int data_len,
int *state,
- long *seconds_since_node_startup,
- long *seconds_since_current_state,
+ int64_t *seconds_since_node_startup,
+ int64_t *seconds_since_current_state,
int *quorumStatus,
int *standbyNodesCount,
bool *escalated)
@@ -655,9 +657,9 @@ parse_beacon_message_json(char *json_data, int data_le
if (json_get_int_value_for_key(root, "State", state))
goto ERROR_EXIT;
- if (json_get_long_value_for_key(root, "SecondsSinceStartup", seconds_since_node_startup))
+ if (json_get_int64_value_for_key(root, "SecondsSinceStartup", seconds_since_node_startup))
goto ERROR_EXIT;
- if (json_get_long_value_for_key(root, "SecondsSinceCurrentState", seconds_since_current_state))
+ if (json_get_int64_value_for_key(root, "SecondsSinceCurrentState", seconds_since_current_state))
goto ERROR_EXIT;
if (json_get_bool_value_for_key(root, "Escalated", escalated))
goto ERROR_EXIT;
More information about the pgpool-hackers
mailing list