Skip to content
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

Match unified NDK headers #641

Closed
ndusart opened this issue Jul 3, 2017 · 3 comments
Closed

Match unified NDK headers #641

ndusart opened this issue Jul 3, 2017 · 3 comments

Comments

@ndusart
Copy link
Contributor

ndusart commented Jul 3, 2017

Before Android NDK r14b, each android api version has their own libc headers set which were not totally correct.

Since Android NDK r14b, Google unified the headers so that they apply to all api version (using #ifdef when needed to differentiate between mismatching versions). These new headers are directly taken from the bionic source which was not apparently the case before.

Unfortunately, this crate was based on these not unified headers and contain some incompatibilities with the new correct headers.
For now, we manage to update the NDK used to r15b by skipping the tests of the functions and structs that have changed. But ideally, we should update these functions to match the new headers for the next major version of libc.

So, for new symbols, please do not look into $NDK_PATH/platforms/android-xx/arch-arm/usr/include for reference but into $NDK_PATH/sysroot/usr/include as this path contains the new headers.

Here is a list of the changes written in C that we need to cover for the next major release:

  • int getpriotity(int, int) --> int getpriority(int, id_t) (id_t is uin32_t and is already defined as libc::id_t)

  • int setpriority(int, int, int) --> int setpriority(int, id_t, int)

  • int personality (unsigned long) --> personality(unsigned int)

  • stat struct is probably a bit harder to fix:
    These fields:

    long st_atime;
    unsigned long st_atime_nsec;
    long st_mtime;
    unsigned long st_mtime_nsec;
    long st_ctime;
    unsigned long st_ctime_nsec;

    became:

    struct timespec st_atim;
    struct timespec st_mtim;
    struct timespec st_ctim;

    where timespec is defined as:

    struct timespec {
      time_t tv_sec;
      long tv_nsec;
    };

    and some #define allow to still have fields like st_mtime_nsec:

    #define st_atime st_atim.tv_sec
    #define st_mtime st_mtim.tv_sec
    #define st_ctime st_ctim.tv_sec
    
    #define st_atime_nsec st_atim.tv_nsec
    #define st_mtime_nsec st_mtim.tv_nsec
    #define st_ctime_nsec st_ctim.tv_nsec
    
    #define st_atimensec st_atim.tv_nsec
    #define st_mtimensec st_mtim.tv_nsec
    #define st_ctimensec st_ctim.tv_nsec

    It means we could just keep the stat struct as is and just change the types of *_mtime and *_nsec to match the nex types and drop the new fields st_atim (without final e) and st_atimensec an alike (without the underscore before nsec) or find a way to present the field with different names.

  • int strerror_r(int, char*, size_t) --> char* strerror_r(int, char*, size_t)
    The old function is still present and accessible through the symbol strerror_r, the new one is accessible through _gnu_strerror_r. The solution is more probably simply just to ignore the new function and let strerror_r as is.

  • int madvise(const void*, size_t, int) --> int madvise(void*, size_t, int)

  • int msync(const void*, size_t, int) --> int msync(void*, size_t, int)

  • int mprotect(const void*, size_t, int) --> int mprotect(void*, size_t, int)

  • ssize_t recvfrom(int, void*, size_t, int, const struct sockaddr*, socklen_t*) --> ssize_t recvfrom(int, void*, size_t, int, struct sockaddr*, socklen_t*)

  • int bind(int, const struct sockaddr*, int) --> int bind(int, const struct sockaddr*, socklen_t) (only applies to android 64 bits, do not know why we bounded it correctly in 32 bits)

  • int writev(int, const struct iovec *, int) --> ssize_t writev(int, const struct iovec*, int) (same remark, only applies to android 64 bits)

  • int readv(int, const struct iovec *, int) --> ssize_t readv(int, const struct iovec*, int) (same remark)

  • int sendmsg(int, const struct msghdr*, int) --> ssize_t sendmsg(int, const struct msghdr*, int) (same remark)

  • int recvmsg(int, struct msghdr*, int) --> ssize_t recvmsg(int, struct msghdr*, int) (same remark)

@alexcrichton
Copy link
Member

Thanks for opening this @ndusart!

@roblabla
Copy link
Contributor

roblabla commented Jul 3, 2017

It means we could just keep the stat struct as is and just change the types of *_mtime and *_nsec to match the nex types and drop the new fields st_atim (without final e) and st_atimensec an alike (without the underscore before nsec) or find a way to present the field with different names.

I think we should just present the same physical layout (a struct with three timespec field) and have people use that. The C #defines are there to keep API compatible. Since we're going to do breaking changes anyway, I think we should just do the "right thing" and ignore those.

@gnzlbg
Copy link
Contributor

gnzlbg commented May 24, 2019

This is fixed (or should be) fixed on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants