[Patches] #100874: prescod -> prescod [Accepted] Better error message with UnboundLocalError

Ka-Ping Yee ping@server1.lfw.org
Wed, 26 Jul 2000 02:16:28 -0500


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

Status: Accepted
Submitted By: prescod
Category: None
Summary: Better error message with UnboundLocalError
Assigned To: prescod
Patch: View Raw Patch
Date: 2000-Jul-12 20:29

2000-Jul-25 23:56 - tim_one wrote:
    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!

2000-Jul-18 06:32 - twouters wrote:
    The helper function is declared 'static', but not defined as such.
    
    Rest of the style issues seem solved ;)

2000-Jul-14 14:17 - prescod wrote:
    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.

2000-Jul-12 21:56 - tim_one wrote:
    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).

2000-Jul-25 23:56 - tim_one changed patch_status_id (previously Open)
2000-Jul-25 23:56 - tim_one changed assigned_to (previously tim_one)
2000-Jul-18 08:01 - prescod changed Patch Code (previously Modified - New Version)
2000-Jul-14 14:17 - prescod changed Patch Code (previously Modified - New Version)
2000-Jul-14 14:17 - prescod changed patch_status_id (previously Rejected)
2000-Jul-14 14:17 - prescod changed assigned_to (previously prescod)
2000-Jul-12 21:56 - tim_one changed patch_status_id (previously Open)
2000-Jul-12 21:56 - tim_one changed assigned_to (previously tim_one)
2000-Jul-12 20:50 - prescod changed assigned_to (previously none)
2000-Jul-12 20:50 - prescod changed summary (previously Better error message with UnboundLocalError (for Tim Peters))