Skip to content

Conversation

devnexen
Copy link
Contributor

No description provided.

@rust-highfive
Copy link

Some changes occurred in OpenBSD module

cc @semarie

@rust-highfive
Copy link

r? @JohnTitor

(rust-highfive has picked a reviewer for you, use r? to override)


// pthread.h

pub struct __c_anonymous_pthread_spinlock {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pthread_spinlock struct is internal and isn't in public header on purpose. the visible part is only a pointer, nothing else.

it is the same kind of type as pthread_t which is defined as pub type pthread_t = ::uintptr_t in Rust.

@@ -1530,6 +1539,12 @@ extern "C" {
pub fn pthread_main_np() -> ::c_int;
pub fn pthread_set_name_np(tid: ::pthread_t, name: *const ::c_char);
pub fn pthread_stackseg_np(thread: ::pthread_t, sinfo: *mut ::stack_t) -> ::c_int;
pub fn pthread_spin_init(lock: *mut pthread_spinlock_t, pshared: ::c_int) -> ::c_int;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I don't mess myself, pthread_spin_ functions could be commun with at least NetBSD.

@@ -193,6 +195,10 @@ s! {
pub struct cpumask_t {
ary: [u64; 4],
}

pub struct __c_anonymous_pthread_spinlock_s {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it isn't exactly my area but I think if the OS define a struct as opaque, it should be kept opaque.

pthread_spinlock_t is opaque in DragonflyBSD too: sys/sys/_pthreadtypes.h

@@ -381,6 +381,7 @@ fn test_openbsd(target: &str) {
"limits.h",
"link.h",
"locale.h",
"machine/spinlock.h",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this definition shouldn't be need now that you don't use _atomic_lock_t type.

the tests passes on OpenBSD without it.

@semarie
Copy link
Contributor

semarie commented Jun 17, 2021

it mess to me you reintroduced __c_anonymous_pthread_spinlock on OpenBSD while removing it from DragonflyBSD...

@devnexen
Copy link
Contributor Author

ah yes wrong rebasing

@semarie
Copy link
Contributor

semarie commented Jun 17, 2021

rebasing still a bit wrong :

  • if ty.starts_with("__c_anonymous_") { isn't need anymore in test_openbsd() function
  • pthread_spin_ functions could be merged with NetBSD inside src/unix/bsd/netbsdlike/mod.rs

@devnexen
Copy link
Contributor Author

true. Merci beaucoup for your review ;-)

Copy link
Contributor

@semarie semarie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine with me. thanks

@JohnTitor
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Jun 18, 2021

📌 Commit 7250a68 has been approved by JohnTitor

@bors
Copy link
Contributor

bors commented Jun 18, 2021

⌛ Testing commit 7250a68 with merge 570fb1b...

@bors
Copy link
Contributor

bors commented Jun 18, 2021

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: JohnTitor
Pushing 570fb1b to master...

@bors bors merged commit 570fb1b into rust-lang:master Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants