Unix Technical Forum

suspicious pointer/integer coersion

This is a discussion on suspicious pointer/integer coersion within the pgsql Hackers forums, part of the PostgreSQL category; --> I have just noticed this code in plperl.c: hv_store(plperl_proc_hash, internal_proname, proname_len, newSViv((IV) prodesc), 0); basically, here prodesc is a ...


Go Back   Unix Technical Forum > Database Server Software > PostgreSQL > pgsql Hackers

FAQ Members List Calendar Search Today's Posts Mark Forums Read
  #1 (permalink)  
Old 04-11-2008, 04:48 AM
Andrew Dunstan
 
Posts: n/a
Default suspicious pointer/integer coersion


I have just noticed this code in plperl.c:

hv_store(plperl_proc_hash, internal_proname, proname_len,
newSViv((IV) prodesc), 0);

basically, here prodesc is a pointer to a struct, and we are storing it
as an integer in a perl hash for easy lookup by stringified oid.

How safe is this conversion on 64 bit platforms?

cheers

andrew


---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #2 (permalink)  
Old 04-11-2008, 04:48 AM
Tom Lane
 
Posts: n/a
Default Re: suspicious pointer/integer coersion

Andrew Dunstan <andrew@dunslane.net> writes:
> I have just noticed this code in plperl.c:


> hv_store(plperl_proc_hash, internal_proname, proname_len,
> newSViv((IV) prodesc), 0);


> basically, here prodesc is a pointer to a struct, and we are storing it
> as an integer in a perl hash for easy lookup by stringified oid.


> How safe is this conversion on 64 bit platforms?


Not at all. I'd vote for converting the whole thing to a dynahash
hashtable ...

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #3 (permalink)  
Old 04-11-2008, 04:48 AM
Andrew Dunstan
 
Posts: n/a
Default Re: suspicious pointer/integer coersion



Tom Lane wrote:

>Andrew Dunstan <andrew@dunslane.net> writes:
>
>
>>I have just noticed this code in plperl.c:
>>
>>

>
>
>
>> hv_store(plperl_proc_hash, internal_proname, proname_len,
>> newSViv((IV) prodesc), 0);
>>
>>

>
>
>
>>basically, here prodesc is a pointer to a struct, and we are storing it
>>as an integer in a perl hash for easy lookup by stringified oid.
>>
>>

>
>
>
>>How safe is this conversion on 64 bit platforms?
>>
>>

>
>Not at all. I'd vote for converting the whole thing to a dynahash
>hashtable ...
>
>
>
>


Works for me. There are some other things about the procdesc stuff I'm
trying to sort out (especially if we should be storing per-call info
inside it).

Cleaning this up is definitely a TODO.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #4 (permalink)  
Old 04-11-2008, 04:48 AM
Tom Lane
 
Posts: n/a
Default Re: suspicious pointer/integer coersion

Andrew Dunstan <andrew@dunslane.net> writes:
> Works for me. There are some other things about the procdesc stuff I'm
> trying to sort out (especially if we should be storing per-call info
> inside it).


Hmm, probably not ... check to see if a recursive plperl function
behaves sanely. (This might not have been much of an issue before
we had SPI support in plperl, since there was no way to recurse;
but it is an issue now.)

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #5 (permalink)  
Old 04-11-2008, 04:49 AM
Andrew Dunstan
 
Posts: n/a
Default Re: suspicious pointer/integer coersion



Tom Lane wrote:

>Andrew Dunstan <andrew@dunslane.net> writes:
>
>
>>Works for me. There are some other things about the procdesc stuff I'm
>>trying to sort out (especially if we should be storing per-call info
>>inside it).
>>
>>

>
>Hmm, probably not ... check to see if a recursive plperl function
>behaves sanely. (This might not have been much of an issue before
>we had SPI support in plperl, since there was no way to recurse;
>but it is an issue now.)
>
>


Behaviour is not good (see below for proof).

ISTM we'll need some sort of implicit of explicit stack of per-call
data. The trick will be getting it to behave right under error recovery.

cheers

andrew


[andrew inst]$ bin/psql -e -f recurse.sql
create or replace function recurse(i int) returns setof text language plperl
as $$

my $i = shift;
elog(NOTICE,"i = $i");
foreach my $x (1..$i)
{
return_next "hello $x";
}
if ($i > 2)
{
my $z = $i-1;
my $cursor = spi_query("select * from recurse($z)");
while (defined(my $row = spi_fetchrow($cursor)))
{
return_next "recurse $i: $row";
}
}
return undef;

$$;
CREATE FUNCTION
select * from recurse(2);
psql:recurse.sql:24: NOTICE: i = 2
recurse
---------
hello 1
hello 2
(2 rows)

select * from recurse(3);
psql:recurse.sql:25: NOTICE: i = 3
psql:recurse.sql:25: NOTICE: i = 2
psql:recurse.sql:25: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
psql:recurse.sql:25: connection to server was lost
[andrew inst]$

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #6 (permalink)  
Old 04-11-2008, 04:49 AM
Andrew Dunstan
 
Posts: n/a
Default Re: suspicious pointer/integer coersion



Andrew Dunstan wrote:

> Tom Lane wrote:
>
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>
>>> Works for me. There are some other things about the procdesc stuff
>>> I'm trying to sort out (especially if we should be storing per-call
>>> info inside it).
>>>

>>
>>
>> Hmm, probably not ... check to see if a recursive plperl function
>> behaves sanely. (This might not have been much of an issue before
>> we had SPI support in plperl, since there was no way to recurse;
>> but it is an issue now.)

>
>
> Behaviour is not good (see below for proof).
>
> ISTM we'll need some sort of implicit of explicit stack of per-call
> data. The trick will be getting it to behave right under error recovery.




Looking further ... we already do this implicitly for prodesc in the
call handler - we would just need to do the same thing for per-call
structures and divorce them from prodesc, which can be repeated on the
implicit stack.

I'll work on that - changes should be quite small.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #7 (permalink)  
Old 04-11-2008, 04:49 AM
Christopher Kings-Lynne
 
Posts: n/a
Default Re: suspicious pointer/integer coersion

> Looking further ... we already do this implicitly for prodesc in the
> call handler - we would just need to do the same thing for per-call
> structures and divorce them from prodesc, which can be repeated on the
> implicit stack.
>
> I'll work on that - changes should be quite small.


Sounds like recursive calls are somethign that should be tested for PLs
in the regressions...

Chris


---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #8 (permalink)  
Old 04-11-2008, 04:49 AM
Andrew Dunstan
 
Posts: n/a
Default Re: suspicious pointer/integer coersion



Andrew Dunstan wrote:

>
>
>
>
> Looking further ... we already do this implicitly for prodesc in the
> call handler - we would just need to do the same thing for per-call
> structures and divorce them from prodesc, which can be repeated on the
> implicit stack.
>
> I'll work on that - changes should be quite small.
>


Attached is a patch that fixes both a recently introduced problem with
recursion and a problem with array returns that became evident as a
result of not throwing away non-fatal warnings (thanks to David Fetter
for noticing this). Regression test updates to include both cases are
included in the patch.

I will start looking at putting the procedure descriptors in a dynahash.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
  #9 (permalink)  
Old 04-11-2008, 04:49 AM
Andrew Dunstan
 
Posts: n/a
Default Re: suspicious pointer/integer coersion



Andrew Dunstan wrote:

>
>
> Andrew Dunstan wrote:
>
>>
>>
>>
>>
>> Looking further ... we already do this implicitly for prodesc in the
>> call handler - we would just need to do the same thing for per-call
>> structures and divorce them from prodesc, which can be repeated on
>> the implicit stack.
>>
>> I'll work on that - changes should be quite small.
>>

>
> Attached is a patch that fixes (I hope) both a recently introduced
> problem with recursion and a problem with array returns that became
> evident as a result of not throwing away non-fatal warnings (thanks to
> David Fetter for noticing this). Regression test updates to include
> both cases are included in the patch.
>
> I will start looking at putting the procedure descriptors in a dynahash.
>
>


and here's the patch this time.

cheers


andrew


---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Digg this Post!Add Post to del.icio.usBookmark Post in TechnoratiFurl this Post!
Reply With Quote
Reply


Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On
Forum Jump


All times are GMT. The time now is 03:00 PM.


Powered by vBulletin® Version 3.6.5
Copyright ©2000 - 2008, Jelsoft Enterprises Ltd.
SEO by vBSEO 3.2.0
www.UnixAdminTalk.com