Fix platform check for macOS

madler / zlib

A massively spiffy yet delicately unobtrusive compression library.

http://zlib.net/

Other

5.55k stars 2.42k forks source link

Fix platform check for macOS #895

Closed zixu-w closed 8 months ago

zixu-w commented 8 months ago

zutils.h contains configurations that are no longer exercised for the macOS target.

The target OS conditional macros are misused. Specifically TARGET_OS_MAC covers all Apple targets, including iOS, and it should not be checked with #if defined as they would always be defined (to either 1 or 0) on Apple platforms.
The assumption that macOS does not have fdopen, or defines fdopen as a macro is no longer valid. The null definition in zutils.h would conflict with the macOS SDK and cause a compilation failure.

This problem has not been noticed until a recent extension in clang (https://github.com/llvm/llvm-project/pull/74676) exposed the issue and broke zlib builds on Apple platforms.

madler commented 8 months ago

This problem has not been noticed until a recent extension in clang (llvm/llvm-project#74676) exposed the issue and broke zlib builds on Apple platforms.

zlib still compiles fine on the latest clang that Apple delivers, 15.0.0. What version of clang does it break on, and exactly how does it break?

zixu-w commented 8 months ago

This problem has not been noticed until a recent extension in clang (llvm/llvm-project#74676) exposed the issue and broke zlib builds on Apple platforms.

zlib still compiles fine on the latest clang that Apple delivers, 15.0.0. What version of clang does it break on, and exactly how does it break?

Thanks for taking a look Mark! The open source change will get into a future Apple clang at some point. The build failure can be reproduced currently by adding a line

#include <TargetConditionals.h>

before the #if defined(TARGET_OS_MAC) check.

madler commented 8 months ago

Thanks. Fixed.

Neustradamus commented 8 months ago

@madler: I think that some commits are missing since 2023-11-15, can you push it? Thanks in advance.

glandium commented 7 months ago

Note that the #if __APPLE block further down the same file redefines OS_CODE.

Neustradamus commented 7 months ago

It is good with the current devel branch?

zixu-w commented 7 months ago

It is good with the current devel branch?

I believe https://github.com/madler/zlib/commit/4bd9a71f3539b5ce47f0c67ab5e01f3196dc8ef9 resolved the issue so yeah it should be good.