From: Thomas Martitz <thomas.martitz_at_student.htw-berlin.de>
Date: Wed, 23 Mar 2011 18:59:21 +0100

On 16.03.2011 20:19, Maurus Cuelenaere wrote:
> Op 16-03-11 15:33, mailer_at_svn.rockbox.org schreef:
> [snip]
> I don't think this needed to be reverted?
>
>> [snip]
> Same here, no users of env_ptr here?
>

I just reverted most the commit and add back what we walked about to
keep. I didn't pay attention what other parts of your commit could have
stayed.


>> 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 */
>

I guess it works, but is it better?


>
>> #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.

Right, but I didn't want to pull lots of unrelated changes into the
commit; especially since it partly reverting an earlier one.

Best regards.
Received on 2011-03-23