Re: [pgpool-hackers: 4583] Fix time_t warnings on OpenBSD

Hello Martijn,

Sorry for not responding earlier. (I have replied on the new
pgpool-hackers mailing list. We have migrated from pgpool.net ML to
PostgreSQL.org ML in July 2025).

I have read through your first patch (posted on 03 May 2025) again and
have started to think that upcasting time calculations to long long is
a good idea. So I rebased the patch against master branch. See
attached patch.

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

From: Martijn van Duren <pgpool(at)list(dot)imperialat(dot)at>
Subject: [pgpool-hackers: 4583] Fix time_t warnings on OpenBSD
Date: Sat, 03 May 2025 16:20:09 +0200
Message-ID: <050e2e86b048f6371d9ee65a5195402678bec50b(dot)camel(at)list(dot)imperialat(dot)at>

> 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.
>
> Mildly tested.
> Compile tested on:
> - AMD64 OpenBSD-current clang 16.0.6
> - AMD64 OpenBSD-current gcc 11.2.0
> - AMD64 Debian trixie/sid gcc 14.2.0
>
> martijn@
>
> [0] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
>
> diff 0ac97e82e90b360632fc6d479ff331375fb619f7 1967584b6bc2b01f6749a32f342d04ba38715512
> commit - 0ac97e82e90b360632fc6d479ff331375fb619f7
> commit + 1967584b6bc2b01f6749a32f342d04ba38715512
> blob - 67cc0255a789499bfa83abd0369af22436387221
> blob + 7e24382e70282401aca03400722368096784fdad
> --- 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_llong_value_for_key(json_value * source, const char *key, long long *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 + dd788da7f963dfed71455deb6428e4de66990e98
> --- 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;
> + long long longVal;
> } data;
> } WDGenericData;
>
> blob - 7b53999ee3342459cb5beb43007dd054b5f84822
> blob + 4ad49101647be6a30804da1aa969e83262781c61
> --- 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,
> + long long *seconds_since_node_startup,
> + long long *seconds_since_current_state,
> int *quorumStatus,
> int *standbyNodesCount,
> bool *escalated);
> blob - 2060867322a0105103161e72c71d98e8886a4961
> blob + 21c67a83898266559463650ab37e516cfa1aba27
> --- 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 time_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 + ddd892afd9c4d1ed497992ff59bae70090259cbb
> --- 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), "%lld", (long long)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), "%lld", (long long)bi->status_changed_time);
>
> pcp_write(frontend, "i", 1);
> wsize = htonl(sizeof(code) +
> blob - d7e1857a42350065b98ff567a349003ec7102c3d
> blob + 76937d681ee0e0c55a2ea8463e46edf46d0d4474
> --- 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: %lld",
> CONNECTION_SLOT(p, main_node_id)->sp->user,
> CONNECTION_SLOT(p, main_node_id)->sp->database,
> - CONNECTION_SLOT(p, main_node_id)->closetime)));
> + (long long)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 %lld", (long long)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 %lld", (long long)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: %lld",
> + (long long)(MAIN_CONNECTION(p)->closetime + pool_config->connection_life_time))));
>
> if (now >= (MAIN_CONNECTION(p)->closetime + pool_config->connection_life_time))
> {
> blob - cc702074d38c1879dbb3ee77cd5af0e66518d642
> blob + 616aa085a7845fcbea5a11b185cd41bad0b3ff41
> --- 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 = %lld", (long long)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 = %lld", (long long)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: %lld timestamp: %lld",
> + (long long)now, (long long)(cih->timestamp + cih->expire))));
> pool_delete_item_shmem_cache(c);
> return NULL;
> }
> blob - 319c8fdbfe62e0b6ed071b53dd12f6591ab53610
> blob + 9336fde0f2aab7fca098831da63c92e4775be38a
> --- 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_llong_value_for_key(json_value * source, const char *key, long long *value)
> {
> json_value *jNode;
>
> blob - 71f871bc4222600d13d217bb6fc868c8406c7863
> blob + 59109994ac929dc4791688dfe9773e27b731b853
> --- 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 %lld", (long long)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 + 1701cb5417d8f3e2f9a7f3756fccdb8f3917fd1a
> --- 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:%lld expiration time:%lld", dbname, table, (long long)now, (long long)relcache->cache[i].expire)));
>
> relcache->cache[i].refcnt = 0;
> break;
> blob - faab4e6e5b8cd07c20226f3ed81b6cff7901740d
> blob + 83b78baa88ccc991b20989e540657b09ddbd8a76
> --- 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;
> + long long seconds_since_node_startup;
> + long long seconds_since_current_state;
> int quorum_status;
> int standby_nodes_count;
> bool escalated;
> blob - 145955865d1941a877ee33999777b4943972d43d
> blob + c5cd1dcff18aa8beb52d9fbaa51508e118fffb76
> --- 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;
> + long long longVal;
>
> - if (json_get_long_value_for_key(root, WD_JSON_KEY_VALUE_DATA, &longVal))
> + if (json_get_llong_value_for_key(root, WD_JSON_KEY_VALUE_DATA, &longVal))
> {
> ereport(WARNING,
> (errmsg("get runtime variable value from watchdog failed"),
> blob - c13e12b6c1b6de597dd6bd21ff1a2ddd033fdf5c
> blob + eab204fa826094edeed3e1cda4afc925c4a060a5
> --- 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=%lld tv_usec=%lld from=%s",
> + (long long)pkt->send_time.tv_sec, (long long)pkt->send_time.tv_usec, pkt->from);
>
> return len;
> }
> blob - 474fc37a495ea541cb1cc8c7a05ed8161a607e29
> blob + 602e3bfbefcd2ef8c630ef13301feb5687637914
> --- 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;
> + long long 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_llong_value_for_key(root, "StartupTimeSecs", &longVal))
> {
> bool escalated;
> - long seconds_since_node_startup;
> - long seconds_since_current_state;
> + long long seconds_since_node_startup;
> + long long seconds_since_current_state;
> struct timeval current_time;
>
> + wdNode->startup_time.tv_sec = longVal;
> gettimeofday(&current_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_llong_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_llong_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,
> + long long *seconds_since_node_startup,
> + long long *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_llong_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_llong_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;
>
> _______________________________________________
> pgpool-hackers mailing list
> pgpool-hackers(at)pgpool(dot)net
> http://www.pgpool.net/mailman/listinfo/pgpool-hackers