SEO

"Marko Kreen" <markokr@gmail.com> writes:
> Following function crashes plpython on x86-64 / gcc 4.1.2 / debian 4.0:
> CREATE FUNCTION crashme(str_len integer)
> RETURNS text AS $$
> raise Exception("X" * str_len)
> $$ LANGUAGE plpythonu;

> SELECT crashme(1000);

> Problem turns out to be va_list handling in PLy_vprintf() which
> uses same va_list repeatedly. Fix is to va_copy to temp variable.

This patch isn't acceptable because va_copy() isn't portable.

I'm kinda wondering why PLy_printf and the functions after it even
exist. They look like rather poorly done reimplementations of
functionality that exists elsewhere in the backend (eg, stringinfo.c).
In particular, why malloc and not palloc?

regards, tom lane

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

Tom Lane escribió:
> "Marko Kreen" <markokr@gmail.com> writes:
> > Following function crashes plpython on x86-64 / gcc 4.1.2 / debian 4.0:
> > CREATE FUNCTION crashme(str_len integer)
> > RETURNS text AS $$
> > raise Exception("X" * str_len)
> > $$ LANGUAGE plpythonu;
>
> > SELECT crashme(1000);
>
> > Problem turns out to be va_list handling in PLy_vprintf() which
> > uses same va_list repeatedly. Fix is to va_copy to temp variable.
>
> This patch isn't acceptable because va_copy() isn't portable.
>
> I'm kinda wondering why PLy_printf and the functions after it even
> exist. They look like rather poorly done reimplementations of
> functionality that exists elsewhere in the backend (eg, stringinfo.c).
> In particular, why malloc and not palloc?

See attached patch.

I didn't bother to change the PLy_malloc and friends because I think
that would be too much change for 8.3. PLy_realloc is gone though
because there are no callers left after this patch.

--
Alvaro Herrera http://www.flickr.com/photos/alvherre/
"Ninguna manada de bestias tiene una voz tan horrible como la humana" (Orual)


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

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Tom Lane escribió:
>> This patch isn't acceptable because va_copy() isn't portable.
>>
>> I'm kinda wondering why PLy_printf and the functions after it even
>> exist. They look like rather poorly done reimplementations of
>> functionality that exists elsewhere in the backend (eg, stringinfo.c).
>> In particular, why malloc and not palloc?

> See attached patch.

> I didn't bother to change the PLy_malloc and friends because I think
> that would be too much change for 8.3. PLy_realloc is gone though
> because there are no callers left after this patch.

Yeah, that is about what I was thinking too. Are you set up to
back-patch this as far as 7.3? If so, please apply.

regards, tom lane

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

http://archives.postgresql.org

Tom Lane escribió:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > Tom Lane escribió:
> >> This patch isn't acceptable because va_copy() isn't portable.
> >>
> >> I'm kinda wondering why PLy_printf and the functions after it even
> >> exist. They look like rather poorly done reimplementations of
> >> functionality that exists elsewhere in the backend (eg, stringinfo.c).
> >> In particular, why malloc and not palloc?
>
> > See attached patch.
>
> > I didn't bother to change the PLy_malloc and friends because I think
> > that would be too much change for 8.3. PLy_realloc is gone though
> > because there are no callers left after this patch.
>
> Yeah, that is about what I was thinking too. Are you set up to
> back-patch this as far as 7.3? If so, please apply.

Yeah, I think I'm done with the patch, but the regression tests do not
work from 8.0 back in VPATH builds it seems, so I'll have to rebuild
them to check.

One problem here is that 7.3 does not have appendStringInfoVA. From 7.4
onwards there is no problem. Should I backport the code from 7.4? (On
a first try to do it I introduced a nasty bug, so it's not as easy as
a cut'n paste).

--
Alvaro Herrera http://www.advogato.org/person/alvherre
"Postgres is bloatware by design: it was built to house
PhD theses." (Joey Hellerstein, SIGMOD annual conference 2002)

---------------------------(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

Alvaro Herrera escribió:

> One problem here is that 7.3 does not have appendStringInfoVA. From 7.4
> onwards there is no problem. Should I backport the code from 7.4? (On
> a first try to do it I introduced a nasty bug, so it's not as easy as
> a cut'n paste).

No nasty bug anywhere -- just that I neglected a initStringInfo call
while manually backporting the patch.

However, in testing I noticed this problem:

alvherre=# select crashme(1);
ERROR: plpython: Unable to create rexec.RExec instance
exceptions.RuntimeError: This code is not secure in Python 2.2 and 2.3

There doesn't seem to be a way to make it work.

What I'm going to do is commit the fix to just 7.4 onwards.

--
Alvaro Herrera http://www.advogato.org/person/alvherre
"We're here to devour each other alive" (Hobbes)

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> What I'm going to do is commit the fix to just 7.4 onwards.

Fair enough. 7.3 is on life support anyway --- I'm not in favor of
heroic efforts to port patches that far back.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

On 11/22/07, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> One problem here is that 7.3 does not have appendStringInfoVA. From 7.4
> onwards there is no problem. Should I backport the code from 7.4? (On
> a first try to do it I introduced a nasty bug, so it's not as easy as
> a cut'n paste).

Just a note - appendStringInfoVA should take *nprinted argument.

Can this be added in 8.4? Should make usage saner.

--
marko

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

"Marko Kreen" <markokr@gmail.com> writes:
> Just a note - appendStringInfoVA should take *nprinted argument.

Why? I can't imagine any real use for it. If you're thinking that
it could provide a guide as to what to resize the buffer to, think
again.

regards, tom lane

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

http://archives.postgresql.org

On 11/23/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Marko Kreen" <markokr@gmail.com> writes:
> > Just a note - appendStringInfoVA should take *nprinted argument.
>
> Why? I can't imagine any real use for it. If you're thinking that
> it could provide a guide as to what to resize the buffer to, think
> again.

It seems perfectly appropriate:

If the output was truncated due to this limit then the return
value is the number of characters (not including the trailing
'\0') which would have been written to the final string if
enough space had been available.

What problem do you see?

Note that I don't want to fix unnecessary memory usage with that
but several reallocs in case some args are large.

--
marko

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

"Marko Kreen" <markokr@gmail.com> writes:
> On 11/23/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Why? I can't imagine any real use for it. If you're thinking that
>> it could provide a guide as to what to resize the buffer to, think
>> again.

> If the output was truncated due to this limit then the return
> value is the number of characters (not including the trailing
> '\0') which would have been written to the final string if
> enough space had been available.

> What problem do you see?

The problem is that you are quoting from some particular system's
manual, and not any kind of standard ... much less any standard that
every platform we support follows.

The real-world situation is that we are lucky to be able to tell
vsnprintf success from failure at all :-(

regards, tom lane

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