Support for masks in getargs.c

[gvanrossum]

BPO	595026
Nosy	@gvanrossum, @theller, @jackjansen
Files	test_getargs2.py: Test for integer PyArg_ParseTuple format codes. getargs.patch: Patch to several core files getargs.c.diff: Missing patch for getargs.c getargs-2.patch: Complete (hopefully) with decrefs fixed. test_getargs2.py not included.

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/theller'
closed_at = <Date 2003-04-17.19:03:24.000>
created_at = <Date 2002-08-14.12:26:55.000>
labels = ['interpreter-core']
title = 'Support for masks in getargs.c'
updated_at = <Date 2003-04-17.19:03:24.000>
user = 'https://github.com/gvanrossum'

bugs.python.org fields:

activity = <Date 2003-04-17.19:03:24.000>
actor = 'gvanrossum'
assignee = 'theller'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2002-08-14.12:26:55.000>
creator = 'gvanrossum'
dependencies = []
files = ['576', '577', '578', '579']
hgrepos = []
issue_num = 595026
keywords = []
message_count = 20.0
messages = ['11944', '11945', '11946', '11947', '11948', '11949', '11950', '11951', '11952', '11953', '11954', '11955', '11956', '11957', '11958', '11959', '11960', '11961', '11962', '11963']
nosy_count = 3.0
nosy_names = ['gvanrossum', 'theller', 'jackjansen']
pr_nums = []
priority = 'high'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue595026'
versions = ['Python 2.3']

[gvanrossum]

We need this implemented:

How about the following counterproposal. This also
changes some of
the other format codes to be a little more regular.

Code C type Range check

b unsigned char 0..UCHAR_MAX
B unsigned char none **
h unsigned short 0..USHRT_MAX
H unsigned short none **
i int
INT_MIN..INT_MAX
I * unsigned int 0..UINT_MAX
l long
LONG_MIN..LONG_MAX
k * unsigned long none
L long long LLONG_MIN..LLONG_MAX
K * unsigned long long none

Notes:

• New format codes.

** Changed from previous "range-and-a-half" to
"none"; the
range-and-a-half checking wasn't particularly useful.

Plus a C API or two, e.g. PyInt_AsLongMask() ->
unsigned long and PyInt_AsLongLongMask() -> unsigned
long long (if that exists).

[theller]

Logged In: YES
user_id=11105

If nobody else comes up, I can do this. I also had similar
checks for the struct module, but Tim killed it because of
backward compatibility.

[gvanrossum]

Logged In: YES
user_id=6380

Thomas: that would be great! (As long it isn't killed by bw
compat. :-)

[theller]

Logged In: YES
user_id=11105

But it would probably take a few days.

What about these changes to the format codes, they are now
more in sync with the struct module (I hope the formatting
is kept):

Code C type Range check

b unsigned char 0..UCHAR_MAX
B unsigned char none **
h unsigned short 0..USHRT_MAX
H unsigned short none **
i int INT_MIN..INT_MAX
I * unsigned int 0..UINT_MAX
l long LONG_MIN..LONG_MAX
L * unsigned long none
q long long LLONG_MIN..LLONG_MAX
Q * unsigned long long none

[gvanrossum]

Logged In: YES
user_id=6380

A few days is fine (this doesn't need to make it into 2.3a2).

But the proposal here is not backwards compatible, is it?
Those codes already mean something different now.

I think we'll need to invent new format codes, or a new
modifier.

[theller]

Logged In: YES
user_id=11105

Ok, I'll use your codes.

Here is my suggestion for the C api functions:

int PyInt_AsLongMask(PyObject *v, unsigned long *pval);
int PyInt_AsLongLongMask(PyObject *v, unsigned LONG_LONG *pval);

return -1 and set exception on error, return 0 otherwise and
store the result in pval. This saves the PyErr_Occurred() in
case the value is -1.

[theller]

Logged In: YES
user_id=11105

Currently the h code means signed short
(SHRT_MIN..SHRT_MAX), do you really want to change that to
unsigned short (0..USHRT_MAX)?

[jackjansen]

Logged In: YES
user_id=45365

Guido,
I would be happy with the no-rangecheck unsigned long formatcode. One request, though: if this is implemented can it be done ASAP after 2.3a2, so there's still some time to shake out the bugs before 2.3b1 comes out? Especially the hand-written code will have to be examined to see where l needs to be turned into k.

[theller]

Logged In: YES
user_id=11105

I've implemented the 'k' getargs code, and
PyInt_AsUnsignedLongMask and PyLong_AsUnsignedLongMask
functions. Is there any facility which would help me to
test this new code?

[gvanrossum]

Logged In: YES
user_id=6380

I suggest writing on python-dev or python-list.

[theller]

Logged In: YES
user_id=11105

Uploading patch whcih implements the 'k' format code. Any
comments? Is this what is needed?
I know, docs are missing, and 'K' is missing.

[theller]

Logged In: YES
user_id=11105

Patch is ready for review (although NEWS and docs are
missing, I will add them later if the patch is accepted).

getargs.patch contains a context diff for several files,
including Modules/_testcapimodule.c.
test_getargs2.py should go into Lib/test, and tests the changes.
It also documents the new behaviour.

In the complete test-suite, test_array is crashing now as a
consequence.

[theller]

Logged In: YES
user_id=11105

I forgot to say: kpatch.diff is obsolete, please ignore.
Hm, I'll better delete it.

[gvanrossum]

Logged In: YES
user_id=6380

Review comments:

• Where's the patch to getargs.c???

• There's a missing DECREF(io) in
  PyInt_AsUnsignedLong[Long]Mask() in the block with the
  "nb_int should return int object" error return. (This is
  also missing from the template you used, PyInt_AsLong()!)

• I get a failure in _testcapi:

[guido@odiug linux]$ ./python ../Lib/test/test_capi.py
internal test_L_code
internal test_config
internal test_dict_iteration
internal test_k_code
Traceback (most recent call last):
  File "../Lib/test/test_capi.py", line 16, in ?
    raise test_support.TestFailed, sys.exc_info()[1]
test.test_support.TestFailed: test_k_code: k code returned
wrong value for long -0xFFF..000042
[guido@odiug linux]$

[theller]

Logged In: YES
user_id=11105

Oops, sorry: getargs.c.diff

[theller]

Logged In: YES
user_id=11105

But wait: I'll fix the missing decrefs, and create a new,
complete patch. Takes a couple of minutes, though.

[theller]

Logged In: YES
user_id=11105

getargs-2.patch, hopefully complete, but test_getargs2.py
not included.

[gvanrossum]

Logged In: YES
user_id=6380

Good enough; check it in, with docs and NEWS.

But please fix these:

• the 'h' opcode comments and error messages still refer to
  it as "signed short" while it is now clearly an *unsigned*
  short. (Hm... maybe 'h' should remain a signed short after
  all, for backwards compatibility?)

• there's still an unused definition of AddSym in testcapi.

[theller]

Logged In: YES
user_id=11105

Checked in with the changes you requested.
I left the 'h' opcode unchanged, if needed this can be fixed
later.

Will do the docs on tuesday, NEWS today, if time permits.

[gvanrossum]

Logged In: YES
user_id=6380

Thanks!