Re: [BUGS] Bug in create operator and/or initdb

From: Paul Vixie <paul(at)vix(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Steve Atkins <steve(at)blighty(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [BUGS] Bug in create operator and/or initdb
Date: 2005-01-31 19:28:28
Message-ID: 20050131192828.E111513960@sa.vix.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

when my cidr datatype was integrated into pgsql, the decision was made to
incorporate a copy of bind's inet_net_pton.c rather than add a link-time
dependence to libbind.a (libbind.so). thus, when this bug was fixed in
2003:

----------------------------
revision 1.14
date: 2003/08/20 02:21:08; author: marka; state: Exp; lines: +10 -4
1580. [bug] inet_net_pton() didn't fully handle implicit
multicast IPv4 network addresses.

the pgsql "fork" of this code did not benefit from the fix. the patch was:

Index: inet_net_pton.c
===================================================================
RCS file: /proj/cvs/prod/bind8/src/lib/inet/inet_net_pton.c,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- inet_net_pton.c 27 Sep 2001 15:08:38 -0000 1.13
+++ inet_net_pton.c 20 Aug 2003 02:21:08 -0000 1.14
@@ -16,7 +16,7 @@
*/

#if defined(LIBC_SCCS) && !defined(lint)
-static const char rcsid[] = "$Id: inet_net_pton.c,v 1.13 2001/09/27 15:08:38 marka Exp $";
+static const char rcsid[] = "$Id: inet_net_pton.c,v 1.14 2003/08/20 02:21:08 marka Exp $";
#endif

#include "port_before.h"
@@ -59,7 +59,7 @@
* Paul Vixie (ISC), June 1996
*/
static int
-inet_net_pton_ipv4( const char *src, u_char *dst, size_t size) {
+inet_net_pton_ipv4(const char *src, u_char *dst, size_t size) {
static const char xdigits[] = "0123456789abcdef";
static const char digits[] = "0123456789";
int n, ch, tmp = 0, dirty, bits;
@@ -152,7 +152,7 @@
if (*odst >= 240) /* Class E */
bits = 32;
else if (*odst >= 224) /* Class D */
- bits = 4;
+ bits = 8;
else if (*odst >= 192) /* Class C */
bits = 24;
else if (*odst >= 128) /* Class B */
@@ -160,8 +160,14 @@
else /* Class A */
bits = 8;
/* If imputed mask is narrower than specified octets, widen. */
- if (bits >= 8 && bits < ((dst - odst) * 8))
+ if (bits < ((dst - odst) * 8))
bits = (dst - odst) * 8;
+ /*
+ * If there are no additional bits specified for a class D
+ * address adjust bits to 4.
+ */
+ if (bits == 8 && *odst == 224)
+ bits = 4;
}
/* Extend network to cover the actual mask. */
while (bits > ((dst - odst) * 8)) {

re:

> To: Steve Atkins <steve(at)blighty(dot)com>
> Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, paul(at)vix(dot)com
> Subject: Re: [HACKERS] [BUGS] Bug in create operator and/or initdb
> Comments: In-reply-to Steve Atkins <steve(at)blighty(dot)com>
> message dated "Mon, 31 Jan 2005 07:23:05 -0800"
> Date: Mon, 31 Jan 2005 12:16:26 -0500
> From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
>
> Steve Atkins <steve(at)blighty(dot)com> writes:
> > The cidr type, including it's external interface, is simply broken.
>
> That is a large claim that I don't think you have demonstrated.
> The only one of your examples that seems to me to contradict the
> documentation is this one:
>
> steve=# select '224.0.0.0'::cidr;
> cidr
> -------------
> 224.0.0.0/4
>
> which should be /32 according to what the docs say:
>
> : If y is omitted, it is calculated using assumptions from the older
> : classful network numbering system, except that it will be at least large
> : enough to include all of the octets written in the input.
>
> The bogus netmask is in turn responsible for this case:
>
> steve=# select '224.10.0.0'::cidr;
> ERROR: invalid cidr value: "224.10.0.0"
> DETAIL: Value has bits set to right of mask.
>
>
> Looking at the source code, there seems to be a special case for "class D"
> network numbers that causes the code not to extend y to cover the
> supplied inputs:
>
> /* If no CIDR spec was given, infer width from net class. */
> if (bits == -1)
> {
> if (*odst >= 240) /* Class E */
> bits = 32;
> else if (*odst >= 224) /* Class D */
> bits = 4;
> else if (*odst >= 192) /* Class C */
> bits = 24;
> else if (*odst >= 128) /* Class B */
> bits = 16;
> else /* Class A */
> bits = 8;
> /* If imputed mask is narrower than specified octets, widen. */
> if (bits >= 8 && bits < ((dst - odst) * 8))
> ^^^^^^^^^
> bits = (dst - odst) * 8;
> }
>
> I think the test for "bits >= 8" should be removed. Does anyone know
> why it's there?
>
> regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Manfred Koizar 2005-01-31 19:35:25 Re: Group-count estimation statistics
Previous Message Tom Lane 2005-01-31 19:26:24 Re: Allow GRANT/REVOKE permissions to be applied to all schema objects with one command