Re: [COMMITTERS] pgsql: Add traceback information to PL/Python errors

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Jan Urbański <wulczer(at)wulczer(dot)org>
Cc: Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add traceback information to PL/Python errors
Date: 2011-04-07 22:25:12
Message-ID: BANLkTi=GhtHT9EYHZrbFdYk-v27vWfPVFw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Thu, Apr 7, 2011 at 5:06 PM, Jan Urbański <wulczer(at)wulczer(dot)org> wrote:
> On 07/04/11 23:01, Robert Haas wrote:
>> On Wed, Apr 6, 2011 at 3:37 PM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
>>> Add traceback information to PL/Python errors
>>>
>>> This mimics the traceback information the Python interpreter prints
>>> with exceptions.
>>>
>>> Jan Urbański
>>
>> On my system this spits out a warning:
>>
>> plpython.c: In function ‘PLy_traceback’:
>> plpython.c:4487: warning: ‘s’ may be used uninitialized in this function
>> plpython.c:4487: note: ‘s’ was declared here
>>
>> That appears to be a live bug, unless it's guaranteed that lineno will
>> always be > 0.
>
> lineno should be > 0, unless Python is trying to tell us that the code
> frame originates from before the function.
>
>> Also, the loop test should really be written as current < lineno,
>> rather than current != lineno, just in case we should manage to pass a
>> lineno < 0, which with the current code would go into the tank and
>> spin.
>
> Yeah, good point.
>
>> This part looks pretty sketchy, too:
>>
>>     while (s && isspace((unsigned char) *s))
>>         s++;
>>
>> Perhaps we meant to test *s here.  It's hard to believe that we're
>> really intending to test whether the pointer has fallen off the end of
>> the address space and wrapped around to NULL.
>
> Gah, so short a function and so many things that I managed to get wrong.

Patch?

> There's also this:
> http://archives.postgresql.org/pgsql-hackers/2011-04/msg00334.php

Yep. I am assuming Peter will look at that one.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Jan Urbański 2011-04-07 22:39:40 Re: [COMMITTERS] pgsql: Add traceback information to PL/Python errors
Previous Message Jan Urbański 2011-04-07 21:06:38 Re: [COMMITTERS] pgsql: Add traceback information to PL/Python errors

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2011-04-07 22:26:37 Re: pg_upgrade bug found!
Previous Message Robert Haas 2011-04-07 22:21:02 Re: pg_upgrade bug found!