gh-Cyan4973/xxHash

Watch

Fork 0

Compilation error on macOS #455

New issue

Closed

opened 2020-09-26 02:16:52 -05:00 by rex4539 · 5 comments

rex4539 commented

2020-09-26 02:16:52 -05:00

(Migrated from github.com)

/Users/rex/xxHash on branch dev rex@MacBook-Pro-2018% make
cc -O3     -c -o xxhash.o xxhash.c
ar rcs libxxhash.a xxhash.o
cc -O3   -fPIC  xxhash.c -shared -install_name /usr/local/lib/libxxhash.0.dylib -compatibility_version 0 -current_version 0.8.0 -o libxxhash.0.8.0.dylib
ln -sf libxxhash.0.8.0.dylib libxxhash.0.dylib
ln -sf libxxhash.0.8.0.dylib libxxhash.dylib
cc -O3     -c -o xxhsum.o xxhsum.c
cc -O3     -c -o cli/xsum_os_specific.o cli/xsum_os_specific.c
cc -O3     -c -o cli/xsum_output.o cli/xsum_output.c
cc -O3     -c -o cli/xsum_sanity_check.o cli/xsum_sanity_check.c
In file included from cli/xsum_sanity_check.c:27:
cli/xsum_sanity_check.h:51:52: error: unknown type name 'size_t'
XSUM_API void XSUM_fillTestBuffer(XSUM_U8* buffer, size_t len);
                                                   ^
1 error generated.
make: *** [cli/xsum_sanity_check.o] Error 1

``` /Users/rex/xxHash on branch dev rex@MacBook-Pro-2018% make cc -O3 -c -o xxhash.o xxhash.c ar rcs libxxhash.a xxhash.o cc -O3 -fPIC xxhash.c -shared -install_name /usr/local/lib/libxxhash.0.dylib -compatibility_version 0 -current_version 0.8.0 -o libxxhash.0.8.0.dylib ln -sf libxxhash.0.8.0.dylib libxxhash.0.dylib ln -sf libxxhash.0.8.0.dylib libxxhash.dylib cc -O3 -c -o xxhsum.o xxhsum.c cc -O3 -c -o cli/xsum_os_specific.o cli/xsum_os_specific.c cc -O3 -c -o cli/xsum_output.o cli/xsum_output.c cc -O3 -c -o cli/xsum_sanity_check.o cli/xsum_sanity_check.c In file included from cli/xsum_sanity_check.c:27: cli/xsum_sanity_check.h:51:52: error: unknown type name 'size_t' XSUM_API void XSUM_fillTestBuffer(XSUM_U8* buffer, size_t len); ^ 1 error generated. make: *** [cli/xsum_sanity_check.o] Error 1 ```

👍 1

rex4539 commented

2020-09-26 02:18:13 -05:00

(Migrated from github.com)

@Cyan4973

easyaspi314 commented

2020-09-26 02:26:15 -05:00

(Migrated from github.com)

Oh wow, that one should have been obvious 🤦

Cyan4973 commented

2020-09-26 08:30:36 -05:00

(Migrated from github.com)

Thanks for the report @rex4539 !
Yes, I can confirm it.

The bigger lesson here is that our test system has missed that case.
The fix is likely obvious, but I don't get why the issue would trigger specifically on OS-X.
Maybe we need to add an OS-X target into our CI. TravisCI supports it.

Thanks for the report @rex4539 ! Yes, I can confirm it. The bigger lesson here is that our test system has missed that case. The fix is likely obvious, but I don't get why the issue would trigger specifically on OS-X. Maybe we need to add an OS-X target into our CI. TravisCI supports it.

easyaspi314 commented

2020-09-26 11:17:26 -05:00

(Migrated from github.com)

Not a bad idea.

This is why it didn't show up on Windows and Linux:

Is there any specific reason that we have this conditional? All of these platforms have unistd.h.

Not a bad idea. This is why it didn't show up on Windows and Linux: ![unistd bug](https://user-images.githubusercontent.com/6258309/94345206-3be12200-fff2-11ea-957a-e48884d8aa48.png) Is there any specific reason that we have this conditional? All of these platforms have unistd.h.

👍 1

Cyan4973 commented

2020-09-26 12:17:20 -05:00

(Migrated from github.com)

Well, it seems that the whole point of this paragraph is to #define XUM_PLATFORM_POSIX_VERSION,
and if #include <unistd.h> doesn't achieve that for macOS,
it feels logical to give it a different path.

If the objective of this paragraph is not clear, then maybe it should be clarified in a code comment.

The real issue here is that the side-effect that <unistd.h> also includes <stddef.h> when compiled on Linux should not be relied upon.

I guess this is only an issue now that the single source file xxhsum.c get split into multiple modules, and now the dependency chain must be setup correctly.

Well, it seems that the whole point of this paragraph is to `#define XUM_PLATFORM_POSIX_VERSION`, and if `#include <unistd.h>` doesn't achieve that for macOS, it feels logical to give it a different path. If the objective of this paragraph is not clear, then maybe it should be clarified in a code comment. The real issue here is that the side-effect that `<unistd.h>` also includes `<stddef.h>` when compiled on Linux should not be relied upon. I guess this is only an issue now that the single source file `xxhsum.c` get split into multiple modules, and now the dependency chain must be setup correctly.

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Cyan4973/xxHash#455

No description provided.

Rows
Columns