[Patches] [Patch #100874] Better error message with UnboundLocalError

[noreply@sourceforge.net]

Patch #100874 has been updated. 

Project: 
Category: None
Status: Accepted
Summary: Better error message with UnboundLocalError

Follow-Ups:

Date: 2000-Jul-13 00:56
By: tim_one

Comment:
Rejected and assigned back to Paul.  I like the idea fine, the rejection is for style reasons and a repeated security hole.  Please make the code look like the rest of ceval.c -- idiosyncracies (that aren't Guido's <0.3 wink>) are harmful in group-maintained code.  Examples:

>    PyObject *nm=NULL;

Should be a space on each side of "=".  Ditto throughout the patch.  BTW, is "nm" meant to be an abbreviation for "name"?  If so, please spell out the two extra letters.  If it's meant to be an abbreviation for New Mexico, capitalize it <wink>.

>    if(!nm) break;

"if" isn't a function call:  use a space between "if" and "(".  It's also bad to slop "break" on the same line:  makes it much harder to set a useful breakpoint here in a debugger.  Everywhere else in the code this line would be spelled (if Guido wrote it -- and the "\t" business here is because SF comments don't preserve indentation):

\tif (!nm)
\t\tbreak

>  sprintf( buf, "Local variable %s

No space between "(" and "buf" here.

>    PyExc_UnboundLocalError, buf );

No space between "buf" and ")" here.

>   char buf[500];
> ... followed by an unchecked sprintf into buf.

This is a security hole:  Python doesn't restrict identifiers to being no longer than 500 chars, so a hacker can easily trick this code into overwriting the buffer bounds.  Easy but tedious to fix (e.g., #define the buf length, and use runtime code in conjunction with strncpy to guarantee buf's bounds are respected).

-------------------------------------------------------

Date: 2000-Jul-14 17:17
By: prescod

Comment:
Small helper function format_exc_check_arg makes it easy to generate one string and a argument-type exceptions. It may be overkill in terms of null checking...reviewer can decide. Otherwise, error messages are added to NameErrors and UnboundLocalErrors. That means that all exceptions thrown in ceval have associated error messages.

-------------------------------------------------------

Date: 2000-Jul-18 09:32
By: twouters

Comment:
The helper function is declared 'static', but not defined as such.

Rest of the style issues seem solved ;)
-------------------------------------------------------

Date: 2000-Jul-26 02:56
By: tim_one

Comment:
Accepted and assigned back to Paul.  Paul, the new format_exc_check_arg function is defined K&R-style, but we've moved to ANSI almost everywhere else in the source.  Would be nice if you fix that before checking this in.  Else I'll look for it and fix it myself.  Sorry for the delay in getting back to this!
-------------------------------------------------------

Date: 2000-Aug-15 14:10
By: tim_one

Comment:
Paul, this was accepted weeks ago.  Are you going to check it in?
-------------------------------------------------------

Date: 2000-Aug-19 21:47
By: tim_one

Comment:
Yo!  Paul!  Check it in already!
-------------------------------------------------------

-------------------------------------------------------
For more info, visit:

http://sourceforge.net/patch/?func=detailpatch&patch_id=100874&group_id=5470