From: Maurus Cuelenaere <mcuelenaere_at_gmail.com>
Date: Wed, 16 Mar 2011 20:19:12 +0100

Op 16-03-11 15:33, mailer_at_svn.rockbox.org schreef:
> Date: 2011-03-16 15:33:55 +0100 (Wed, 16 Mar 2011)
> New Revision: 29601
>
> Log Message:
> Android: Partly revert r29569 and only call the new getJavaEnvironment() when needed.
>
> The environment is fine to share in general, just not across OS threads, so it's only needed
> for functions which are possibly called from multiple OS threads (only 1 currently).
>
[snip]
> Modified: trunk/apps/hosted/android/yesno.c
> ===================================================================
> --- trunk/apps/hosted/android/yesno.c 2011-03-16 11:38:49 UTC (rev 29600)
> +++ trunk/apps/hosted/android/yesno.c 2011-03-16 14:33:55 UTC (rev 29601)
> _at__at_ -28,8 +28,8 _at__at_
> #include "settings.h"
> #include "lang.h"
> #include "kernel.h"
> -#include "system.h"
>
> +extern JNIEnv *env_ptr;
> static jobject RockboxYesno_instance = NULL;
> static jmethodID yesno_func;
> static struct semaphore yesno_done;
> _at__at_ -44,7 +44,7 _at__at_
> semaphore_release(&yesno_done);
> }
>
> -static void yesno_init(JNIEnv *env_ptr)
> +static void yesno_init(void)
> {
> JNIEnv e = *env_ptr;
> static jmethodID yesno_is_usable;
> _at__at_ -74,7 +74,7 _at__at_
> sleep(HZ/10);
> }
>
> -static jstring build_message(JNIEnv *env_ptr, const struct text_message *message)
> +jstring build_message(const struct text_message *message)

I don't think this needed to be reverted?

> {
> char msg[1024] = "";
> JNIEnv e = *env_ptr;
> _at__at_ -98,12 +98,10 _at__at_
> {
> (void)yes_message;
> (void)no_message;
> - JNIEnv *env_ptr = getJavaEnvironment();
> -
> - yesno_init(env_ptr);
> -
> + yesno_init();
> +
> JNIEnv e = *env_ptr;
> - jstring message = build_message(env_ptr, main_message);
> + jstring message = build_message(main_message);
> jstring yes = (*env_ptr)->NewStringUTF(env_ptr, str(LANG_SET_BOOL_YES));
> jstring no = (*env_ptr)->NewStringUTF(env_ptr, str(LANG_SET_BOOL_NO));
>
>
> Modified: trunk/firmware/target/hosted/android/button-android.c
> ===================================================================
> --- trunk/firmware/target/hosted/android/button-android.c 2011-03-16 11:38:49 UTC (rev 29600)
> +++ trunk/firmware/target/hosted/android/button-android.c 2011-03-16 14:33:55 UTC (rev 29601)
> _at__at_ -29,6 +29,7 _at__at_
> #include "system.h"
> #include "touchscreen.h"
>
> +extern JNIEnv *env_ptr;

Same here, no users of env_ptr here?

> static int last_y, last_x;
> static int last_btns;
>
>
[snip]
> Modified: trunk/firmware/target/hosted/android/system-target.h
> ===================================================================
> --- trunk/firmware/target/hosted/android/system-target.h 2011-03-16 11:38:49 UTC (rev 29600)
> +++ trunk/firmware/target/hosted/android/system-target.h 2011-03-16 14:33:55 UTC (rev 29601)
> _at__at_ -21,8 +21,6 _at__at_
> #ifndef __SYSTEM_TARGET_H__
> #define __SYSTEM_TARGET_H__
>
> -#include <jni.h>
> -
> #define disable_irq()
> #define enable_irq()
> #define disable_irq_save() 0
> _at__at_ -32,20 +30,16 _at__at_
> void wait_for_interrupt(void);
> void interrupt(void);
>
> -/* A JNI environment is specific to its thread, so use the correct way to
> - * obtain it: share a pointer to the JavaVM structure and ask that the JNI
> - * environment attached to the current thread. */
> -static inline JNIEnv* getJavaEnvironment(void)
> -{
> - extern JavaVM *vm_ptr;
> - JNIEnv *env = NULL;
> + /* don't pull in jni.h for every user of this file, it should be only needed
> + * within the target tree (if at all)
> + * define this before #including system.h or system-target.h */
> +#ifdef _SYSTEM_WITH_JNI
> +#include <jni.h>
> +/*
> + * discover the JNIEnv for this the calling thread in case it's not known */
> +extern JNIEnv* getJavaEnvironment(void);
> +#endif /* _SYSTEM_WITH_JNI */
>
> - if (vm_ptr)
> - (*vm_ptr)->GetEnv(vm_ptr, (void**) &env, JNI_VERSION_1_2);
> -
> - return env;
> -}
> -


This should be solvable with the following trick:

diff --git a/firmware/target/hosted/android/pcm-android.c
b/firmware/target/hosted/android/pcm-android.c
index cc8bd9c..735956a 100644
--- a/firmware/target/hosted/android/pcm-android.c
+++ b/firmware/target/hosted/android/pcm-android.c
_at__at_ -21,7 +21,6 _at__at_

 #include <jni.h>
 #include <stdbool.h>
-#define _SYSTEM_WITH_JNI /* for getJavaEnvironment */
 #include <system.h>
 #include "debug.h"
 #include "pcm.h"
diff --git a/firmware/target/hosted/android/system-target.h
b/firmware/target/hosted/android/system-target.h
index 325c101..c547a58 100644
--- a/firmware/target/hosted/android/system-target.h
+++ b/firmware/target/hosted/android/system-target.h
_at__at_ -30,15 +30,11 _at__at_ void power_off(void);
 void wait_for_interrupt(void);
 void interrupt(void);

- /* don't pull in jni.h for every user of this file, it should be only needed
- * within the target tree (if at all)
- * define this before #including system.h or system-target.h */
-#ifdef _SYSTEM_WITH_JNI
-#include <jni.h>
-/*
- * discover the JNIEnv for this the calling thread in case it's not known */
+struct JNINativeInterface;
+typedef const struct JNINativeInterface* JNIEnv;
+
+/* discover the JNIEnv for this the calling thread in case it's not known */
 extern JNIEnv* getJavaEnvironment(void);
-#endif /* _SYSTEM_WITH_JNI */



> #endif /* __SYSTEM_TARGET_H__ */
>
> #define NEED_GENERIC_BYTESWAPS
>

Also, is not wanting to include jni.h for everyone the same reason for that ugly
"extern JNIEnv *env_ptr;" all over the place? If so, that can be solved the same
way as above.
Received on 2011-03-16