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

Since revision 10c, Android NDK includes proper struct ucontext_t #6617

Closed
vicuna opened this issue Oct 18, 2014 · 6 comments
Closed

Since revision 10c, Android NDK includes proper struct ucontext_t #6617

vicuna opened this issue Oct 18, 2014 · 6 comments
Assignees
Labels

Comments

@vicuna
Copy link

@vicuna vicuna commented Oct 18, 2014

Original bug ID: 6617
Reporter: @whitequark
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2016-12-07T10:47:27Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.1
Fixed in version: 4.03.0+dev / +beta1
Category: ~DO NOT USE (was: OCaml general)
Has duplicate: #6830
Related to: #5886

Bug description

In asmrun/signals_osdep.h we have this code:

#if defined(ANDROID)
// The Android NDK does not have sys/ucontext.h yet.
typedef struct ucontext {
uint32_t uc_flags;
struct ucontext *uc_link;
stack_t uc_stack;
struct sigcontext uc_mcontext;
// Other fields omitted...
} ucontext_t;
#else
#include <sys/ucontext.h>
#endif

There probably should be an s.h HAVE_ macro for this, as since r10c NDK includes ucontext_t, which is included from signal.h, and it clashes with this definition.

File attachments

@vicuna
Copy link
Author

@vicuna vicuna commented Oct 18, 2014

Comment author: @gasche

The fastest way for this to be fixed would probably be that you write a patch yourself. I don't know if other frequent contributors compile OCaml on Android -- the code you want to improve was contributed by Jérôme Vouillon in #5886.

Loading

@vicuna
Copy link
Author

@vicuna vicuna commented Oct 18, 2014

Comment author: @whitequark

The problem is that I don't do native builds on Android (only cross builds) and thus cannot verify the changes to ./configure script.

Loading

@vicuna
Copy link
Author

@vicuna vicuna commented Oct 20, 2014

Comment author: vouillon

I'm not sure anyone has ever done an Android native build...

A simple option is to just remove this Android specific code. I don't think we need to support older versions of the Android NDK.

Otherwise, one could refine the conditional:

#if defined(__ANDROID__) && !defined(_SYS_UCONTEXT_H_)

Loading

@vicuna
Copy link
Author

@vicuna vicuna commented Oct 20, 2014

Comment author: @whitequark

I'm in favor of removing the conditional altogether. 10c supports all Android versions anyway. Could someone commit this?

Loading

@vicuna
Copy link
Author

@vicuna vicuna commented Dec 13, 2014

Comment author: @gasche

Merged in trunk.

Loading

@vicuna
Copy link
Author

@vicuna vicuna commented Feb 15, 2015

Comment author: @gasche

This is also included in branch 4.02 as commit 15707.

Loading

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

Successfully merging a pull request may close this issue.

None yet
2 participants