-
Notifications
You must be signed in to change notification settings - Fork 476
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
Cygwin compile broken #696
Comments
No opinion on the correctness of the patch, but this could probably use a comment. If they're the same file I would expect the headers to make sure double-includes work fine already tbh, perhaps this should be reported to the Cygwin folks as well? |
@Thulinma I agree with @tp-m , it should be fine to include both, are there no include guards in place? It should be noted that in the main branch integers.h has been removed, but maybe this problem has just moved to a different file now, would need to test. |
I figured it could use some changes, yes - I was in a hurry to "just get it working" for my own purposes 😅. (This is exactly why I only opened an issue with the information and didn't open a PR instead; to give you guys the room to solve this more cleanly than I did.) And yeah, I'd expect a double include like that to have some sort of guards around it... my guess it might not be getting caught because it's two different paths for the same file and the compiler isn't detecting they are the same. I'll take a look at this soon-ish and will report to Cygwin as well if it looks like that makes sense. 👍 |
Hey there! Building on latest Cygwin (using Meson at least - but I assume other systems are affected too) doesn't work because
machine/types.h
andsys/int_types.h
both exist on Cygwin, but are actually one and the same file. Both are detected and included by./crypto/include/integers.h
, causing an error due to redefinitions.I fixed this with the following patch for myself (since I'm using libsrtp as a meson subproject, so patching is trivial):
I opened this issue since I figured you might want to upstream that fix, and potentially also fix it in other build systems than just Meson (I assume the problem affects every build system, at least... but have not checked that 😇).
And yes, I realize my "fix" is rather hacky, but it works and it's a very small change 🤷. Feel free to do it some other way of course! This issue was more intended to make you aware of the problem rather than acting as a PR (hence no PR and all that).
The text was updated successfully, but these errors were encountered: