Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ctypes arrays have incorrect buffer information (PEP-3118) #54953

Closed
pv mannequin opened this issue Dec 20, 2010 · 11 comments
Closed

ctypes arrays have incorrect buffer information (PEP-3118) #54953

pv mannequin opened this issue Dec 20, 2010 · 11 comments
Labels
topic-ctypes type-bug An unexpected behavior, bug, or error

Comments

@pv
Copy link
Mannequin

pv mannequin commented Dec 20, 2010

BPO 10744
Nosy @theller, @ronaldoussoren, @amauryfa, @mdickinson, @abalkin, @pitrou, @vstinner, @pv, @skrah, @meadori, @mattip, @eryksun
Files
  • 001-ctypes-fix-pep-3118-format-strings-for-arrays.patch: Suggested fix (on py3k branch)
  • hg-python2.7-ctypes-fix-pep3118-format-strings-for-arrays-updated.patch: python 2.7 patch
  • hg-default-ctypes-fix-pep3118-format-strings-for-arrays-update.patch: patch against HEAD
  • hg-default-ctypes-fix-pep3118-format-strings-for-arrays-update.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 = None
closed_at = <Date 2014-05-17.22:07:43.632>
created_at = <Date 2010-12-20.22:06:47.639>
labels = ['ctypes', 'type-bug']
title = 'ctypes arrays have incorrect buffer information (PEP-3118)'
updated_at = <Date 2014-05-17.22:07:43.630>
user = 'https://github.com/pv'

    bugs.python.org fields:

    activity = <Date 2014-05-17.22:07:43.630>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2014-05-17.22:07:43.632>
closer = 'python-dev'
components = ['ctypes']
creation = <Date 2010-12-20.22:06:47.639>
creator = 'pv'
dependencies = []
files = ['20123', '35217', '35218', '35224']
hgrepos = []
issue_num = 10744
keywords = ['patch']
message_count = 11.0
messages = ['124406', '142081', '142192', '191562', '191575', '191583', '218292', '218293', '218298', '218322', '218716']
nosy_count = 14.0
nosy_names = ['theller', 'ronaldoussoren', 'amaury.forgeotdarc', 'mark.dickinson', 'belopolsky', 'pitrou', 'vstinner', 'pv', 'skrah', 'meador.inge', 'python-dev', 'Tilka', 'mattip', 'eryksun']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue10744'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']
    @pv
    Copy link
    Mannequin Author

    pv mannequin commented Dec 20, 2010

    Ctypes arrays have invalid buffer interface information (on Python 3.1.2):

    >>> import ctypes
>>> x = (ctypes.c_double*2)()
>>> y = memoryview(x)
>>> y.shape
(2,)
>>> y.format
'(2)<d'

    This implies that the array contains 2 items, each consisting of 2 floats, which is incorrect -- the shape information is duplicated.

    A suggested fix is attached.

    (From http://projects.scipy.org/numpy/ticket/1699)

    @pv pv mannequin assigned theller Dec 20, 2010
    @pv pv mannequin added topic-ctypes type-bug An unexpected behavior, bug, or error labels Dec 20, 2010
    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Aug 14, 2011

    Thanks for the patch. I agree with the interpretation of the format string.
    One thing is unclear though: Using this interpretation the multi-dimensional
    array notation in format strings only seems useful for pointers to arrays.

    The PEP isn't so clear on that, would you agree?

    I'm not done reviewing the patch, just a couple of nitpicks:

    • We need a function declaration of _ctypes_alloc_format_string_with_shape()
      in ctypes.h.

    • prefix_len = 32*(ndim+1) + 3: This is surely sufficient, but (ndim+1)
      is not obvious to me. I think we need (20 + 1) * ndim + 3.

    • I'd use "%zd" for Py_ssize_t (I know that in other parts of the
      code "%ld" is used, too).

    @skrah skrah mannequin unassigned theller Aug 14, 2011
    @pv
    Copy link
    Mannequin Author

    pv mannequin commented Aug 16, 2011

    The array notation is useful for arrays inside structs, such as "T{(4)i(2,3)f}".

    @vstinner
    Copy link
    Member

    @skrah: What is the status of this issue?

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jun 21, 2013

    Basically someone has to review the patch and commit it. I'm not really
    using ctypes, so for me a decent review would take a while.

    @ronaldoussoren
    Copy link
    Contributor

    The correct format string for Py_ssize_t with sprintf is
    "%"PY_FORMAT_SIZE_T"d" (defined and explained in pyport.h).

    @mattip
    Copy link
    Contributor

    mattip commented May 11, 2014

    Here are updated patches for 2.7 and HEAD.

    I've updated the patches for both python 2 and python 3 and addressed some of the issues here:

    1. Added _ctypes_alloc_format_string_with_shape() to ctypes.h
    2. Changed the buffer size calculation to 32 *ndim + 3 (for the question asked before, the 32 size is just because this was the size used before the patch....)
    3. Changed the sprintf to use the macro: sprintf(buf, "%"PY_FORMAT_SIZE_T"d,", shape[k]);

    I really hope this patch makes it to 2.7.7 and python 3 versions, as currently the situation is broken and code that once worked with older numpy versions does not work today (and we have legacy code that stopped working).

    @mattip
    Copy link
    Contributor

    mattip commented May 11, 2014

    Here is the patch for HEAD

    @eryksun
    Copy link
    Contributor

    eryksun commented May 11, 2014

    The shape for the StructWithArrays test should be () in 3.x; it's None in 2.x.

    @mattip
    Copy link
    Contributor

    mattip commented May 12, 2014

    Updated patch for default for empty shape, thanks eryksun

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 17, 2014

    New changeset 44dac2e7fcb8 by Benjamin Peterson in branch '2.7':
    support PEP-3118 format strings for ctypes objects with nontrivial shapes (closes bpo-10744)
    http://hg.python.org/cpython/rev/44dac2e7fcb8

    New changeset 22938bf57161 by Benjamin Peterson in branch '3.4':
    support PEP-3118 format strings for ctypes objects with nontrivial shapes (closes bpo-10744)
    http://hg.python.org/cpython/rev/22938bf57161

    New changeset ea9193340d6c by Benjamin Peterson in branch 'default':
    merge 3.4 (bpo-10744)
    http://hg.python.org/cpython/rev/ea9193340d6c

    @python-dev python-dev mannequin closed this as completed May 17, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    topic-ctypes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @vstinner @ronaldoussoren @mattip @eryksun @theller