Re: DatumGetInetP buggy

On 08.11.2011 11:22, Boszormenyi Zoltan wrote:
> Hi,
>
> I wanted to do some transformation on an inet value from
> an SPI-using function. The inet Datum passed from SPI_getbinval()
> to the values array in heap_form_tuple() obviously gives good data
> to the frontend. When I use DatumGetInetP() on the Datum,
> the structure passed to me is corrupt:
>
> zozo=# select * from inet_test() as (id integer, i1 inet, i2 inet);
> NOTICE:  i1 family=CORRUPTED
> NOTICE:  i1 family=CORRUPTED
> NOTICE:  i1 family=CORRUPTED
>   id |     i1      |      i2
> ----+-------------+---------------
>    1 | 192.168.0.1 | 192.168.0.101
>    2 | 192.168.0.2 | 192.168.0.102
>    3 | 192.168.0.3 | 192.168.0.103
> (3 rows)
>
> I looked at utils/inet.h and DatumGetInetP() uses PG_DETOAST_DATUM_PACKED().
> fmgr.h warns about PG_DETOAST_DATUM_PACKED() that it may give you
> an unaligned pointer. Indeed, using PG_DETOAST_DATUM() instead of the
> _PACKED variant on the Datum give me a well formed inet structure:

Hmm, it seems to be intentional, but I agree it's very much contrary to 
the usual convention that DatumGetXXXP() returns a detoasted and 
depacked datum. I think we should change it. I propose the attached 
patch. It changes DatumGetInetP() to do PG_DETOAST_DATUM(), and adds new 
DatumGetInetPP() macro to return the packed version. I also moved the 
access macros like ip_family() from network.c to inet.h, so that they're 
available for whoever wants to look at the fields without having to depack.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com