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

Minor refactoring to the atomics implementation #92

Closed
wants to merge 3 commits into from

Conversation

KonstantinRitt
Copy link
Contributor

No description provided.

For most cases, the GCC/Intel atomics (__sync functions) are fine,
but there are some systems for which libgcc is incorrectly built
and thus configure reports HAVE_INTEL_ATOMIC_PRIMITIVES
even though the Intel atomics aren't really implemented (i.e. QNX 6.5).

As of now, it is possible to implement the atomics in config.h
and define HB_NO_ATOMIC_IMPL to force HB use *that* implementation.
Just to bring it in par with the hb_atomic-s.
s/atomic_int/atomic_int_impl/ and s/atomic_ptr/atomic_ptr_impl/
to bring it in par with hb_mutex_impl_t, then re-introduce
hb_atomic_int_t as a wrapper around hb_atomic_int_impl_t.

In hb_reference_count_t, make it clear the non-atomic get and set
are intentional due to nature of the cases they are used in
(comparison to -1 and the debug output/tracing).
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 58.6% when pulling bb60d4b on KonstantinRitt:atomics into aee6850 on behdad:master.

@behdad
Copy link
Member

behdad commented Apr 8, 2015

Thanks. I implemented the first two commits in a different way.

behdad added a commit that referenced this pull request Apr 8, 2015
@behdad
Copy link
Member

behdad commented Apr 8, 2015

Thanks. I merged this after minor changes. Please check.

@KonstantinRitt
Copy link
Contributor Author

LGTM so far. Thanks.

@KonstantinRitt
Copy link
Contributor Author

missing HB_ATOMIC_INT_IMPL_INIT definition in
#elif !defined(HB_NO_MT) && (defined(_WIN32) || defined(CYGWIN))
section

@behdad
Copy link
Member

behdad commented Apr 8, 2015

Fixed. Thanks.

@KonstantinRitt KonstantinRitt deleted the atomics branch November 6, 2015 23:52
gpgreen pushed a commit to gpgreen/harfbuzz that referenced this pull request Jan 10, 2024
Derive Clone rather than impl it.

This fixes warnings under clippy, but also happily makes it match
how current bindgen generates code, which will reduce some diffs
that will be coming up.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-harfbuzz/92)
<!-- Reviewable:end -->
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.

None yet

3 participants