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

AVRO-3959: [C] Avoid deprecated OSX atomic ops #2797

Merged
merged 1 commit into from
Mar 12, 2024
Merged

Conversation

SahilKang
Copy link
Contributor

What is the purpose of the change

macOS 10.12 deprecated the OSAtomic* functions. This removes the following warnings:

  warning: 'OSAtomicIncrement32' is deprecated: first deprecated in macOS 10.12
  warning: 'OSAtomicDecrement32' is deprecated: first deprecated in macOS 10.12

Verifying this change

This change is already covered by existing tests, such as make test

Documentation

No new features are added

macOS 10.12 deprecated the OSAtomic* functions. This removes the following
warnings:

  warning: 'OSAtomicIncrement32' is deprecated: first deprecated in macOS 10.12
  warning: 'OSAtomicDecrement32' is deprecated: first deprecated in macOS 10.12

Signed-off-by: Sahil Kang <[email protected]>
Signed-off-by: Sahil Kang <[email protected]>
@github-actions github-actions bot added the C label Mar 10, 2024
@mkmkme
Copy link
Contributor

mkmkme commented Mar 11, 2024

Wouldn't it be better to introduce a new branch for newer version of macOS with usage of atomic_fetch_sub_explicit(memory_order_relaxed) and atomic_fetch_add_explicit(memory_order_relaxed), as suggested by the compiler warning?

@SahilKang
Copy link
Contributor Author

using the c11 atomic ops would require changing the volatile int input to atomic_int...and that'd require changing volatile int in several places

to maintain backwards compatibility with the pre-c11 branches, we'd have to use preprocessor wrappers at a higher level

something like:

#ifdef __IS_C_11__
#define ATOMIC_INT atomic_int
#define ATOMIC_INC(x) atomic_fetch_add_explicit(x)
...
#else
#define ATOMIC_INT volatile int
#define ATOMIC_INC(x) __sync_add_and_fetch(x, 1)
...
#endif

fwiw, I think that'd be a nice followup change to standardize things a bit more

@mkmkme
Copy link
Contributor

mkmkme commented Mar 12, 2024

I see. Thanks for the clarification!

@martin-g martin-g merged commit 01ba73c into apache:main Mar 12, 2024
5 checks passed
@martin-g
Copy link
Member

Thank you, @SahilKang and @mkmkme !

@SahilKang SahilKang deleted the c-osx branch April 20, 2024 05:51
RanbirK pushed a commit to RanbirK/avro that referenced this pull request May 13, 2024
macOS 10.12 deprecated the OSAtomic* functions. This removes the following
warnings:

  warning: 'OSAtomicIncrement32' is deprecated: first deprecated in macOS 10.12
  warning: 'OSAtomicDecrement32' is deprecated: first deprecated in macOS 10.12

Signed-off-by: Sahil Kang <[email protected]>
Signed-off-by: Sahil Kang <[email protected]>
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