fix: don't declare bswap32/bswap64 function if macros are defined#419
fix: don't declare bswap32/bswap64 function if macros are defined#419botantony wants to merge 1 commit intomaxmind:mainfrom
bswap32/bswap64 function if macros are defined#419Conversation
Summary of ChangesHello @botantony, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a specific compilation issue encountered on macOS systems. By introducing conditional compilation directives, it ensures that custom Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a compilation failure on macOS where bswap32 and bswap64 can be defined as macros. The change correctly wraps the inline function definitions of bswap32 and bswap64 with #ifndef preprocessor guards to avoid redefinition. This is a standard and effective way to handle such portability issues. The changes are correct and I have no further suggestions.
botantony
force-pushed
the
macos-compilation-failure
branch
from
45fd484 to
f8cb24f
Compare
Declaring these functions causes compilation failure on macOS because `sys/endian.h` header already defined macros with the same name Signed-off-by: botantony <antonsm21@gmail.com>
botantony
force-pushed
the
macos-compilation-failure
branch
from
f8cb24f to
0138e84
Compare
|
Thanks for reporting this! We do have macOS builds in our CI, but I think the Rather than doing the |
Our CI was using -std=c99 for all builds, which causes Darwin headers to hide BSD extension symbols. This masked the bswap32/bswap64 macro collision reported in #419, since real-world users (e.g. Homebrew) build without -std=c99 and see the conflict. Remove -std=c99 from the default CFLAGS so the main CI matrix matches what downstream consumers build with. Add a dedicated strict-C99 matrix entry (ubuntu/gcc) to preserve language compliance checking. Also change CMAKE_C_EXTENSIONS from OFF to ON to match default CMake behavior and what users get when building with cmake. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Our CI was using -std=c99 for all builds, which causes Darwin headers to hide BSD extension symbols. This masked the bswap32/bswap64 macro collision reported in #419, since real-world users (e.g. Homebrew) build without -std=c99 and see the conflict. Remove -std=c99 from the default CFLAGS so the main CI matrix matches what downstream consumers build with. Add a dedicated strict-C99 matrix entry (ubuntu/gcc) to preserve language compliance checking. Also change CMAKE_C_EXTENSIONS from OFF to ON to match default CMake behavior and what users get when building with cmake. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Our CI was using -std=c99 for all builds, which causes Darwin headers to hide BSD extension symbols. This masked the bswap32/bswap64 macro collision reported in #419, since real-world users (e.g. Homebrew) build without -std=c99 and see the conflict. Remove -std=c99 from the default CFLAGS so the main CI matrix matches what downstream consumers build with. Add a dedicated strict-C99 matrix entry (ubuntu/gcc) to preserve language compliance checking. Also change CMAKE_C_EXTENSIONS from OFF to ON to match default CMake behavior and what users get when building with cmake. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Our CI was using -std=c99 for all builds, which causes Darwin headers to hide BSD extension symbols. This masked the bswap32/bswap64 macro collision reported in #419, since real-world users (e.g. Homebrew) build without -std=c99 and see the conflict. Remove -std=c99 from the default CFLAGS so the main CI matrix matches what downstream consumers build with. Add a dedicated strict-C99 matrix entry (ubuntu/gcc) to preserve language compliance checking. Also change CMAKE_C_EXTENSIONS from OFF to ON to match default CMake behavior and what users get when building with cmake. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Our CI was using -std=c99 for all builds, which causes Darwin headers to hide BSD extension symbols. This masked the bswap32/bswap64 macro collision reported in #419, since real-world users (e.g. Homebrew) build without -std=c99 and see the conflict. Remove -std=c99 from the default CFLAGS so the main CI matrix matches what downstream consumers build with. Add -std=c99 as a separate matrix entry to preserve language compliance checking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Our CI was using -std=c99 for all builds, which causes Darwin headers to hide BSD extension symbols. This masked the bswap32/bswap64 macro collision reported in #419, since real-world users (e.g. Homebrew) build without -std=c99 and see the conflict. Remove -std=c99 from the default CFLAGS and add it as a separate matrix entry to preserve language compliance checking. Also add macos-26 to the OS matrix since the bswap collision appears to be specific to macOS 26 (Tahoe), which is not covered by macos-latest (currently macOS 15). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I was able to reproduce this by switching our |
On macOS 26, sys/endian.h defines bswap32 and bswap64 as macros. Since maxminddb.h includes sys/endian.h when available, our static inline function declarations collide with those macros, causing a compilation failure. Rename to mmdb_bswap32/mmdb_bswap64 to avoid the namespace collision entirely. These are internal static functions so the rename is safe. Fixes #419 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On macOS 26, sys/endian.h defines bswap32 and bswap64 as macros. Since maxminddb.h includes sys/endian.h when available, our static inline function declarations collide with those macros, causing a compilation failure. Rename to mmdb_bswap32/mmdb_bswap64 to avoid the namespace collision entirely. These are internal static functions so the rename is safe. Fixes #419 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On macOS 26, sys/endian.h defines bswap32 and bswap64 as macros. Since maxminddb.h includes sys/endian.h when available, our static inline function declarations collide with those macros, causing a compilation failure. Rename to mmdb_bswap32/mmdb_bswap64 to avoid the namespace collision entirely. These are internal static functions so the rename is safe. Fixes #419 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Declaring these functions causes compilation failure on macOS because
sys/endian.hheader already defined macros with the same nameRelated to Homebrew/homebrew-core#269271