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.

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

Don't define ruby_qsort when POSIX qsort_r is available. #6332

Merged
merged 1 commit into from
Sep 8, 2022
Merged

Don't define ruby_qsort when POSIX qsort_r is available. #6332

merged 1 commit into from
Sep 8, 2022

Conversation

delphij
Copy link
Contributor

@delphij delphij commented Sep 8, 2022

The current code would define ruby_qsort as a wrapper of qsort_s when it is available. When both qsort_s and POSIX (GNU) qsort_r are available, we should call qsort_r directly instead, and the qsort_s wrapper is redundant.

This fixes ruby build on FreeBSD with qsort_r interface modified to match POSIX one (more information at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=266227 and http://gohan05.nyi.freebsd.org/data/mainamd64PR266227-default/2022-09-06_09h13m56s/logs/errors/ruby32-3.2.0.p1_2,1.log ).

The current code would define ruby_qsort as a wrapper of qsort_s
when it is available. When both qsort_s and POSIX (GNU) qsort_r
are available, we should call qsort_r directly instead, and
the qsort_s wrapper is redundant.
@nobu
Copy link
Member

nobu commented Sep 8, 2022

I can't that log because the server is not found.

Anyway, since this seems to need backports to earlier versions, please file a ticket at https://bugs.ruby-lang.org/issues/new.

@delphij
Copy link
Contributor Author

delphij commented Sep 8, 2022

https://bugs.ruby-lang.org/issues/18997

The logs are only accessible over IPv6. I will paste part of it here in a follow up comment.

@delphij
Copy link
Contributor Author

delphij commented Sep 8, 2022

Log snippet:

compiling transcode.c
compiling transient_heap.c
compiling util.c
util.c:255:1: error: expected identifier or '('
ruby_qsort(void* base, const size_t nel, const size_t size, cmpfunc_t *cmp, void *d)
^
./include/ruby/util.h:124:21: note: expanded from macro 'ruby_qsort'
# define ruby_qsort qsort_r
                    ^
/usr/include/stdlib.h:344:5: note: expanded from macro 'qsort_r'
    __generic(arg, int (*)(void *, const void *, const void *),         \
    ^
/usr/include/sys/cdefs.h:320:2: note: expanded from macro '__generic'
        _Generic(expr, t: yes, default: no)
        ^
1 error generated.
*** Error code 1

@nobu nobu merged commit 7400628 into ruby:master Sep 8, 2022
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this pull request Sep 10, 2022
This change merges the following upstream pull request into the Ruby
interpreter:

	ruby/ruby#6332

Adding this patch is no-op right now because we are not using POSIX (GNU)
qsort_r(3) interface yet. It will fix build when the change is applied.

Reviewed by:	bapt (portmgr@), sunpoet (ruby@)
PR:		ports/266227
MFH:		2022Q3
Differential Revision: https://reviews.freebsd.org/D36492
freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this pull request Sep 10, 2022
This change merges the following upstream pull request into the Ruby
interpreter:

	ruby/ruby#6332

Adding this patch is no-op right now because we are not using POSIX (GNU)
qsort_r(3) interface yet. It will fix build when the change is applied.

Reviewed by:	bapt (portmgr@), sunpoet (ruby@)
PR:		ports/266227
MFH:		2022Q3
Differential Revision: https://reviews.freebsd.org/D36492

(cherry picked from commit b4b8c98)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants