[Patches] [Patch #101544] Fix for 110609: Allow large long integers in formatting

noreply@sourceforge.net noreply@sourceforge.net
Tue, 19 Sep 2000 21:06:33 -0700


Patch #101544 has been updated. 

Project: 
Category: core (C code)
Status: Open
Summary: Fix for 110609: Allow large long integers in formatting

Follow-Ups:

Date: 2000-Sep-18 21:03
By: gvanrossum

Comment:
For Tim, because he likes numerical tweakage.
(Tim, you may also give this to Marc-Andre, since it affects Unicode.)
-------------------------------------------------------

Date: 2000-Sep-19 21:06
By: tim_one

Comment:
I'm afraid there are a lot of low-level problems with this patch.  Have spent about 90 minutes fixing problems so far, and think it will take another 90 before it's done.  Since I'm in the middle of it now, I'd rather finish it than start over from scratch with another stab (and I *do* want to see this get in -- it's a frequent complaint on c.l.py, and is sure *perceived* as a bug today).

Examples include:

+ There was no prototype in scope for stringobject.c's new Py_formatlong when it was referenced in unicodeobject.c.  This caused the compiler to assume it returned int instead of PyObject*, and that caused bad code generation.

+ "Py_formatlong" isn't a proper name for an internal function; if it was meant to be part of the public API, it needed docs.

+ The new formatlong in unicodeobject.c should have been declared static.

+ Ubiquitous use of 0 instead of NULL for null pointers; but this is C, not C++.

+ Non-Guido-like code formatting in small ways.

+ The logic for "skip" and "sign" is hosed in some cases.  For example,

>>> "%20x" % 2L**31
'            80000000'
>>> "%.20x" % 2L**31
'8000000000000000000'
>>>

This because the first "prec > len" loop copies skip+sign characters from the start of buf regardless of whether F_ALT is set (in the 2nd case above, F_ALT is not set, so it was incorrect to copy 2 characters before zero-filling).

+ The changed test_format failed for me; haven't tracked down why yet.

+ Not your doing:  stringobject.c's existing formatint uses a too-small buffer, under the bad assumption that a C long is at most 2**31.  (Speaking of which, why do people constantly try to get a temp buffer down to as few bytes as possible?!  The difference between 20 and (say) 256 in a leaf routine is meaningless.)

That last one was just to reassure you that I didn't spend the *whole* 90 minutes so far just picking on this patch <wink>.
-------------------------------------------------------

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

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