Hi all,

The inet regression test has been failed on my LinuxPPC. While
investigating the reason, I found a code that doesn't work on
LinuxPPC. From network_broadcast() in utils/adt/network.c:

int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));

Here ip_bits() returns from (unsigned char)0 to 32. My question is:
what is the correct result of (0xffffffff >> ip_bits())?

1. 0x0
2. 0xffffffff (actually does nothing)

LinuxPPC is 1. FreeBSD and Solaris are 2. network_broadcast() seems to
expect 2. My guess is shifting over 32bit against a 32bit integer is
not permitted and the result is platform depedent. If this would true,
it could be said that network_broadcast() has a portabilty
problem. Comments?
---
Tatsuo Ishii

Mon, 22 Feb 1999 11:54:39 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
inet data type regression test fails
> The inet regression test has been failed on my LinuxPPC. While
> investigating the reason, I found a code that doesn't work on
> LinuxPPC. From network_broadcast() in utils/adt/network.c:
> 
> int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> 
> Here ip_bits() returns from (unsigned char)0 to 32. My question is:
> what is the correct result of (0xffffffff >> ip_bits())?

I should have said that:

what is the correct result of (0xffffffff >> ip_bits()) if ip_bits() == 32?

> 1. 0x0
> 2. 0xffffffff (actually does nothing)
> 
> LinuxPPC is 1. FreeBSD and Solaris are 2. network_broadcast() seems to
> expect 2. My guess is shifting over 32bit against a 32bit integer is
> not permitted and the result is platform depedent. If this would true,
> it could be said that network_broadcast() has a portabilty
> problem. Comments?
> ---
> Tatsuo Ishii
> 


Mon, 22 Feb 1999 23:09:08 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
Re: [HACKERS] inet data type regression test fails 
> what is the correct result of
>   (0xffffffff >> ip_bits()) if ip_bits() == 32?
> > 1. 0x0
> > 2. 0xffffffff (actually does nothing)

In both cases, it does something. I haven't looked it up, but I suspect
that this is an implementation-defined result, since you are seeing the
results of right-shifting the sign bit *or* the high bit downward. On
some systems it does not propagate, and on others it does.

Have you tried coercing 0xffffffff to be a signed char? The better
solution is probably to mask the result before comparing, or handling
shifts greater than 31 as a special case. For example,

  /* It's an IP V4 address: */
  int addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));

becomes

  /* It's an IP V4 address: */
  int addr = htonl(ntohl(ip_v4addr(ip));
  if (ip_bits(ip) < sizeof(addr))
    addr |= (0xffffffff >> ip_bits(ip)));

or something like that...

                        - Tom

Tue, 23 Feb 1999 02:49:26 +0000
"Thomas G. Lockhart" <lockhart@alumni.caltech.edu>
Re: [HACKERS] inet data type regression test fails
>> what is the correct result of
>>   (0xffffffff >> ip_bits()) if ip_bits() == 32?
>> > 1. 0x0
>> > 2. 0xffffffff (actually does nothing)
>
>In both cases, it does something. I haven't looked it up, but I suspect
>that this is an implementation-defined result, since you are seeing the
>results of right-shifting the sign bit *or* the high bit downward. On
>some systems it does not propagate, and on others it does.
>
>Have you tried coercing 0xffffffff to be a signed char? The better
>solution is probably to mask the result before comparing, or handling
>shifts greater than 31 as a special case. For example,
>
>  /* It's an IP V4 address: */
>  int addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
>
>becomes
>
>  /* It's an IP V4 address: */
>  int addr = htonl(ntohl(ip_v4addr(ip));
>  if (ip_bits(ip) < sizeof(addr))
>    addr |= (0xffffffff >> ip_bits(ip)));
>
>or something like that...

Thank you for the advice.  I concluded that current inet code has a
portability problem. Included patches should be applied to both
current and 6.4 tree. I have tested on LinuxPPC, FreeBSD and Solaris
2.6. Now the inet regression tests on these platforms are all happy.
---
Tatsuo Ishii
------------------------------------------------------------------------
*** pgsql/src/backend/utils/adt/network.c.orig	Fri Jan  1 13:17:13 1999
--- pgsql/src/backend/utils/adt/network.c	Tue Feb 23 21:31:41 1999
***************
*** 356,362 ****
  	if (ip_family(ip) == AF_INET)
  	{
  		/* It's an IP V4 address: */
! 		int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
  
  		if (inet_net_ntop(AF_INET, &addr, 32, tmp, sizeof(tmp)) == NULL)
  		{
--- 356,367 ----
  	if (ip_family(ip) == AF_INET)
  	{
  		/* It's an IP V4 address: */
! 		int addr;
! 		unsigned long mask = 0xffffffff;
! 
! 		if (ip_bits(ip) < 32)
! 			mask >>= ip_bits(ip);
! 		addr = htonl(ntohl(ip_v4addr(ip)) | mask);
  
  		if (inet_net_ntop(AF_INET, &addr, 32, tmp, sizeof(tmp)) == NULL)
  		{

Wed, 24 Feb 1999 12:02:57 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
Re: [HACKERS] inet data type regression test fails 
Applied.


> >> what is the correct result of
> >>   (0xffffffff >> ip_bits()) if ip_bits() == 32?
> >> > 1. 0x0
> >> > 2. 0xffffffff (actually does nothing)
> >
> >In both cases, it does something. I haven't looked it up, but I suspect
> >that this is an implementation-defined result, since you are seeing the
> >results of right-shifting the sign bit *or* the high bit downward. On
> >some systems it does not propagate, and on others it does.
> >
> >Have you tried coercing 0xffffffff to be a signed char? The better
> >solution is probably to mask the result before comparing, or handling
> >shifts greater than 31 as a special case. For example,
> >
> >  /* It's an IP V4 address: */
> >  int addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> >
> >becomes
> >
> >  /* It's an IP V4 address: */
> >  int addr = htonl(ntohl(ip_v4addr(ip));
> >  if (ip_bits(ip) < sizeof(addr))
> >    addr |= (0xffffffff >> ip_bits(ip)));
> >
> >or something like that...
> 
> Thank you for the advice.  I concluded that current inet code has a
> portability problem. Included patches should be applied to both
> current and 6.4 tree. I have tested on LinuxPPC, FreeBSD and Solaris
> 2.6. Now the inet regression tests on these platforms are all happy.
> ---
> Tatsuo Ishii
> ------------------------------------------------------------------------
> *** pgsql/src/backend/utils/adt/network.c.orig	Fri Jan  1 13:17:13 1999
> --- pgsql/src/backend/utils/adt/network.c	Tue Feb 23 21:31:41 1999
> ***************
> *** 356,362 ****
>   	if (ip_family(ip) == AF_INET)
>   	{
>   		/* It's an IP V4 address: */
> ! 		int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
>   
>   		if (inet_net_ntop(AF_INET, &addr, 32, tmp, sizeof(tmp)) == NULL)
>   		{
> --- 356,367 ----
>   	if (ip_family(ip) == AF_INET)
>   	{
>   		/* It's an IP V4 address: */
> ! 		int addr;
> ! 		unsigned long mask = 0xffffffff;
> ! 
> ! 		if (ip_bits(ip) < 32)
> ! 			mask >>= ip_bits(ip);
> ! 		addr = htonl(ntohl(ip_v4addr(ip)) | mask);
>   
>   		if (inet_net_ntop(AF_INET, &addr, 32, tmp, sizeof(tmp)) == NULL)
>   		{
> 
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tue, 23 Feb 1999 22:16:24 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] inet data type regression test fails
> Hi all,
> 
> The inet regression test has been failed on my LinuxPPC. While
> investigating the reason, I found a code that doesn't work on
> LinuxPPC. From network_broadcast() in utils/adt/network.c:
> 
> int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> 
> Here ip_bits() returns from (unsigned char)0 to 32. My question is:
> what is the correct result of (0xffffffff >> ip_bits())?
> 
> 1. 0x0
> 2. 0xffffffff (actually does nothing)
> 
> LinuxPPC is 1. FreeBSD and Solaris are 2. network_broadcast() seems to
> expect 2. My guess is shifting over 32bit against a 32bit integer is
> not permitted and the result is platform depedent. If this would true,
> it could be said that network_broadcast() has a portabilty
> problem. Comments?

If 0xffffff is unsigned, it should allow the right shift.  When you say
1 or 2, how do you get those values?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 15 Mar 1999 10:25:25 -0500 (EST)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] inet data type regression test fails
> > The inet regression test has been failed on my LinuxPPC. While
> > investigating the reason, I found a code that doesn't work on
> > LinuxPPC. From network_broadcast() in utils/adt/network.c:
> > 
> > int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> > 
> > Here ip_bits() returns from (unsigned char)0 to 32. My question is:
> > what is the correct result of (0xffffffff >> ip_bits())?
> > 
> > 1. 0x0
> > 2. 0xffffffff (actually does nothing)
> > 
> > LinuxPPC is 1. FreeBSD and Solaris are 2. network_broadcast() seems to
> > expect 2. My guess is shifting over 32bit against a 32bit integer is
> > not permitted and the result is platform depedent. If this would true,
> > it could be said that network_broadcast() has a portabilty
> > problem. Comments?
> 
> If 0xffffff is unsigned, it should allow the right shift.  

No. it does not depend on if 0xffffffff is signed or not.  Suppose a
is signed and b is unsigned. In "a >> b", before doing an actual
shifting operation, a is "upgraded" to unsigned by the compiler.

>When you say
> 1 or 2, how do you get those values?

You could observe the "32 bit shift efect" I mentioned in the previous
mail by running following small program.

main()
{
  unsigned char c;
  for (c = 0;c <=32;c++) {
    printf("shift: %d result: 0x%08x\n",c,0xffffffff >> c);
  }
}
---
Tatsuo Ishii

Tue, 16 Mar 1999 10:35:04 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
Re: [HACKERS] inet data type regression test fails 

Can someone comment on this one?  Is it fixed?


> Hi all,
> 
> The inet regression test has been failed on my LinuxPPC. While
> investigating the reason, I found a code that doesn't work on
> LinuxPPC. From network_broadcast() in utils/adt/network.c:
> 
> int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> 
> Here ip_bits() returns from (unsigned char)0 to 32. My question is:
> what is the correct result of (0xffffffff >> ip_bits())?
> 
> 1. 0x0
> 2. 0xffffffff (actually does nothing)
> 
> LinuxPPC is 1. FreeBSD and Solaris are 2. network_broadcast() seems to
> expect 2. My guess is shifting over 32bit against a 32bit integer is
> not permitted and the result is platform depedent. If this would true,
> it could be said that network_broadcast() has a portabilty
> problem. Comments?
> ---
> Tatsuo Ishii
> 
> 


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Sun, 9 May 1999 10:56:07 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] inet data type regression test fails
> Can someone comment on this one?  Is it fixed?
> > The inet regression test has been failed on my LinuxPPC. 
> > My guess is shifting over 32bit against a 32bit integer is
> > not permitted and the result is platform depedent.

Yes, it is fixed. You applied the patches :)

backend/utils/adt/network.c:
revision 1.6
date: 1999/02/24 03:17:05;  author: momjian;  state: Exp;  lines: +7
-2
Thank you for the advice.  I concluded that current inet code has a
portability problem. Included patches should be applied to both
current and 6.4 tree. I have tested on LinuxPPC, FreeBSD and Solaris
2.6. Now the inet regression tests on these platforms are all happy.

                       - Thomas

-- 
Thomas Lockhart				lockhart@alumni.caltech.edu
South Pasadena, California

Sun, 09 May 1999 15:24:13 +0000
Thomas Lockhart <lockhart@alumni.caltech.edu>
Re: [HACKERS] inet data type regression test fails
On Sun, 9 May 1999, Bruce Momjian wrote:

> > int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));

There needs to be a UL on the end of that constant. Otherwise it depends
on whether or not the compiler chooses to make it signed or unsigned. Not
only that, but shifting by >=32 is undefined... Intel chipsets will go mod
32 and change 32 to 0.

Taral


Sun, 9 May 1999 15:51:10 -0500 (CDT)
Taral <taral@taral.net>
Re: [HACKERS] inet data type regression test fails
> On Sun, 9 May 1999, Bruce Momjian wrote:
> 
> > > int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> 
> There needs to be a UL on the end of that constant. Otherwise it depends
> on whether or not the compiler chooses to make it signed or unsigned. Not
> only that, but shifting by >=32 is undefined... Intel chipsets will go mod
> 32 and change 32 to 0.
> 

Anyone want to supply a patch?


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 10 May 1999 14:28:22 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] inet data type regression test fails
> > > > int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> > 
> > There needs to be a UL on the end of that constant. Otherwise it depends
> > on whether or not the compiler chooses to make it signed or unsigned. Not
> > only that, but shifting by >=32 is undefined... Intel chipsets will go mod
> > 32 and change 32 to 0.
> > 
> 
> Anyone want to supply a patch?

This has been already fixed. Now it looks like:

		unsigned long mask = 0xffffffff;

		if (ip_bits(ip) < 32)
			mask >>= ip_bits(ip);
		addr = htonl(ntohl(ip_v4addr(ip)) | mask);
---
Tatsuo Ishii

Tue, 11 May 1999 09:54:25 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
Re: [HACKERS] inet data type regression test fails 
> > > > > int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
> > > 
> > > There needs to be a UL on the end of that constant. Otherwise it depends
> > > on whether or not the compiler chooses to make it signed or unsigned. Not
> > > only that, but shifting by >=32 is undefined... Intel chipsets will go mod
> > > 32 and change 32 to 0.
> > > 
> > 
> > Anyone want to supply a patch?
> 
> This has been already fixed. Now it looks like:
> 
> 		unsigned long mask = 0xffffffff;
> 
> 		if (ip_bits(ip) < 32)
> 			mask >>= ip_bits(ip);
> 		addr = htonl(ntohl(ip_v4addr(ip)) | mask);

Oh.  Very nice.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Mon, 10 May 1999 21:05:08 -0400 (EDT)
Bruce Momjian <maillist@candle.pha.pa.us>
Re: [HACKERS] inet data type regression test fails
On Tue, 11 May 1999, Tatsuo Ishii wrote:

> 		unsigned long mask = 0xffffffff;
> 
> 		if (ip_bits(ip) < 32)
> 			mask >>= ip_bits(ip);
> 		addr = htonl(ntohl(ip_v4addr(ip)) | mask);

That's wrong too. There needs to be:

else
	mask = 0;

Taral


Mon, 10 May 1999 20:05:28 -0500 (CDT)
Taral <taral@taral.net>
Re: [HACKERS] inet data type regression test fails 
>On Tue, 11 May 1999, Tatsuo Ishii wrote:
>
>> 		unsigned long mask = 0xffffffff;
>> 
>> 		if (ip_bits(ip) < 32)
>> 			mask >>= ip_bits(ip);
>> 		addr = htonl(ntohl(ip_v4addr(ip)) | mask);
>
>That's wrong too. There needs to be:
>
>else
>	mask = 0;
>
>Taral

No. it is expected addr == 0xffffffff if ip_bits() returns >= 32. This 
is how the function (network_broadcast()) is made.
See included posting.

>From: Tatsuo Ishii <t-ishii@sra.co.jp>
>To: hackers@postgreSQL.org
>Subject: [HACKERS] inet data type regression test fails
>Date: Mon, 22 Feb 1999 11:54:39 +0900
>
>Hi all,
>
>The inet regression test has been failed on my LinuxPPC. While
>investigating the reason, I found a code that doesn't work on
>LinuxPPC. From network_broadcast() in utils/adt/network.c:
>
>int	addr = htonl(ntohl(ip_v4addr(ip)) | (0xffffffff >> ip_bits(ip)));
>
>Here ip_bits() returns from (unsigned char)0 to 32. My question is:
>what is the correct result of (0xffffffff >> ip_bits())?
>
>1. 0x0
>2. 0xffffffff (actually does nothing)
>
>LinuxPPC is 1. FreeBSD and Solaris are 2. network_broadcast() seems to
>expect 2. My guess is shifting over 32bit against a 32bit integer is
>not permitted and the result is platform depedent. If this would true,
>it could be said that network_broadcast() has a portabilty
>problem. Comments?
>---
>Tatsuo Ishii
>
>

Tue, 11 May 1999 10:22:16 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
Re: [HACKERS] inet data type regression test fails 
On Tue, 11 May 1999, Tatsuo Ishii wrote:

> >On Tue, 11 May 1999, Tatsuo Ishii wrote:
> >
> >> 		unsigned long mask = 0xffffffff;
> >> 
> >> 		if (ip_bits(ip) < 32)
> >> 			mask >>= ip_bits(ip);
> >> 		addr = htonl(ntohl(ip_v4addr(ip)) | mask);

> No. it is expected addr == 0xffffffff if ip_bits() returns >= 32. This 
> is how the function (network_broadcast()) is made.
> See included posting.

ip_bits(ip) = 0  => mask = 0xffffffff
ip_bits(ip) = 31 => mask = 1
ip_bits(ip) = 32 => mask = 0xffffffff

You sure?

Taral


Mon, 10 May 1999 20:48:54 -0500 (CDT)
Taral <taral@taral.net>
Re: [HACKERS] inet data type regression test fails 
>> >> 		unsigned long mask = 0xffffffff;
>> >> 
>> >> 		if (ip_bits(ip) < 32)
>> >> 			mask >>= ip_bits(ip);
>> >> 		addr = htonl(ntohl(ip_v4addr(ip)) | mask);
>
>> No. it is expected addr == 0xffffffff if ip_bits() returns >= 32. This 
>> is how the function (network_broadcast()) is made.
>> See included posting.
>
>ip_bits(ip) = 0  => mask = 0xffffffff
>ip_bits(ip) = 31 => mask = 1
>ip_bits(ip) = 32 => mask = 0xffffffff
>
>You sure?

Yes. That's exactly what I expected.
---
Tatsuo Ishii


Tue, 11 May 1999 19:13:59 +0900
Tatsuo Ishii <t-ishii@sra.co.jp>
Re: [HACKERS] inet data type regression test fails 