Android target issues #755
Conversation
| #define __checkint_int64_mul(x, y, err) __check_int64_mul(x, y, err) | ||
| #define __checkint_int32_mul(x, y, err) __check_int32_mul(x, y, err) | ||
| #define __checkint_uint64_add(x, y, err) __check_uint64_add(x, y, err) | ||
| #define __checkint_uint32_add(x, y, err) __check_uint32_add(x, y, err) |
There was a problem hiding this comment.
For some reason when the __checkint_ shim macros are defined in CFSortFunctions.c, the prefix used is __check_ instead of __checkint_, and then only the used references are redefined as __checkint_. I added the 32 bits version of those references used.
I can rewrite this shim macros with the prefix __checkint_, as for what I see here they're not used anywhere else.
There was a problem hiding this comment.
this is just a stub until it can actually be done correctly to actually check... so looks fine to me
| @@ -894,7 +894,6 @@ void CFLog1(CFLogLevel lev, CFStringRef message) { | |||
| if (maxLength > sizeof(stack_buffer) / sizeof(stack_buffer[0])) { | |||
| buffer = calloc(sizeof(char), maxLength); | |||
| if (!buffer) { | |||
| - __android_log_print(ANDROID_LOG_ERROR, tag, "Unable to allocate %d bytes for log message - truncating.", (int)axLength); | |||
There was a problem hiding this comment.
@johnno1962 @phausler for what I understand the intention in this commit was to remove this line. Is this correct?
There was a problem hiding this comment.
tbh if the calloc fails bigger problems are going to happen down the road anyhow.
My opinion is just let it fail. It is not like this is a recoverable path or a container like Data
| @@ -894,7 +894,6 @@ void CFLog1(CFLogLevel lev, CFStringRef message) { | |||
| if (maxLength > sizeof(stack_buffer) / sizeof(stack_buffer[0])) { | |||
| buffer = calloc(sizeof(char), maxLength); | |||
| if (!buffer) { | |||
| - __android_log_print(ANDROID_LOG_ERROR, tag, "Unable to allocate %d bytes for log message - truncating.", (int)axLength); | |||
There was a problem hiding this comment.
tbh if the calloc fails bigger problems are going to happen down the road anyhow.
My opinion is just let it fail. It is not like this is a recoverable path or a container like Data
| @@ -27,8 +27,10 @@ | |||
| #if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_LINUX || DEPLOYMENT_TARGET_FREEBSD | |||
| #include <dirent.h> | |||
| #include <unistd.h> | |||
| #if !TARGET_OS_ANDROID | |||
There was a problem hiding this comment.
doesn't Android have it as #include <fcntl.h>?
There was a problem hiding this comment.
Correct. It was already in the same file, 10 lines before this.
| #define __checkint_int64_mul(x, y, err) __check_int64_mul(x, y, err) | ||
| #define __checkint_int32_mul(x, y, err) __check_int32_mul(x, y, err) | ||
| #define __checkint_uint64_add(x, y, err) __check_uint64_add(x, y, err) | ||
| #define __checkint_uint32_add(x, y, err) __check_uint32_add(x, y, err) |
There was a problem hiding this comment.
this is just a stub until it can actually be done correctly to actually check... so looks fine to me
|
@swift-ci please test |
This PR fixes a few issues when trying to compile for Android target.
Adding comments inline.