Fix read_table compilation on windows
#455
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #455 +/- ##
=======================================
Coverage 78.08% 78.08%
=======================================
Files 55 55
Lines 11605 11605
Branches 11605 11605
=======================================
Hits 9062 9062
Misses 2043 2043
Partials 500 500 ☔ View full report in Codecov by Sentry. |
| @@ -79,6 +79,18 @@ EngineError* allocate_error(KernelError etype, const KernelStringSlice msg) | |||
| return (EngineError*)error; | |||
| } | |||
|
|
|||
| #ifdef WIN32 // windows doesn't have strndup | |||
| char *strndup(const char *s, size_t n) { | |||
| size_t len = strnlen(s, n); | |||
There was a problem hiding this comment.
does this blow up if s is NULL? should check if it's null and just return null if so?
There was a problem hiding this comment.
Yes, but this shouldn't be called on nulls to start, a lot of these older windows functions don't check for null (or they have null safe variants)
There was a problem hiding this comment.
Yep, but so does strndup (it'll segfault), so this is pretty much equivalent. (yay c)
| @@ -18,11 +18,11 @@ add_test(NAME read_and_print_basic_partitioned COMMAND ${TestRunner} ${DatPath}/ | |||
|
|
|||
| if(WIN32) | |||
| set(CMAKE_C_FLAGS_DEBUG "/MT") | |||
| target_link_libraries(read_table PUBLIC ws2_32 userenv bcrypt ntdll) | |||
| target_link_libraries(read_table PUBLIC ws2_32 userenv bcrypt ncrypt crypt32 secur32 ntdll RuntimeObject) | |||
There was a problem hiding this comment.
I'd like to also point out a lot of these are because the underlying rust libs aren't linking these statically when they build. I'm not entirely sure how to investigate or remediate this, but I am just mentioning I noticed this when figure out what to link.
What changes are proposed in this pull request?
Make
read_tableactually compile on windows.strndupimplementation for windows (thanks @hntd187)CMakeLists.txtwith correct things (thanks @hntd187 again)How was this change tested?
Built and run on windows (without
PRINT_DATA)