Fix incorrect addition, subtraction, and overflow checking in new

inet operators.

src/backend/utils/adt/network.c

 /*
  *	PostgreSQL type definitions for the INET and CIDR types.
  *
- *	$PostgreSQL: pgsql/src/backend/utils/adt/network.c,v 1.64 2006/02/11 03:32:39 momjian Exp $
+ *	$PostgreSQL: pgsql/src/backend/utils/adt/network.c,v 1.65 2006/02/11 20:39:58 tgl Exp $
  *
  *	Jon Postel RIP 16 Oct 1998
  */
@@ -27,7 +27,7 @@ static int32 network_cmp_internal(inet *a1, inet *a2);
 static int	bitncmp(void *l, void *r, int n);
 static bool addressOK(unsigned char *a, int bits, int family);
 static int	ip_addrsize(inet *inetptr);
-static Datum internal_inetpl(inet *ip, int64 iarg);
+static inet *internal_inetpl(inet *ip, int64 addend);
 
 /*
  *	Access macros.
@@ -1292,8 +1292,7 @@ inetand(PG_FUNCTION_ARGS)
 	if (ip_family(ip) != ip_family(ip2))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("mismatch in address family (%d) != (%d)",
-						ip_family(ip), ip_family(ip2))));
+				 errmsg("cannot AND inet values of different sizes")));
 	else
 	{
 		int nb = ip_addrsize(ip);
@@ -1327,8 +1326,7 @@ inetor(PG_FUNCTION_ARGS)
 	if (ip_family(ip) != ip_family(ip2))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("mismatch in address family (%d) != (%d)",
-						ip_family(ip), ip_family(ip2))));
+				 errmsg("cannot OR inet values of different sizes")));
 	else
 	{
 		int nb = ip_addrsize(ip);
@@ -1350,8 +1348,8 @@ inetor(PG_FUNCTION_ARGS)
 }
 
 
-static Datum
-internal_inetpl(inet *ip, int64 plus)
+static inet *
+internal_inetpl(inet *ip, int64 addend)
 {
 	inet	   *dst;
 
@@ -1365,15 +1363,31 @@ internal_inetpl(inet *ip, int64 plus)
 
 		while (nb-- > 0)
 		{
-			pdst[nb] = carry = pip[nb] + plus + carry;
-			plus /= 0x100;		/* process next byte */
-			carry /= 0x100;		/* remove low byte */
-			/* Overflow on high byte? */
-			if (nb == 0 && (plus != 0 || carry != 0))
-				ereport(ERROR,
-						(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-						 errmsg("result out of range")));
+			carry = pip[nb] + (int) (addend & 0xFF) + carry;
+			pdst[nb] = (unsigned char) (carry & 0xFF);
+			carry >>= 8;
+			/*
+			 * We have to be careful about right-shifting addend because
+			 * right-shift isn't portable for negative values, while
+			 * simply dividing by 256 doesn't work (the standard rounding
+			 * is in the wrong direction, besides which there may be machines
+			 * out there that round the wrong way).  So, explicitly clear
+			 * the low-order byte to remove any doubt about the correct
+			 * result of the division, and then divide rather than shift.
+			 */
+			addend &= ~((int64) 0xFF);
+			addend /= 0x100;
 		}
+		/*
+		 * At this point we should have addend and carry both zero if
+		 * original addend was >= 0, or addend -1 and carry 1 if original
+		 * addend was < 0.  Anything else means overflow.
+		 */
+		if (!((addend == 0 && carry == 0) ||
+			  (addend == -1 && carry == 1)))
+			ereport(ERROR,
+					(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+					 errmsg("result out of range")));
 	}
 	ip_bits(dst) = ip_bits(ip);
 
@@ -1382,7 +1396,7 @@ internal_inetpl(inet *ip, int64 plus)
 		((char *) ip_addr(dst) - (char *) VARDATA(dst)) +
 		ip_addrsize(dst);
 
-	PG_RETURN_INET_P(dst);
+	return dst;
 }
 
 
@@ -1390,9 +1404,9 @@ Datum
 inetpl(PG_FUNCTION_ARGS)
 {
 	inet   *ip = PG_GETARG_INET_P(0);
-	int64	plus = PG_GETARG_INT64(1);
+	int64	addend = PG_GETARG_INT64(1);
 
-	return internal_inetpl(ip, plus);
+	PG_RETURN_INET_P(internal_inetpl(ip, addend));
 }
 
 
@@ -1400,9 +1414,9 @@ Datum
 inetmi_int8(PG_FUNCTION_ARGS)
 {
 	inet   *ip = PG_GETARG_INET_P(0);
-	int64	plus = PG_GETARG_INT64(1);
+	int64	addend = PG_GETARG_INT64(1);
 
-	return internal_inetpl(ip, -plus);
+	PG_RETURN_INET_P(internal_inetpl(ip, -addend));
 }
 
 
@@ -1416,42 +1430,53 @@ inetmi(PG_FUNCTION_ARGS)
 	if (ip_family(ip) != ip_family(ip2))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("mismatch in address family (%d) != (%d)",
-						ip_family(ip), ip_family(ip2))));
+				 errmsg("cannot subtract inet values of different sizes")));
 	else
 	{
+		/*
+		 * We form the difference using the traditional complement,
+		 * increment, and add rule, with the increment part being handled
+		 * by starting the carry off at 1.  If you don't think integer
+		 * arithmetic is done in two's complement, too bad.
+		 */
 		int nb = ip_addrsize(ip);
 		int	byte = 0;
 		unsigned char	*pip = ip_addr(ip);
 		unsigned char	*pip2 = ip_addr(ip2);
+		int carry = 1;
 
 		while (nb-- > 0)
 		{
-			/*
-			 *	Error if overflow on last byte.  This test is tricky
-			 *	because if the subtraction == 128 and res is negative, or
-			 *	if subtraction == -128 and res is positive, the result
-			 *	would still fit in int64.
-			 */
-			if (byte + 1 == sizeof(int64) &&
-				(pip[nb] - pip2[nb] >= 128 + (res < 0) ||
-				 pip[nb] - pip2[nb] <= -128 - (res > 0)))
-				ereport(ERROR,
-						(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-						 errmsg("result out of range")));
-			if (byte >= sizeof(int64))
+			int		lobyte;
+
+			carry = pip[nb] + (~pip2[nb] & 0xFF) + carry;
+			lobyte = carry & 0xFF;
+			if (byte < sizeof(int64))
 			{
-				/* Error if bytes beyond int64 length differ. */
-				if (pip[nb] != pip2[nb])
+				res |= ((int64) lobyte) << (byte * 8);
+			}
+			else
+			{
+				/*
+				 * Input wider than int64: check for overflow.  All bytes
+				 * to the left of what will fit should be 0 or 0xFF,
+				 * depending on sign of the now-complete result.
+				 */
+				if ((res < 0) ? (lobyte != 0xFF) : (lobyte != 0))
 					ereport(ERROR,
 							(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
 							 errmsg("result out of range")));
 			}
-			else
-				res += (int64)(pip[nb] - pip2[nb]) << (byte * 8);
-
+			carry >>= 8;
 			byte++;
 		}
+
+		/*
+		 * If input is narrower than int64, overflow is not possible, but
+		 * we have to do proper sign extension.
+		 */
+		if (carry == 0 && byte < sizeof(int64))
+			res |= ((int64) -1) << (byte * 8);
 	}
 
 	PG_RETURN_INT64(res);

src/include/catalog/pg_proc.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.395 2006/02/11 03:32:39 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.396 2006/02/11 20:39:58 tgl Exp $
  *
  * NOTES
  *	  The script catalog/genbki.sh reads this file and generates .bki
@@ -2297,7 +2297,7 @@ DESCR("bitwise or");
 DATA(insert OID = 1675 (  bitxor			PGNSP PGUID 12 f f t f i 2 1560 "1560 1560" _null_ _null_ _null_	bitxor - _null_ ));
 DESCR("bitwise exclusive or");
 DATA(insert OID = 1676 (  bitnot			PGNSP PGUID 12 f f t f i 1 1560 "1560" _null_ _null_ _null_ bitnot - _null_ ));
-DESCR("bitwise negation");
+DESCR("bitwise not");
 DATA(insert OID = 1677 (  bitshiftleft		PGNSP PGUID 12 f f t f i 2 1560 "1560 23" _null_ _null_ _null_	bitshiftleft - _null_ ));
 DESCR("bitwise left shift");
 DATA(insert OID = 1678 (  bitshiftright		PGNSP PGUID 12 f f t f i 2 1560 "1560 23" _null_ _null_ _null_	bitshiftright - _null_ ));
