INT64_MIN and _MAX

Started by Andrew Gierthalmost 11 years ago13 messages

Andrew Gierth

andrew@tao11.riddles.org.uk

almost 11 years ago

A couple of places (adt/timestamp.c and pgbench.c) have this:

#ifndef INT64_MAX
#define INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#endif

#ifndef INT64_MIN
#define INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
#endif

On the other hand, int8.c uses the INT64_MIN expression directly inline.

On the third hand, INT64_MIN etc. would typically be defined in stdint.h
if it exists.

So wouldn't it make more sense to move these definitions into c.h and
standardize their usage?

--
Andrew (irc:RhodiumToad)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Petr Jelinek

petr@2ndquadrant.com

almost 11 years ago

In reply to: Andrew Gierth (#1)

Re: INT64_MIN and _MAX

On 21/03/15 23:45, Andrew Gierth wrote:

A couple of places (adt/timestamp.c and pgbench.c) have this:

#ifndef INT64_MAX
#define INT64_MAX INT64CONST(0x7FFFFFFFFFFFFFFF)
#endif

#ifndef INT64_MIN
#define INT64_MIN (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
#endif

On the other hand, int8.c uses the INT64_MIN expression directly inline.

On the third hand, INT64_MIN etc. would typically be defined in stdint.h
if it exists.

So wouldn't it make more sense to move these definitions into c.h and
standardize their usage?

I was thinking the same when I've seen Peter's version of Numeric
abbreviations patch. So +1 for that.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Gierth

andrew@tao11.riddles.org.uk

almost 11 years ago

In reply to: Petr Jelinek (#2)

1 attachment(s)

Re: INT64_MIN and _MAX

"Petr" == Petr Jelinek <petr@2ndquadrant.com> writes:

So wouldn't it make more sense to move these definitions into c.h and
standardize their usage?

Petr> I was thinking the same when I've seen Peter's version of Numeric
Petr> abbreviations patch. So +1 for that.

Suggested patch attached.

--
Andrew (irc:RhodiumToad)

Attachments:

int64_minmax.patchtext/x-patchDownload

diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index b9c2b49..d472d49 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -153,7 +153,7 @@ ts_dist(PG_FUNCTION_ARGS)
 		p->day = INT_MAX;
 		p->month = INT_MAX;
 #ifdef HAVE_INT64_TIMESTAMP
-		p->time = INT64CONST(0x7FFFFFFFFFFFFFFF);
+		p->time = INT64_MAX;
 #else
 		p->time = DBL_MAX;
 #endif
@@ -181,7 +181,7 @@ tstz_dist(PG_FUNCTION_ARGS)
 		p->day = INT_MAX;
 		p->month = INT_MAX;
 #ifdef HAVE_INT64_TIMESTAMP
-		p->time = INT64CONST(0x7FFFFFFFFFFFFFFF);
+		p->time = INT64_MAX;
 #else
 		p->time = DBL_MAX;
 #endif
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..822adfd 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -49,10 +49,6 @@
 #include <sys/resource.h>		/* for getrlimit */
 #endif
 
-#ifndef INT64_MAX
-#define INT64_MAX	INT64CONST(0x7FFFFFFFFFFFFFFF)
-#endif
-
 #ifndef M_PI
 #define M_PI 3.14159265358979323846
 #endif
@@ -453,7 +449,7 @@ strtoint64(const char *str)
 		 */
 		if (strncmp(ptr, "9223372036854775808", 19) == 0)
 		{
-			result = -INT64CONST(0x7fffffffffffffff) - 1;
+			result = INT64_MIN;
 			ptr += 19;
 			goto gotdigits;
 		}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e2d187f..656d55b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1408,7 +1408,7 @@ WALInsertLockAcquireExclusive(void)
 	{
 		LWLockAcquireWithVar(&WALInsertLocks[i].l.lock,
 							 &WALInsertLocks[i].l.insertingAt,
-							 UINT64CONST(0xFFFFFFFFFFFFFFFF));
+							 UINT64_MAX);
 	}
 	LWLockAcquireWithVar(&WALInsertLocks[i].l.lock,
 						 &WALInsertLocks[i].l.insertingAt,
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 56d909a..b3a1191 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -78,7 +78,7 @@ scanint8(const char *str, bool errorOK, int64 *result)
 		 */
 		if (strncmp(ptr, "9223372036854775808", 19) == 0)
 		{
-			tmp = -INT64CONST(0x7fffffffffffffff) - 1;
+			tmp = INT64_MIN;
 			ptr += 19;
 			goto gotdigits;
 		}
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index d77799a..585da1e 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -190,7 +190,7 @@ pg_lltoa(int64 value, char *a)
 	 * Avoid problems with the most negative integer not being representable
 	 * as a positive integer.
 	 */
-	if (value == (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1))
+	if (value == INT64_MIN)
 	{
 		memcpy(a, "-9223372036854775808", 21);
 		return;
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 723c670..33e859d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -44,14 +44,6 @@
 
 #define SAMESIGN(a,b)	(((a) < 0) == ((b) < 0))
 
-#ifndef INT64_MAX
-#define INT64_MAX	INT64CONST(0x7FFFFFFFFFFFFFFF)
-#endif
-
-#ifndef INT64_MIN
-#define INT64_MIN	(-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
-#endif
-
 /* Set at postmaster start */
 TimestampTz PgStartTime;
 
diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c
index f973ef9..31f8033 100644
--- a/src/backend/utils/adt/txid.c
+++ b/src/backend/utils/adt/txid.c
@@ -34,7 +34,7 @@
 
 
 /* txid will be signed int8 in database, so must limit to 63 bits */
-#define MAX_TXID   UINT64CONST(0x7FFFFFFFFFFFFFFF)
+#define MAX_TXID   ((uint64) INT64_MAX)
 
 /* Use unsigned variant internally */
 typedef uint64 txid;
diff --git a/src/include/c.h b/src/include/c.h
index 7447218..e3ed527 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -284,6 +284,17 @@ typedef unsigned long long int uint64;
 #define UINT64CONST(x) ((uint64) x)
 #endif
 
+/* should be defined in stdint.h */
+#ifndef INT64_MIN
+#define INT64_MIN	(-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
+#endif
+#ifndef INT64_MAX
+#define INT64_MAX	INT64CONST(0x7FFFFFFFFFFFFFFF)
+#endif
+#ifndef UINT64_MAX
+#define UINT64_MAX	UINT64CONST(0xFFFFFFFFFFFFFFFF)
+#endif
+
 /* snprintf format strings to use for 64-bit integers */
 #define INT64_FORMAT "%" INT64_MODIFIER "d"
 #define UINT64_FORMAT "%" INT64_MODIFIER "u"
diff --git a/src/include/datatype/timestamp.h b/src/include/datatype/timestamp.h
index 6dfaf23..d3450d6 100644
--- a/src/include/datatype/timestamp.h
+++ b/src/include/datatype/timestamp.h
@@ -119,8 +119,8 @@ typedef struct
  * DT_NOBEGIN represents timestamp -infinity; DT_NOEND represents +infinity
  */
 #ifdef HAVE_INT64_TIMESTAMP
-#define DT_NOBEGIN		(-INT64CONST(0x7fffffffffffffff) - 1)
-#define DT_NOEND		(INT64CONST(0x7fffffffffffffff))
+#define DT_NOBEGIN		INT64_MIN
+#define DT_NOEND		INT64_MAX
 #else							/* !HAVE_INT64_TIMESTAMP */
 #ifdef HUGE_VAL
 #define DT_NOBEGIN		(-HUGE_VAL)
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 5cfc0ae..a207aeb 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -48,7 +48,7 @@
 /*
  * Set the upper and lower bounds of sequence values.
  */
-#define SEQ_MAXVALUE	INT64CONST(0x7FFFFFFFFFFFFFFF)
+#define SEQ_MAXVALUE	INT64_MAX
 #define SEQ_MINVALUE	(-SEQ_MAXVALUE)
 
 /*
diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h
index 89868b5..9917364 100644
--- a/src/include/port/atomics.h
+++ b/src/include/port/atomics.h
@@ -489,7 +489,7 @@ STATIC_IF_INLINE uint64
 pg_atomic_fetch_sub_u64(volatile pg_atomic_uint64 *ptr, int64 sub_)
 {
 	AssertPointerAlignment(ptr, 8);
-	Assert(sub_ != -INT64CONST(0x7FFFFFFFFFFFFFFF) - 1);
+	Assert(sub_ != INT64_MIN);
 	return pg_atomic_fetch_sub_u64_impl(ptr, sub_);
 }
 
@@ -518,7 +518,7 @@ STATIC_IF_INLINE uint64
 pg_atomic_sub_fetch_u64(volatile pg_atomic_uint64 *ptr, int64 sub_)
 {
 	AssertPointerAlignment(ptr, 8);
-	Assert(sub_ != -INT64CONST(0x7FFFFFFFFFFFFFFF) - 1);
+	Assert(sub_ != INT64_MIN);
 	return pg_atomic_sub_fetch_u64_impl(ptr, sub_);
 }
 
diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h
index 2aaf580..8ecf923 100644
--- a/src/include/storage/predicate_internals.h
+++ b/src/include/storage/predicate_internals.h
@@ -33,7 +33,7 @@ typedef uint64 SerCommitSeqNo;
  *	  at that point.  It's earlier than all normal sequence numbers,
  *	  and is only used by recovered prepared transactions
  */
-#define InvalidSerCommitSeqNo		((SerCommitSeqNo) UINT64CONST(0xFFFFFFFFFFFFFFFF))
+#define InvalidSerCommitSeqNo		((SerCommitSeqNo) UINT64_MAX)
 #define RecoverySerCommitSeqNo		((SerCommitSeqNo) 1)
 #define FirstNormalSerCommitSeqNo	((SerCommitSeqNo) 2)

Andrew Gierth

andrew@tao11.riddles.org.uk

almost 11 years ago

In reply to: Andrew Gierth (#3)

Re: INT64_MIN and _MAX

"Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

"Petr" == Petr Jelinek <petr@2ndquadrant.com> writes:

So wouldn't it make more sense to move these definitions into c.h and
standardize their usage?

Petr> I was thinking the same when I've seen Peter's version of Numeric
Petr> abbreviations patch. So +1 for that.

Hm, it looks like the same could be said for INT32_MIN and _MAX; some
places use INT_MIN etc., others say "we shouldn't assume int = int32"
and use (-0x7fffffff - 1) or whatever instead.

--
Andrew (irc:RhodiumToad)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund

andres@anarazel.de

almost 11 years ago

In reply to: Andrew Gierth (#4)

Re: INT64_MIN and _MAX

On March 22, 2015 6:19:52 AM GMT+01:00, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote:

"Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

"Petr" == Petr Jelinek <petr@2ndquadrant.com> writes:

So wouldn't it make more sense to move these definitions into c.h

and

standardize their usage?

Petr> I was thinking the same when I've seen Peter's version of Numeric
Petr> abbreviations patch. So +1 for that.

Hm, it looks like the same could be said for INT32_MIN and _MAX; some
places use INT_MIN etc., others say "we shouldn't assume int = int32"
and use (-0x7fffffff - 1) or whatever instead.

I have been annoyed by this multiple times. I think we should make sure the C99 defines are there (providing values if they aren't) and always use those. We've used them in parts of the tree long enough that it's unlikely to cause problems. Nothing is helped by using different things in other parts of the tree.

Willing to cook up a patch?

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Gierth

andrew@tao11.riddles.org.uk

almost 11 years ago

In reply to: Andres Freund (#5)

1 attachment(s)

Re: INT64_MIN and _MAX

"Andres" == Andres Freund <andres@anarazel.de> writes:

Hm, it looks like the same could be said for INT32_MIN and _MAX;
some places use INT_MIN etc., others say "we shouldn't assume int =
int32" and use (-0x7fffffff - 1) or whatever instead.

Andres> I have been annoyed by this multiple times. I think we should
Andres> make sure the C99 defines are there (providing values if they
Andres> aren't) and always use those. We've used them in parts of the
Andres> tree long enough that it's unlikely to cause problems. Nothing
Andres> is helped by using different things in other parts of the tree.

Andres> Willing to cook up a patch?

How's this one?

This replaces the one I posted before; it does both INT64_MIN/MAX and
INT32_MIN/MAX, and also int16/int8/uint*. Uses of 0x7fffffff in code
have been replaced unless there was a reason not to, with either INT_MAX
or INT32_MAX according to the type required.

What I have _not_ done yet is audit uses of INT_MIN/MAX to see which
ones should really be INT32_MIN/MAX.

--
Andrew (irc:RhodiumToad)

Attachments:

int_minmax.patchtext/x-patchDownload

diff --git a/contrib/btree_gist/btree_ts.c b/contrib/btree_gist/btree_ts.c
index b9c2b49..d472d49 100644
--- a/contrib/btree_gist/btree_ts.c
+++ b/contrib/btree_gist/btree_ts.c
@@ -153,7 +153,7 @@ ts_dist(PG_FUNCTION_ARGS)
 		p->day = INT_MAX;
 		p->month = INT_MAX;
 #ifdef HAVE_INT64_TIMESTAMP
-		p->time = INT64CONST(0x7FFFFFFFFFFFFFFF);
+		p->time = INT64_MAX;
 #else
 		p->time = DBL_MAX;
 #endif
@@ -181,7 +181,7 @@ tstz_dist(PG_FUNCTION_ARGS)
 		p->day = INT_MAX;
 		p->month = INT_MAX;
 #ifdef HAVE_INT64_TIMESTAMP
-		p->time = INT64CONST(0x7FFFFFFFFFFFFFFF);
+		p->time = INT64_MAX;
 #else
 		p->time = DBL_MAX;
 #endif
diff --git a/contrib/intarray/_int_gist.c b/contrib/intarray/_int_gist.c
index 876a7b9..07108eb 100644
--- a/contrib/intarray/_int_gist.c
+++ b/contrib/intarray/_int_gist.c
@@ -3,6 +3,8 @@
  */
 #include "postgres.h"
 
+#include <limits.h>
+
 #include "access/gist.h"
 #include "access/skey.h"
 
@@ -191,7 +193,7 @@ g_int_compress(PG_FUNCTION_ARGS)
 		cand = 1;
 		while (len > MAXNUMRANGE * 2)
 		{
-			min = 0x7fffffff;
+			min = INT_MAX;
 			for (i = 2; i < len; i += 2)
 				if (min > (dr[i] - dr[i - 1]))
 				{
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..822adfd 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -49,10 +49,6 @@
 #include <sys/resource.h>		/* for getrlimit */
 #endif
 
-#ifndef INT64_MAX
-#define INT64_MAX	INT64CONST(0x7FFFFFFFFFFFFFFF)
-#endif
-
 #ifndef M_PI
 #define M_PI 3.14159265358979323846
 #endif
@@ -453,7 +449,7 @@ strtoint64(const char *str)
 		 */
 		if (strncmp(ptr, "9223372036854775808", 19) == 0)
 		{
-			result = -INT64CONST(0x7fffffffffffffff) - 1;
+			result = INT64_MIN;
 			ptr += 19;
 			goto gotdigits;
 		}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e2d187f..656d55b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1408,7 +1408,7 @@ WALInsertLockAcquireExclusive(void)
 	{
 		LWLockAcquireWithVar(&WALInsertLocks[i].l.lock,
 							 &WALInsertLocks[i].l.insertingAt,
-							 UINT64CONST(0xFFFFFFFFFFFFFFFF));
+							 UINT64_MAX);
 	}
 	LWLockAcquireWithVar(&WALInsertLocks[i].l.lock,
 						 &WALInsertLocks[i].l.insertingAt,
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index ca5b543..d95fdb1 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -14,6 +14,8 @@
 
 #include "postgres.h"
 
+#include <limits.h>
+
 #include "catalog/pg_collation.h"
 #include "commands/defrem.h"
 #include "tsearch/ts_locale.h"
@@ -2047,7 +2049,7 @@ hlCover(HeadlineParsedText *prs, TSQuery query, int *p, int *q)
 	int			pos = *p;
 
 	*q = -1;
-	*p = 0x7fffffff;
+	*p = INT_MAX;
 
 	for (j = 0; j < query->size; j++)
 	{
@@ -2258,7 +2260,7 @@ mark_hl_fragments(HeadlineParsedText *prs, TSQuery query, int highlight,
 	for (f = 0; f < max_fragments; f++)
 	{
 		maxitems = 0;
-		minwords = 0x7fffffff;
+		minwords = INT32_MAX;
 		minI = -1;
 
 		/*
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index 56d909a..b3a1191 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -78,7 +78,7 @@ scanint8(const char *str, bool errorOK, int64 *result)
 		 */
 		if (strncmp(ptr, "9223372036854775808", 19) == 0)
 		{
-			tmp = -INT64CONST(0x7fffffffffffffff) - 1;
+			tmp = INT64_MIN;
 			ptr += 19;
 			goto gotdigits;
 		}
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index d77799a..585da1e 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -190,7 +190,7 @@ pg_lltoa(int64 value, char *a)
 	 * Avoid problems with the most negative integer not being representable
 	 * as a positive integer.
 	 */
-	if (value == (-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1))
+	if (value == INT64_MIN)
 	{
 		memcpy(a, "-9223372036854775808", 21);
 		return;
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 723c670..33e859d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -44,14 +44,6 @@
 
 #define SAMESIGN(a,b)	(((a) < 0) == ((b) < 0))
 
-#ifndef INT64_MAX
-#define INT64_MAX	INT64CONST(0x7FFFFFFFFFFFFFFF)
-#endif
-
-#ifndef INT64_MIN
-#define INT64_MIN	(-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
-#endif
-
 /* Set at postmaster start */
 TimestampTz PgStartTime;
 
diff --git a/src/backend/utils/adt/tsrank.c b/src/backend/utils/adt/tsrank.c
index 733203e..a3066fa 100644
--- a/src/backend/utils/adt/tsrank.c
+++ b/src/backend/utils/adt/tsrank.c
@@ -13,6 +13,7 @@
  */
 #include "postgres.h"
 
+#include <limits.h>
 #include <math.h>
 
 #include "tsearch/ts_utils.h"
@@ -555,7 +556,7 @@ Cover(DocRepresentation *doc, int len, QueryRepresentation *qr, CoverExt *ext)
 
 	memset(qr->operandexist, 0, sizeof(bool) * qr->query->size);
 
-	ext->p = 0x7fffffff;
+	ext->p = INT_MAX;
 	ext->q = 0;
 	ptr = doc + ext->pos;
 
diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c
index f973ef9..31f8033 100644
--- a/src/backend/utils/adt/txid.c
+++ b/src/backend/utils/adt/txid.c
@@ -34,7 +34,7 @@
 
 
 /* txid will be signed int8 in database, so must limit to 63 bits */
-#define MAX_TXID   UINT64CONST(0x7FFFFFFFFFFFFFFF)
+#define MAX_TXID   ((uint64) INT64_MAX)
 
 /* Use unsigned variant internally */
 typedef uint64 txid;
diff --git a/src/include/c.h b/src/include/c.h
index 7447218..e7ee510 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -249,6 +249,36 @@ typedef uint8 bits8;			/* >= 8 bits */
 typedef uint16 bits16;			/* >= 16 bits */
 typedef uint32 bits32;			/* >= 32 bits */
 
+/* should be defined in stdint.h, but we guarantee them here */
+#ifndef INT8_MIN
+#define INT8_MIN	(-0x7F-1)
+#endif
+#ifndef INT8_MAX
+#define INT8_MAX	(0x7F)
+#endif
+#ifndef INT16_MIN
+#define INT16_MIN	(-0x7FFF-1)
+#endif
+#ifndef INT16_MAX
+#define INT16_MAX	(0x7FFF)
+#endif
+#ifndef INT32_MIN
+#define INT32_MIN	(-0x7FFFFFFF-1)
+#endif
+#ifndef INT32_MAX
+#define INT32_MAX	(0x7FFFFFFF)
+#endif
+
+#ifndef UINT8_MAX
+#define UINT8_MAX	(0xFF)
+#endif
+#ifndef UINT16_MAX
+#define UINT16_MAX	(0xFFFF)
+#endif
+#ifndef UINT32_MAX
+#define UINT32_MAX	(0xFFFFFFFF)
+#endif
+
 /*
  * 64-bit integers
  */
@@ -284,6 +314,17 @@ typedef unsigned long long int uint64;
 #define UINT64CONST(x) ((uint64) x)
 #endif
 
+/* should be defined in stdint.h, but we guarantee them here */
+#ifndef INT64_MIN
+#define INT64_MIN	(-INT64CONST(0x7FFFFFFFFFFFFFFF) - 1)
+#endif
+#ifndef INT64_MAX
+#define INT64_MAX	INT64CONST(0x7FFFFFFFFFFFFFFF)
+#endif
+#ifndef UINT64_MAX
+#define UINT64_MAX	UINT64CONST(0xFFFFFFFFFFFFFFFF)
+#endif
+
 /* snprintf format strings to use for 64-bit integers */
 #define INT64_FORMAT "%" INT64_MODIFIER "d"
 #define UINT64_FORMAT "%" INT64_MODIFIER "u"
diff --git a/src/include/datatype/timestamp.h b/src/include/datatype/timestamp.h
index 6dfaf23..d3450d6 100644
--- a/src/include/datatype/timestamp.h
+++ b/src/include/datatype/timestamp.h
@@ -119,8 +119,8 @@ typedef struct
  * DT_NOBEGIN represents timestamp -infinity; DT_NOEND represents +infinity
  */
 #ifdef HAVE_INT64_TIMESTAMP
-#define DT_NOBEGIN		(-INT64CONST(0x7fffffffffffffff) - 1)
-#define DT_NOEND		(INT64CONST(0x7fffffffffffffff))
+#define DT_NOBEGIN		INT64_MIN
+#define DT_NOEND		INT64_MAX
 #else							/* !HAVE_INT64_TIMESTAMP */
 #ifdef HUGE_VAL
 #define DT_NOBEGIN		(-HUGE_VAL)
diff --git a/src/include/executor/instrument.h b/src/include/executor/instrument.h
index 1c3b2b0..db9d1e8 100644
--- a/src/include/executor/instrument.h
+++ b/src/include/executor/instrument.h
@@ -38,7 +38,7 @@ typedef enum InstrumentOption
 	INSTRUMENT_TIMER = 1 << 0,	/* needs timer (and row counts) */
 	INSTRUMENT_BUFFERS = 1 << 1,	/* needs buffer usage */
 	INSTRUMENT_ROWS = 1 << 2,	/* needs row count */
-	INSTRUMENT_ALL = 0x7FFFFFFF
+	INSTRUMENT_ALL = INT32_MAX
 } InstrumentOption;
 
 typedef struct Instrumentation
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ec0d0ea..2893cef 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -587,7 +587,7 @@ typedef enum TableLikeOption
 	CREATE_TABLE_LIKE_INDEXES = 1 << 2,
 	CREATE_TABLE_LIKE_STORAGE = 1 << 3,
 	CREATE_TABLE_LIKE_COMMENTS = 1 << 4,
-	CREATE_TABLE_LIKE_ALL = 0x7FFFFFFF
+	CREATE_TABLE_LIKE_ALL = INT32_MAX
 } TableLikeOption;
 
 /*
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index 5cfc0ae..d3e9888 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -48,7 +48,7 @@
 /*
  * Set the upper and lower bounds of sequence values.
  */
-#define SEQ_MAXVALUE	INT64CONST(0x7FFFFFFFFFFFFFFF)
+#define SEQ_MAXVALUE	INT64_MAX
 #define SEQ_MINVALUE	(-SEQ_MAXVALUE)
 
 /*
@@ -185,7 +185,7 @@
  * the older rand() function, which is often different from --- and
  * considerably inferior to --- random().
  */
-#define MAX_RANDOM_VALUE  (0x7FFFFFFF)
+#define MAX_RANDOM_VALUE  INT32_MAX
 
 /*
  * On PPC machines, decide whether to use the mutex hint bit in LWARX
diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h
index 89868b5..9917364 100644
--- a/src/include/port/atomics.h
+++ b/src/include/port/atomics.h
@@ -489,7 +489,7 @@ STATIC_IF_INLINE uint64
 pg_atomic_fetch_sub_u64(volatile pg_atomic_uint64 *ptr, int64 sub_)
 {
 	AssertPointerAlignment(ptr, 8);
-	Assert(sub_ != -INT64CONST(0x7FFFFFFFFFFFFFFF) - 1);
+	Assert(sub_ != INT64_MIN);
 	return pg_atomic_fetch_sub_u64_impl(ptr, sub_);
 }
 
@@ -518,7 +518,7 @@ STATIC_IF_INLINE uint64
 pg_atomic_sub_fetch_u64(volatile pg_atomic_uint64 *ptr, int64 sub_)
 {
 	AssertPointerAlignment(ptr, 8);
-	Assert(sub_ != -INT64CONST(0x7FFFFFFFFFFFFFFF) - 1);
+	Assert(sub_ != INT64_MIN);
 	return pg_atomic_sub_fetch_u64_impl(ptr, sub_);
 }
 
diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h
index 2aaf580..8ecf923 100644
--- a/src/include/storage/predicate_internals.h
+++ b/src/include/storage/predicate_internals.h
@@ -33,7 +33,7 @@ typedef uint64 SerCommitSeqNo;
  *	  at that point.  It's earlier than all normal sequence numbers,
  *	  and is only used by recovered prepared transactions
  */
-#define InvalidSerCommitSeqNo		((SerCommitSeqNo) UINT64CONST(0xFFFFFFFFFFFFFFFF))
+#define InvalidSerCommitSeqNo		((SerCommitSeqNo) UINT64_MAX)
 #define RecoverySerCommitSeqNo		((SerCommitSeqNo) 1)
 #define FirstNormalSerCommitSeqNo	((SerCommitSeqNo) 2)
 
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index b57f4bb..f07011c 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -35,11 +35,9 @@ typedef struct
 
 /*
  * Infinity and minus infinity must be the max and min values of DateADT.
- * We could use INT_MIN and INT_MAX here, but seems better to not assume that
- * int32 == int.
  */
-#define DATEVAL_NOBEGIN		((DateADT) (-0x7fffffff - 1))
-#define DATEVAL_NOEND		((DateADT) 0x7fffffff)
+#define DATEVAL_NOBEGIN		((DateADT) INT32_MIN)
+#define DATEVAL_NOEND		((DateADT) INT32_MAX)
 
 #define DATE_NOBEGIN(j)		((j) = DATEVAL_NOBEGIN)
 #define DATE_IS_NOBEGIN(j)	((j) == DATEVAL_NOBEGIN)

Peter Geoghegan

pg@heroku.com

almost 11 years ago

In reply to: Andres Freund (#5)

Re: INT64_MIN and _MAX

On Sun, Mar 22, 2015 at 2:26 AM, Andres Freund <andres@anarazel.de> wrote:

I have been annoyed by this multiple times. I think we should make sure the C99 defines are there (providing values if they aren't) and always use those. We've used them in parts of the tree long enough that it's unlikely to cause problems. Nothing is helped by using different things in other parts of the tree.

--
Peter Geoghegan

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund

andres@anarazel.de

almost 11 years ago

In reply to: Andrew Gierth (#6)

Re: INT64_MIN and _MAX

Hi,

On 2015-03-22 17:20:22 +0000, Andrew Gierth wrote:

This replaces the one I posted before; it does both INT64_MIN/MAX and
INT32_MIN/MAX, and also int16/int8/uint*. Uses of 0x7fffffff in code
have been replaced unless there was a reason not to, with either INT_MAX
or INT32_MAX according to the type required.

Any reason you did that for most of 0x7FFFFFFF, but not for the
corresponding 0xFFFFFFFF/unsigned case? I'd like to either avoid going
around changing other definitions, or do a somewhat systematic job.

What I have _not_ done yet is audit uses of INT_MIN/MAX to see which
ones should really be INT32_MIN/MAX.

I'm doubtful it's worthwhile to do check that all over the codebase...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Gierth

andrew@tao11.riddles.org.uk

almost 11 years ago

In reply to: Andres Freund (#8)

Re: INT64_MIN and _MAX

"Andres" == Andres Freund <andres@anarazel.de> writes:

This replaces the one I posted before; it does both INT64_MIN/MAX and
INT32_MIN/MAX, and also int16/int8/uint*. Uses of 0x7fffffff in code
have been replaced unless there was a reason not to, with either INT_MAX
or INT32_MAX according to the type required.

Andres> Any reason you did that for most of 0x7FFFFFFF, but not for the
Andres> corresponding 0xFFFFFFFF/unsigned case? I'd like to either
Andres> avoid going around changing other definitions, or do a somewhat
Andres> systematic job.

I didn't replace the 0xFFFFFFFF ones because most or all of them looked
like basically bit-masking operations rather than actually dealing with
the bounds of an unsigned int or uint32. I was specifically looking for
places where literals were being used to represent maximum or minimum
values.

--
Andrew (irc:RhodiumToad)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10

Kevin Grittner

kgrittn@ymail.com

almost 11 years ago

In reply to: Andrew Gierth (#9)

Re: INT64_MIN and _MAX

Andrew Gierth <andrew@tao11.riddles.org.uk> wrote:

I didn't replace the 0xFFFFFFFF ones because most or all of them looked
like basically bit-masking operations rather than actually dealing with
the bounds of an unsigned int or uint32. I was specifically looking for
places where literals were being used to represent maximum or minimum
values.

Well, InvalidSerCommitSeqNo was initially defined to be UINT64_MAX
-- but some buildfarm members didn't know about that so it was
changed to UINT64CONST(0xFFFFFFFFFFFFFFFF). It is very much about
wanting the maximum value for uint64. As the comment says:

* - InvalidSerCommitSeqNo is used to indicate a transaction that
* hasn't committed yet, so use a number greater than all valid
* ones to make comparison do the expected thing

It does seem odd to only define *some* of these constants.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11

Andrew Gierth

andrew@tao11.riddles.org.uk

almost 11 years ago

In reply to: Kevin Grittner (#10)

Re: INT64_MIN and _MAX

"Kevin" == Kevin Grittner <kgrittn@ymail.com> writes:

I didn't replace the 0xFFFFFFFF ones because most or all of them
looked like basically bit-masking operations rather than actually
dealing with the bounds of an unsigned int or uint32. I was
specifically looking for places where literals were being used to
represent maximum or minimum values.

Kevin> Well, InvalidSerCommitSeqNo was initially defined to be
Kevin> UINT64_MAX -- but some buildfarm members didn't know about that
Kevin> so it was changed to UINT64CONST(0xFFFFFFFFFFFFFFFF). It is
Kevin> very much about wanting the maximum value for uint64.

That one _is_ changed to UINT64_MAX in my patch.

--
Andrew (irc:RhodiumToad)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12

Kyotaro HORIGUCHI

horiguchi.kyotaro@lab.ntt.co.jp

almost 11 years ago

In reply to: Andrew Gierth (#11)

1 attachment(s)

Re: INT64_MIN and _MAX

Hello,

Grep showed me some unfixed usages of bare constant or INT64CONST
as (u)int64 max/min values.

At Tue, 24 Mar 2015 21:57:42 +0000, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote in <87619q6ouh.fsf@news-spur.riddles.org.uk>

"Kevin" == Kevin Grittner <kgrittn@ymail.com> writes:

Kevin> Well, InvalidSerCommitSeqNo was initially defined to be
Kevin> UINT64_MAX -- but some buildfarm members didn't know about that
Kevin> so it was changed to UINT64CONST(0xFFFFFFFFFFFFFFFF). It is
Kevin> very much about wanting the maximum value for uint64.

That one _is_ changed to UINT64_MAX in my patch.

./src/interfaces/ecpg/pgtypeslib/dt.h:

#define DT_NOBEGIN (-INT64CONST(0x7fffffffffffffff) - 1)
#define DT_NOEND (INT64CONST(0x7fffffffffffffff))

./contrib/pgcrypto/imath.h:

#define MP_WORD_MAX 0xFFFFFFFFFFFFFFFFULL

Likewise, bare (u)int32/16 min/max's are found as following.

./contrib/pgcrypto/imath.h:

#define MP_DIGIT_MAX 0xFFFFFFFFULL

#define MP_DIGIT_MAX 0xFFFFUL
#define MP_WORD_MAX 0xFFFFFFFFUL

# MP_DIGIT_MAX was wrong in word length. They are in the half
# length of MP_WORD_MAX.

./src/backend/utils/mb/wchar.c
./src/bin/psql/mbprint.c:

return 0xffffffff;

Additional fixes for the above are in the patch attached.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

int_minmax_add1.patchtext/x-patch; charset=us-asciiDownload

diff --git a/contrib/pgcrypto/imath.h b/contrib/pgcrypto/imath.h
index 0a4f0f7..7c30bd4 100644
--- a/contrib/pgcrypto/imath.h
+++ b/contrib/pgcrypto/imath.h
@@ -44,14 +44,14 @@ typedef int mp_result;
 typedef uint32 mp_digit;
 typedef uint64 mp_word;
 
-#define MP_DIGIT_MAX	   0xFFFFFFFFULL
-#define MP_WORD_MAX		   0xFFFFFFFFFFFFFFFFULL
+#define MP_DIGIT_MAX	   UINT32_MAX
+#define MP_WORD_MAX		   UINT64_MAX
 #else
 typedef uint16 mp_digit;
 typedef uint32 mp_word;
 
-#define MP_DIGIT_MAX	   0xFFFFUL
-#define MP_WORD_MAX		   0xFFFFFFFFUL
+#define MP_DIGIT_MAX	   UINT16_MAX
+#define MP_WORD_MAX		   UINT32_MAX
 #endif
 
 typedef struct mpz
diff --git a/src/backend/utils/mb/wchar.c b/src/backend/utils/mb/wchar.c
index 0cc753e..6ff99e6 100644
--- a/src/backend/utils/mb/wchar.c
+++ b/src/backend/utils/mb/wchar.c
@@ -729,7 +729,7 @@ utf8_to_unicode(const unsigned char *c)
 						   (c[3] & 0x3f));
 	else
 		/* that is an invalid code on purpose */
-		return 0xffffffff;
+		return UINT32_MAX;
 }
 
 static int
diff --git a/src/bin/psql/mbprint.c b/src/bin/psql/mbprint.c
index e29c619..6b3f17d 100644
--- a/src/bin/psql/mbprint.c
+++ b/src/bin/psql/mbprint.c
@@ -67,7 +67,7 @@ utf8_to_unicode(const unsigned char *c)
 						   (c[3] & 0x3f));
 	else
 		/* that is an invalid code on purpose */
-		return 0xffffffff;
+		return UINT32_MAX;
 }
 
 
diff --git a/src/interfaces/ecpg/pgtypeslib/dt.h b/src/interfaces/ecpg/pgtypeslib/dt.h
index 145e2b7..028f0df 100644
--- a/src/interfaces/ecpg/pgtypeslib/dt.h
+++ b/src/interfaces/ecpg/pgtypeslib/dt.h
@@ -320,8 +320,8 @@ do { \
 
 #ifdef HAVE_INT64_TIMESTAMP
 
-#define DT_NOBEGIN		(-INT64CONST(0x7fffffffffffffff) - 1)
-#define DT_NOEND		(INT64CONST(0x7fffffffffffffff))
+#define DT_NOBEGIN		INT64_MIN
+#define DT_NOEND		INT64_MAX
 #else
 
 #ifdef HUGE_VAL

#13

Andrew Gierth

andrew@tao11.riddles.org.uk

almost 11 years ago

In reply to: Kyotaro HORIGUCHI (#12)

Re: INT64_MIN and _MAX

"Kyotaro" == Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:

Kyotaro> Hello,
Kyotaro> Grep showed me some unfixed usages of bare constant or
Kyotaro> INT64CONST as (u)int64 max/min values.

Kyotaro> ./src/interfaces/ecpg/pgtypeslib/dt.h:

I didn't touch the ecpg stuff since it wasn't too clear that it was safe
to change, but on second look it is.

Kyotaro> ./contrib/pgcrypto/imath.h:

I didn't touch this since it was obviously a library copied from
somewhere else.

Kyotaro> ./src/backend/utils/mb/wchar.c
Kyotaro> ./src/bin/psql/mbprint.c:

return 0xffffffff;

Here 0xffffffff is not a uint or uint32, but a pg_wchar (which is
unsigned int, not uint32). What's needed there is not UINT_MAX but
rather a PG_WCHAR_INVALID or similar definition in pg_wchar.h.

--
Andrew (irc:RhodiumToad)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers