Skip to content

Add PTHREAD_*_MUTEX_INITIALIZER_NP for glibc #960

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

Merged
merged 2 commits into from
Apr 10, 2018
Merged

Add PTHREAD_*_MUTEX_INITIALIZER_NP for glibc #960

merged 2 commits into from
Apr 10, 2018

Conversation

glandium
Copy link
Contributor

@glandium glandium commented Apr 4, 2018

pthread_mutex_t varies across architectures, in several ways:

  • endianness alters the ordering of bytes, since the contents of the
    struct are larger than 8-bit.
  • its length varies.
  • the location of the mutex kind (PTHREAD_MUTEX_RECURSIVE,
    PTHREAD_MUTEX_ERRORCHECK or PTHREAD_MUTEX_ADAPTIVE_NP) varies
    between 32-bit and 64-bit: On 32-bit architectures, it is preceded by
    three int/unsigned int, while on 64-bit architectures, it is preceded
    by four of them.

These initializers are only available from <pthread.h> when _GNU_SOURCE
is defined.

@glandium
Copy link
Contributor Author

glandium commented Apr 4, 2018

rust 1.0 didn't like it:

src/unix/notbsd/linux/other/b64/not_x32.rs:1:5: 1:41 error: unresolved import
src/unix/notbsd/linux/other/b64/not_x32.rs:1 use unix::notbsd::linux::pthread_mutex_t;
                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@glandium
Copy link
Contributor Author

glandium commented Apr 4, 2018

Working around the rust 1.0 issue is an abuse of style.rs, but in the end, I think it's much better this way.

@alexcrichton
Copy link
Member

Could the cfg_if! be avoided and instead these definitions copied to the respective modules?

@glandium
Copy link
Contributor Author

glandium commented Apr 4, 2018

Could the cfg_if! be avoided and instead these definitions copied to the respective modules?

See the original patch and how it blantantly failed to build on rust 1.0. I also find it's a mess to have this spread across multiple files.

@alexcrichton
Copy link
Member

Hm I no longer see the logs, but presumably there exist some construction that works, right?

@glandium
Copy link
Contributor Author

glandium commented Apr 5, 2018

Apparently, rust 1.0 didn't support use of ancestor module items. So it's not possible to use unix::notbsd::linux::pthread_mutex_t from unix::notbsd::linux::something. The only work around I can think of is to move and duplicate the definition of pthread_mutex_t in all the submodules. I think there's already way too much duplication in the libc crate, I don't think this is a great idea.

@alexcrichton
Copy link
Member

Yes that's why everything uses ::foo, and the duplication is intended today and is verified via a script so works out alright.

@glandium
Copy link
Contributor Author

glandium commented Apr 5, 2018

I tried ::foo already, and that didn't work:

src/unix/notbsd/linux/other/b64/not_x32.rs:1:5: 1:43 error: unresolved import
src/unix/notbsd/linux/other/b64/not_x32.rs:1 use ::unix::notbsd::linux::pthread_mutex_t;
                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@alexcrichton
Copy link
Member

Er yes, I mean in the code itself, aka fn foo(a: ::c_int) instead of fn foo(a: c_int)

@glandium
Copy link
Contributor Author

glandium commented Apr 5, 2018

::pthread_mutex_t doesn't work either.

@glandium
Copy link
Contributor Author

glandium commented Apr 5, 2018

Ah ok, I could make it work.

@glandium
Copy link
Contributor Author

ping @alexcrichton

@alexcrichton
Copy link
Member

@glandium hm this still has all the definitions in one location? Could the definitions not be pushed down into their respective modules and avoid the need for cfg_if!?

@glandium
Copy link
Contributor Author

Argh, it looks like I rebased and pushed the wrong branch after #962.

`pthread_mutex_t` varies across architectures, in several ways:
- endianness alters the ordering of bytes, since the contents of the
  struct are larger than 8-bit.
- its length varies.
- the location of the mutex kind (`PTHREAD_MUTEX_RECURSIVE`,
  `PTHREAD_MUTEX_ERRORCHECK` or `PTHREAD_MUTEX_ADAPTIVE_NP`) varies
  between 32-bit and 64-bit: On 32-bit architectures, it is preceded by
  three int/unsigned int, while on 64-bit architectures, it is preceded
  by four of them.

These initializers are only available from <pthread.h> when _GNU_SOURCE
is defined.

Relax the cfg_if check in ci/style.rs to allow #[cfg(target_endian)]
tests.
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 10, 2018

📌 Commit d901327 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 10, 2018

⌛ Testing commit d901327 with merge 846ecb0...

bors added a commit that referenced this pull request Apr 10, 2018
Add PTHREAD_*_MUTEX_INITIALIZER_NP for glibc

`pthread_mutex_t` varies across architectures, in several ways:
- endianness alters the ordering of bytes, since the contents of the
  struct are larger than 8-bit.
- its length varies.
- the location of the mutex kind (`PTHREAD_MUTEX_RECURSIVE`,
  `PTHREAD_MUTEX_ERRORCHECK` or `PTHREAD_MUTEX_ADAPTIVE_NP`) varies
  between 32-bit and 64-bit: On 32-bit architectures, it is preceded by
  three int/unsigned int, while on 64-bit architectures, it is preceded
  by four of them.

These initializers are only available from <pthread.h> when _GNU_SOURCE
is defined.
@bors
Copy link
Contributor

bors commented Apr 10, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 846ecb0 to master...

@bors bors merged commit d901327 into rust-lang:master Apr 10, 2018
glandium added a commit to glandium/ctest that referenced this pull request Apr 11, 2018
glandium added a commit to glandium/ctest that referenced this pull request Apr 11, 2018
@glandium glandium deleted the mutex-init branch April 11, 2018 04:47
tgross35 pushed a commit to tgross35/ctest2 that referenced this pull request Apr 2, 2025
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Apr 2, 2025
tgross35 pushed a commit to tgross35/ctest2 that referenced this pull request Apr 2, 2025
sanstzu pushed a commit to sanstzu/rust-libc that referenced this pull request May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants