Skip to content

bpo-44114: Fix dictkeys_reversed and dictvalues_reversed function signatures#26062

Merged
serhiy-storchaka merged 2 commits into
python:mainfrom
joemarshall:patch-1
May 13, 2021
Merged

bpo-44114: Fix dictkeys_reversed and dictvalues_reversed function signatures#26062
serhiy-storchaka merged 2 commits into
python:mainfrom
joemarshall:patch-1

Conversation

@joemarshall

@joemarshall joemarshall commented May 12, 2021

Copy link
Copy Markdown
Contributor

Fix for bug number 44114

These are passed and called as PyCFunction, which looks like:

PyObject *PyCFunction(PyObject *self,
                      PyObject *args);

however they are defined here without the (ignored) args parameter.

This works fine in some C compilers, but fails in webassembly or anything else that has strict function pointer call type checking.

https://bugs.python.org/issue44114

These are passed and called as PyCFunction, which looks like:

```
PyObject *PyCFunction(PyObject *self,
                      PyObject *args);
```

however they are defined here without the (ignored) args parameter. 

This works fine in some C compilers, but fails in webassembly or anything else that has strict function pointer call type checking.
@the-knights-who-say-ni

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@joemarshall

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@JelleZijlstra

JelleZijlstra commented May 12, 2021

Copy link
Copy Markdown
Member

You should put "bpo-44114: " in your PR title. Also, you should add a news entry using blurb (item 8 in https://devguide.python.org/#quick-reference).

The code itself looks good to me (though I'm no expert on the C code). I'm going to attach labels to request backports to 3.10 and 3.9.

@joemarshall joemarshall changed the title Fix dictkeys_reversed and dictvalues_reversed function signatures bpo-44114: Fix dictkeys_reversed and dictvalues_reversed function signatures May 12, 2021
@joemarshall

Copy link
Copy Markdown
Contributor Author

I think I've done the right blurb things, and renamed things right now.

@markshannon

Copy link
Copy Markdown
Member

LGTM, but the CLA needs signing

@methane

methane commented May 12, 2021

Copy link
Copy Markdown
Member
    {"__reversed__",    (PyCFunction)(void(*)(void))dictvalues_reversed,    METH_NOARGS,

Can we remove this (void(*)(void)) cast?

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @joemarshall for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 13, 2021
…natures (pythonGH-26062)

These are passed and called as PyCFunction, however they are defined here without the (ignored) args parameter.

This works fine in some C compilers, but fails in webassembly or anything else that has strict function pointer call type checking.
(cherry picked from commit ab383eb)

Co-authored-by: Joe Marshall <joe.marshall@nottingham.ac.uk>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label May 13, 2021
@bedevere-bot

Copy link
Copy Markdown

GH-26092 is a backport of this pull request to the 3.10 branch.

@bedevere-bot

Copy link
Copy Markdown

GH-26093 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 13, 2021
…natures (pythonGH-26062)

These are passed and called as PyCFunction, however they are defined here without the (ignored) args parameter.

This works fine in some C compilers, but fails in webassembly or anything else that has strict function pointer call type checking.
(cherry picked from commit ab383eb)

Co-authored-by: Joe Marshall <joe.marshall@nottingham.ac.uk>
@serhiy-storchaka serhiy-storchaka added needs backport to 3.9 needs backport to 3.10 only security fixes type-bug An unexpected behavior, bug, or error labels May 13, 2021
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @joemarshall for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @joemarshall for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @joemarshall for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@bedevere-bot

Copy link
Copy Markdown

GH-26094 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request May 13, 2021
…natures (GH-26062)

These are passed and called as PyCFunction, however they are defined here without the (ignored) args parameter.

This works fine in some C compilers, but fails in webassembly or anything else that has strict function pointer call type checking.
(cherry picked from commit ab383eb)

Co-authored-by: Joe Marshall <joe.marshall@nottingham.ac.uk>
serhiy-storchaka pushed a commit that referenced this pull request May 16, 2021
…natures (GH-26062) (GH-26093)

These are passed and called as PyCFunction, however they are defined here without the (ignored) args parameter.

This works fine in some C compilers, but fails in webassembly or anything else that has strict function pointer call type checking.
(cherry picked from commit ab383eb)

Co-authored-by: Joe Marshall <joe.marshall@nottingham.ac.uk>

Co-authored-by: Joe Marshall <joe.marshall@nottingham.ac.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants