ndk icon indicating copy to clipboard operation
ndk copied to clipboard

remove remaining #if guards on _constants_?

Open enh-google opened this issue 3 years ago • 2 comments
trafficstars

when i saw https://github.com/strace/strace/pull/204 i assumed it was someone using a really old NDK, but, no even today <fcntl.h> looks like this:

/** Flag for open(). */
#define O_ASYNC FASYNC
/** Flag for open(). */
#define O_RSYNC O_SYNC

#if __ANDROID_API__ >= 21
/** Flag for splice(). */
#define SPLICE_F_MOVE 1
/** Flag for splice(). */
#define SPLICE_F_NONBLOCK 2
/** Flag for splice(). */
#define SPLICE_F_MORE 4
/** Flag for splice(). */
#define SPLICE_F_GIFT 8
#endif

#if __ANDROID_API__ >= 26
/** Flag for sync_file_range(). */
#define SYNC_FILE_RANGE_WAIT_BEFORE 1
/** Flag for sync_file_range(). */
#define SYNC_FILE_RANGE_WRITE 2
/** Flag for sync_file_range(). */
#define SYNC_FILE_RANGE_WAIT_AFTER 4
#endif

i know we've been recommending against adding #if guards like that for constants, and i'm pretty sure i went through the "NDK APIs" removing stuff like this, but it looks like we never did the same for bionic itself?! or am i forgetting that we had a reason to keep it like this?

afaik, https://github.com/android/ndk/issues/394 was our motivating case: bad "configure script" type stuff gets confused when it has the constants but not the function (which is obviously less of a problem with "NDK API"). and, indeed, it looks like that's what's going on in <fcntl.h> --- the API levels around the #defines match the API levels for the corresponding functions. it just isn't obvious because there's a big gap between them. (maybe we should move them to be next to each other rather than having all the #defines at the top?)

anyway, i think i explained the current situation to myself while writing this, but i'll hit "submit" anyway for the record, even if there's probably nothing useful for us to do here...

enh-google avatar Jan 11 '22 21:01 enh-google

(even if we do nothing, i think this will solve itself over time as we remove guard relating to no-longer-supported API levels...)

enh-google avatar Jan 11 '22 21:01 enh-google

We probably need to at least change the guards so that the constants get exposed in the weak-symbol mode?

DanAlbert avatar Jan 11 '22 23:01 DanAlbert

https://r.android.com/3236543 fixes most of them. The stuff for posix_madvise is going to remain hidden because the boost code that caused us to hide them in the first place is still there: https://github.com/boostorg/interprocess/blob/9fc4ee7dd7277d86c691921007856aca99c3c1ac/include/boost/interprocess/mapped_region.hpp#L811-L814 (see https://github.com/android/ndk/issues/395)

DanAlbert avatar Aug 21 '24 22:08 DanAlbert