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

Fixed 'undef' warning with gcc and clang #2131

Merged
merged 2 commits into from Sep 20, 2021
Merged

Fixed 'undef' warning with gcc and clang #2131

merged 2 commits into from Sep 20, 2021

Conversation

lelegard
Copy link
Contributor

@lelegard lelegard commented Sep 20, 2021

Fix for this warning:

srt/platform_sys.h:48:5: error: "__APPLE__" is not defined, evaluates to 0 [-Werror=undef]
   48 | #if __APPLE__
      |     ^~~~~~~~~

With clang, it is possible to use a pragma diagnostics before #include <srt/srt.h> to mute the warning.

With gcc, however, the situation is critical. Although the same pragma exists, it does nothing due to a known gcc bug since 2012, never fixed. Apparently, the bug appears too early in the lex analysis phase and #pragma are not yet parsed. It seems a bit difficult to solve and nobody wanted to fix it in 9 years.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431

The problem is important in applications with a strict "no warning policy" (-Wall -Wextra -Werror). If a warning is impossible to mute, the compilation fails.

Note: please note that #if __APPLE__ was replaced with #if defined(__APPLE__) && __APPLE__. Please keep it like this and do not use #ifdef __APPLE__. Because of the gcc bug, in order to compile the application, I have found no other solution than this:

#if defined(__GNUC__) && !defined(__APPLE__)
#define __APPLE__ 0
#define ZERO__APPLE__ 1
#endif

#include <srt/srt.h>

#if defined(ZERO__APPLE__)
#undef __APPLE__
#undef ZERO__APPLE__
#endif

So, if you use #ifdef __APPLE__, it won't work as expected.

@maxsharabayko maxsharabayko added this to the v1.4.4 milestone Sep 20, 2021
@maxsharabayko maxsharabayko added [core] Area: Changes in SRT library core Type: Bug Indicates an unexpected problem or unintended behavior labels Sep 20, 2021
Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

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

Hi @lelegard. Thank you for the fix!
I would suggest to add a dedicated comment, otherwise sooner or later it will be changed back to #if __APPLE__.

Also CMake defines DARWIN here. It is always defined with either 0 or 1. Probably makes sense to use it instead. But does not sound like something to check in v1.4.4 release. 🙂

srtcore/platform_sys.h Show resolved Hide resolved
@maxsharabayko maxsharabayko merged commit add4058 into Haivision:master Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants