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

add inclusion protection #1049

Closed
wants to merge 1 commit into from
Closed

Conversation

Vollstrecker
Copy link
Contributor

The change you asked for.

@irwir
Copy link

irwir commented Feb 18, 2025

@madler
All looks green, still the <> form should never be in the library.

The reason is not implementation defined nature - it would be never known what header was included.
For example, this file was found in Catalina: /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/zconf.h
Angle bracket were redirecting to this file; needless to say, the preprocessed code was quite different.

@madler
Copy link
Owner

madler commented Feb 18, 2025

5a82f71

@madler madler closed this Feb 18, 2025
@madler
Copy link
Owner

madler commented Feb 18, 2025

Looks good. Thanks!

@madler
Copy link
Owner

madler commented Feb 19, 2025

@madler All looks green, still the <> form should never be in the library.

Yep. It didn't take long for an issue to pop up because of it. #1048
Resolved now.

@Vollstrecker
Copy link
Contributor Author

I'm not sure about that one as even when a zconf.h from /usr/include was used it was adjusted to the system. I'm on Debian with installed zlib-dev and it worked. The error looks like the ones we got when the zconf in the source-tree was used so my guess is that they are using add_subdirectory in a not intended way. But as the compiler command was missing and I'm not going to investigate their code, we'll never know.

Back to topic, the savest way to get this (if there are still problems) to change to "zconf_path/zconf.h" and throwing the right locating in there from commandline.

@irwir
Copy link

irwir commented Feb 20, 2025

I'm not sure about that one as even when a zconf.h from /usr/include was used it was adjusted to the system.

An unknown file named zconf.h might never belongs to zlib library.
Or be from a wrong version of the library.
Angle brackets should not exist even in conditional compilation.

@Vollstrecker
Copy link
Contributor Author

We'll nver know, as you just told us the file exists and looks strange. For me that looks like someone thinks it's a good idea to ship third-party stuff, like in Visual Studio where they bundle ninja (beside cmake).

If you ever come with a solution that works (as the defaults in zconf.h don't do that everywhere) and doesn't pollute the source-treem I'm open for everything, but I doubt that.

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.

3 participants