@@ -2423,28 +2423,28 @@ DATA(insert OID = 1715 (  cidr				PGNSP PGUID 12 f f t f i 1 650 "869" _null_ _n
 DESCR("coerce inet to cidr");
 
 DATA(insert OID = 2196 (  inet_client_addr		PGNSP PGUID 12 f f f f s 0 869 "" _null_ _null_ _null_	inet_client_addr - _null_ ));
-DESCR("INET address of the client");
+DESCR("inet address of the client");
 DATA(insert OID = 2197 (  inet_client_port		PGNSP PGUID 12 f f f f s 0 23 "" _null_ _null_ _null_  inet_client_port - _null_ ));
 DESCR("client's port number for this connection");
 DATA(insert OID = 2198 (  inet_server_addr		PGNSP PGUID 12 f f f f s 0 869 "" _null_ _null_ _null_	inet_server_addr - _null_ ));
-DESCR("INET address of the server");
+DESCR("inet address of the server");
 DATA(insert OID = 2199 (  inet_server_port		PGNSP PGUID 12 f f f f s 0 23 "" _null_ _null_ _null_  inet_server_port - _null_ ));
 DESCR("server's port number for this connection");
 
 DATA(insert OID = 2627 (  inetnot			PGNSP PGUID 12 f f t f i 1 869 "869" _null_ _null_ _null_	inetnot - _null_ ));
-DESCR("binary NOT");
+DESCR("bitwise not");
 DATA(insert OID = 2628 (  inetand			PGNSP PGUID 12 f f t f i 2 869 "869 869" _null_ _null_ _null_	inetand - _null_ ));
-DESCR("binary AND");
+DESCR("bitwise and");
 DATA(insert OID = 2629 (  inetor			PGNSP PGUID 12 f f t f i 2 869 "869 869" _null_ _null_ _null_	inetor - _null_ ));
-DESCR("binary OR");
+DESCR("bitwise or");
 DATA(insert OID = 2630 (  inetpl			PGNSP PGUID 12 f f t f i 2 869 "869 20" _null_ _null_ _null_	inetpl - _null_ ));
-DESCR("add integer to INET value");
-DATA(insert OID = 2631 ( int8pl_inet		PGNSP PGUID 14 f f t f i 2 869 "20 869" _null_ _null_ _null_	"select $2 + $1" - _null_ ));
-DESCR("add integer to INET value");
+DESCR("add integer to inet value");
+DATA(insert OID = 2631 (  int8pl_inet		PGNSP PGUID 14 f f t f i 2 869 "20 869" _null_ _null_ _null_	"select $2 + $1" - _null_ ));
+DESCR("add integer to inet value");
 DATA(insert OID = 2632 (  inetmi_int8		PGNSP PGUID 12 f f t f i 2 869 "869 20" _null_ _null_ _null_	inetmi_int8 - _null_ ));
-DESCR("subtract integer from INET value");
+DESCR("subtract integer from inet value");
 DATA(insert OID = 2633 (  inetmi			PGNSP PGUID 12 f f t f i 2 20 "869 869" _null_ _null_ _null_	inetmi - _null_ ));
-DESCR("subtract INET values");
+DESCR("subtract inet values");
 
 DATA(insert OID = 1686 ( numeric			PGNSP PGUID 12 f f t f i 1 1700 "25" _null_ _null_ _null_ text_numeric - _null_ ));
 DESCR("(internal)");

src/test/regress/sql/inet.sql

@@ -62,14 +62,29 @@ CREATE INDEX inet_idx1 ON inet_tbl(i);
 SET enable_seqscan TO off;
 SELECT * FROM inet_tbl WHERE i<<'192.168.1.0/24'::cidr;
 SELECT * FROM inet_tbl WHERE i<<='192.168.1.0/24'::cidr;
-
-SELECT ~i FROM inet_tbl;
-SELECT i & c FROM inet_tbl;
-SELECT i | c FROM inet_tbl;
-SELECT i + 500 FROM inet_tbl;
-SELECT i - 500 FROM inet_tbl;
-SELECT i - c FROM inet_tbl;
-
 SET enable_seqscan TO on;
 DROP INDEX inet_idx1;
 
+-- simple tests of inet boolean and arithmetic operators
+SELECT i, ~i AS "~i" FROM inet_tbl;
+SELECT i, c, i & c AS "and" FROM inet_tbl;
+SELECT i, c, i | c AS "or" FROM inet_tbl;
+SELECT i, i + 500 AS "i+500" FROM inet_tbl;
+SELECT i, i - 500 AS "i-500" FROM inet_tbl;
+SELECT i, c, i - c AS "minus" FROM inet_tbl;
+SELECT '127.0.0.1'::inet + 257;
+SELECT ('127.0.0.1'::inet + 257) - 257;
+SELECT '127::1'::inet + 257;
+SELECT ('127::1'::inet + 257) - 257;
+SELECT '127.0.0.2'::inet  - ('127.0.0.2'::inet + 500);
+SELECT '127.0.0.2'::inet  - ('127.0.0.2'::inet - 500);
+SELECT '127::2'::inet  - ('127::2'::inet + 500);
+SELECT '127::2'::inet  - ('127::2'::inet - 500);
+-- these should give overflow errors:
+SELECT '127.0.0.1'::inet + 10000000000;
+SELECT '127.0.0.1'::inet - 10000000000;
+SELECT '126::1'::inet - '127::2'::inet;
+SELECT '127::1'::inet - '126::2'::inet;
+-- but not these
+SELECT '127::1'::inet + 10000000000;
+SELECT '127::1'::inet - '127::2'::inet;