redundant "base" field in memoryview objects

[pitrou]

BPO	3560
Nosy	@gvanrossum, @loewis, @warsaw, @birkenfeld, @pitrou
Files	memapi.patch

^{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/pitrou'
closed_at = <Date 2008-08-19.18:23:47.456>
created_at = <Date 2008-08-15.09:37:06.316>
labels = ['interpreter-core', 'type-bug']
title = 'redundant "base" field in memoryview objects'
updated_at = <Date 2008-08-19.18:23:47.455>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2008-08-19.18:23:47.455>
actor = 'pitrou'
assignee = 'pitrou'
closed = True
closed_date = <Date 2008-08-19.18:23:47.456>
closer = 'pitrou'
components = ['Interpreter Core']
creation = <Date 2008-08-15.09:37:06.316>
creator = 'pitrou'
dependencies = []
files = ['11119']
hgrepos = []
issue_num = 3560
keywords = ['patch']
message_count = 12.0
messages = ['71164', '71173', '71175', '71179', '71180', '71181', '71182', '71418', '71449', '71450', '71451', '71458']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'loewis', 'barry', 'georg.brandl', 'teoliphant', 'pitrou']
pr_nums = []
priority = 'critical'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue3560'
versions = ['Python 3.0']

[pitrou]

PyMemoryViewObject has a "base" field which points to the originator of
the buffer. However, this field has become redundant now that the
Py_buffer struct has received an "obj" field which also points to the
originator of the buffer (this has been done as part of the fix to bpo-3139).

Not removing "base" would make for a confusing and error-prone API.
However, removing it is complicated by the fact that "base" is sometimes
abused to contain a tuple (for PyBUF_SHADOW type buffers, which are
neither mentioned in the PEP nor used anywhere in py3k).

[loewis]

Why is this a "critical" bug?

[birkenfeld]

Because it should be fixed before 3.0 final?

[loewis]

Because it should be fixed before 3.0 final?

And why should that be done? IMO, this can still
be fixed in 3.1, or not a fixed at all - I fail to
see the true bug (apart from the minor redundancy).

[pitrou]

Le vendredi 15 août 2008 à 19:10 +0000, Martin v. Löwis a écrit :

Martin v. Löwis <martin@v.loewis.de> added the comment:

> Because it should be fixed before 3.0 final?

And why should that be done? IMO, this can still
be fixed in 3.1, or not a fixed at all - I fail to
see the true bug (apart from the minor redundancy).

I've filed this as critical because it is a new API and, if we change
it, we'd better change it before 3.0 is released.

It is a minor redundancy but could easily lead to subtle problems
(crashes, memory leaks...) if for whatever reason, "base" and "view.obj"
start pointing to different objects.

Of course, if other people disagree, it can be bumped down to "normal".

[loewis]

I've filed this as critical because it is a new API and, if we change
it, we'd better change it before 3.0 is released.

I don't think it is API. The structure may be defined in a public
header, but it is not intended to be used directly. Instead,
only the functions around it should be used.

To make that clear, the structure could be moved into the C file,
and PyMemoryView should become a function (or also be moved into
the C file). This should be done before the next beta, indeed
(but I won't have the time to do it)

[pitrou]

Ok, here is a simple patch. It:

• moves the struct definition at the end of memoryobject.h with a
  comment that the definition should not be considered public
• adds two macros for accessing the underlying Py_buffer* and PyObject*,
  respectively
• removes the ill-named PyMemoryView() macro (PyMemoryView_GET_BUFFER()
  can be used instead)
• renames PyMemory_Check() to PyMemoryView_Check()
• renames PyMemoryView_FromMemory() to PyMemoryView_FromBuffer()

I didn't try to clean up the existing documentation comments, although I
find them difficult to follow. I also didn't change anything in the
semantics and implementation of the memoryview object.

Let me know what you think.

[warsaw]

I don't have time to review the patch, but based on the description, I'm
+1 on landing this before beta 3. I'd like to get the API right for 3.0
and it will be too late to land it after the release candidates start.

[gvanrossum]

Reviewers: GvR,

Message:
Looks good. Go ahead and submit, assuming you've run full tests.

Description:
http://bugs.python.org/issue3560

Please review this at http://codereview.appspot.com/3003

Affected files:
Include/memoryobject.h
Modules/_json.c
Objects/memoryobject.c
Objects/unicodeobject.c

[gvanrossum]

PS. The PEP probably needs an update. And the docs.

http://codereview.appspot.com/3003

[gvanrossum]

On 2008/08/19 17:18:45, GvR wrote:

PS. The PEP probably needs an update. And the docs.

And Misc/NEWS

http://codereview.appspot.com/3003

[pitrou]

Well, the PEP needs some love generally speaking; many things are un- or
under-specified (and I don't understand better than anyone else). As for
docs, there aren't any ("grep -ri memoryview Doc/" yields only the two
lines referencing the "memoryview" builtin): see bpo-2619.

The patch is committed in r65862.