Skip to content

BUG: Fix comparator function signatures#20833

Merged
seberg merged 1 commit intonumpy:mainfrom
hoodmane:compare-npy-unused
Jan 16, 2022
Merged

BUG: Fix comparator function signatures#20833
seberg merged 1 commit intonumpy:mainfrom
hoodmane:compare-npy-unused

Conversation

@hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jan 16, 2022

The comparator functions are expected to have signature int f(void *, void*, PyArrayObject *).
Some of the comparators drop the third argument because they don't use them. Because the
comparators are unused, with most architectures this works fine. However, calling a function
pointer that has been cast to have a different number of arguments is undefined in the C
specification. With web assembly targets, it crashes at runtime.

This fixes the comparators defined in arraytypes.c.src to all take three arguments.

See also related work:
258ce25
23c05e6

The comparator functions are expected to have signature int f(void *, void*, PyArrayObject *).
Some of the comparators drop the third argument because they don't use them. Because the
comparators are unused, with most architectures this works fine. However, calling a function
pointer that has been cast to have a different number of arguments is undefined in the C
specification. With web assembly targets, it crashes at runtime.

This fixes the comparators defined in arraytypes.c.src to all take three arguments.

See also related work:
258ce25
23c05e6
@seberg seberg added the 09 - Backport-Candidate PRs tagged should be backported label Jan 16, 2022
@seberg
Copy link
Member

seberg commented Jan 16, 2022

LGTM, thanks for this cleanup @hoodmane. (Oddly, the typedef uses void * here, I think that is a bit of pattern even though what is passed is always an array... In any case, that doesn't really matter.)

@seberg seberg merged commit bac5117 into numpy:main Jan 16, 2022
@charris charris added this to the 1.22.2 release milestone Jan 16, 2022
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jan 17, 2022
@charris charris removed this from the 1.22.2 release milestone Jan 17, 2022
@hoodmane hoodmane deleted the compare-npy-unused branch January 27, 2022 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants