[Patches] [ python-Patches-1691070 ] Fix for bug #1283289

[SourceForge.net]

Patches item #1691070, was opened at 2007-03-30 01:45
Message generated for change (Comment added) made by rupole
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1691070&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Roger Upole (rupole)
Assigned to: Nobody/Anonymous (nobody)
Summary: Fix for bug #1283289

Initial Comment:
This is a fix for [ 1283289 ] PyArg_ParseTupleAndKeywords gives misleading error message.  It also yields a 10-15% decrease in cpu usage, depending on the number of arguments.


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

>Comment By: Roger Upole (rupole)
Date: 2007-04-04 04:40

Message:
Logged In: YES 
user_id=771074
Originator: YES

File Added: test_getargs2.patch

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

Comment By: Roger Upole (rupole)
Date: 2007-04-04 00:39

Message:
Logged In: YES 
user_id=771074
Originator: YES

File Added: _testcapimodule.patch

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

Comment By: Roger Upole (rupole)
Date: 2007-04-04 00:26

Message:
Logged In: YES 
user_id=771074
Originator: YES

File Added: getargs_1.patch

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

Comment By: Paul Hankin (paulhankin)
Date: 2007-04-01 13:00

Message:
Logged In: YES 
user_id=1740099
Originator: NO

Patched code compiles and passes test suite, and looks cleaner than the
code it replaces. It works on the cases the bug report.

There should be unit tests added to Lib/test/test_getargs2; I've skimmed
the tests in there and can't see any that test combinations of kw, tuple
and positional arguments. The patch replaces significant chunks of arg
parsing code, so I'd be more convinced of its correctness if there were
some unit tests. Certainly there should be at least one test for the
bug the patch fixes.

The code is 99% written in the prevailing style, but:
Source contains several over-long lines, and non-standard comments, ie
you should use
 /* XXX something .. */
and not
 /* ??? something .. ??? */
Also there are a few minor spacing inconsistencies which would be nice
to tidy up.

The diff is one level too high up - it contains the directory Python2.5/
It applies cleanly to the trunk though, it just makes it slightly more
difficult
to apply the patch.

The original bug report requests a post to python-dev describing the new
error messages. I suggest this post includes all cases where
the new code produces a different message than the old.

I suggest to proceed:
1. Add unit tests, both for the bug-fix and for existing functionality
   which has been modified
2. Fix up comments and long lines
3. Check with python-dev about new error messages


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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1691070&group_id=5470