Skip to content

Commit 6da17d6

Browse files
twalltwall
committed
Merge pull request #498 from java-native-access/496-direct-typemapper-with-wstring
[496] Handle TypeMapper-returned String/WString in direct mode
2 parents 4a2d3be + 744d924 commit 6da17d6

16 files changed

+399
-125
lines changed

‎CHANGES.md

+1
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ Bug Fixes
7575
* [#420](https://github.com/twall/jna/pull/420): Fix structure leaving always one element in ThreadLocal set - [@sjappig](https://github.com/sjappig).
7676
* [#467](https://github.com/twall/jna/issues/467): Fix TypeMapper usage with direct-mapped libraries converting primitives to Java objects (specifically enums) - [@twall](https://github.com/twall).
7777
* [#475](https://github.com/twall/jna/issues/475): Avoid modifying native memory in `Structure.equals()/hashCode()`- [@twall](https://github.com/twall).
78+
* [#496](https://github.com/twall/jna/issues/496): Properly handle direct mapping with type mappers which return String/WString - [@twall](https://github.com/twall).
7879

7980
Release 4.1
8081
===========

‎native/callback.c

+21-4
Original file line numberDiff line numberDiff line change
@@ -378,15 +378,18 @@ invoke_callback(JNIEnv* env, callback *cb, ffi_cif* cif, void *resp, void **cbar
378378
args[2] = &cb->methodID;
379379
memcpy(&args[3], cbargs, cif->nargs * sizeof(void *));
380380

381+
// Note that there is no support for CVT_TYPE_MAPPER here
381382
if (cb->conversion_flags) {
382383
for (i=0;i < cif->nargs;i++) {
383384
switch(cb->conversion_flags[i]) {
384385
case CVT_INTEGER_TYPE:
385386
case CVT_POINTER_TYPE:
386387
case CVT_NATIVE_MAPPED:
388+
case CVT_NATIVE_MAPPED_STRING:
389+
case CVT_NATIVE_MAPPED_WSTRING:
387390
// Make sure we have space enough for the new argument
388391
args[i+3] = alloca(sizeof(void *));
389-
*((void **)args[i+3]) = fromNative(env, cb->arg_classes[i], cif->arg_types[i], cbargs[i], JNI_FALSE);
392+
*((void **)args[i+3]) = fromNative(env, cb->arg_classes[i], cif->arg_types[i], cbargs[i], JNI_FALSE, cb->encoding);
390393
break;
391394
case CVT_POINTER:
392395
*((void **)args[i+3]) = newJavaPointer(env, *(void **)cbargs[i]);
@@ -411,6 +414,11 @@ invoke_callback(JNIEnv* env, callback *cb, ffi_cif* cif, void *resp, void **cbar
411414
args[i+3] = alloca(sizeof(double));
412415
*((double *)args[i+3]) = *(float*)cbargs[i];
413416
break;
417+
case CVT_DEFAULT:
418+
break;
419+
default:
420+
fprintf(stderr, "JNA: Unhandled arg conversion type %d\n", cb->conversion_flags[i]);
421+
break;
414422
}
415423
}
416424
}
@@ -447,7 +455,13 @@ invoke_callback(JNIEnv* env, callback *cb, ffi_cif* cif, void *resp, void **cbar
447455
*(void **)resp = getPointerTypeAddress(env, *(void **)resp);
448456
break;
449457
case CVT_NATIVE_MAPPED:
450-
toNative(env, *(void **)resp, oldresp, cb->cif.rtype->size, JNI_TRUE);
458+
toNative(env, *(void **)resp, oldresp, cb->cif.rtype->size, JNI_TRUE, cb->encoding);
459+
break;
460+
case CVT_NATIVE_MAPPED_STRING:
461+
case CVT_NATIVE_MAPPED_WSTRING:
462+
// TODO: getNativeString rather than allocated memory
463+
fprintf(stderr, "JNA: Likely memory leak here\n");
464+
toNative(env, *(void **)resp, oldresp, cb->cif.rtype->size, JNI_TRUE, cb->encoding);
451465
break;
452466
case CVT_POINTER:
453467
*(void **)resp = getNativeAddress(env, *(void **)resp);
@@ -469,7 +483,10 @@ invoke_callback(JNIEnv* env, callback *cb, ffi_cif* cif, void *resp, void **cbar
469483
case CVT_CALLBACK:
470484
*(void **)resp = getCallbackAddress(env, *(void **)resp);
471485
break;
486+
case CVT_DEFAULT:
487+
break;
472488
default:
489+
fprintf(stderr, "JNA: Unhandled result conversion: %d\n", cb->rflag);
473490
break;
474491
}
475492
if (cb->conversion_flags) {
@@ -487,7 +504,7 @@ invoke_callback(JNIEnv* env, callback *cb, ffi_cif* cif, void *resp, void **cbar
487504
unsigned int i;
488505

489506
for (i=0;i < cif->nargs;i++) {
490-
jobject arg = new_object(env, cb->arg_jtypes[i], cbargs[i], JNI_FALSE);
507+
jobject arg = new_object(env, cb->arg_jtypes[i], cbargs[i], JNI_FALSE, cb->encoding);
491508
(*env)->SetObjectArrayElement(env, params, i, arg);
492509
}
493510
result = (*env)->CallObjectMethod(env, self, cb->methodID, params);
@@ -501,7 +518,7 @@ invoke_callback(JNIEnv* env, callback *cb, ffi_cif* cif, void *resp, void **cbar
501518
memset(resp, 0, cif->rtype->size);
502519
}
503520
else {
504-
extract_value(env, result, resp, cif->rtype->size, JNI_TRUE);
521+
extract_value(env, result, resp, cif->rtype->size, JNI_TRUE, cb->encoding);
505522
}
506523
}
507524
}

‎native/dispatch.c

+64-24
Original file line numberDiff line numberDiff line change
@@ -962,8 +962,9 @@ get_conversion_flag(JNIEnv* env, jclass cls) {
962962
int
963963
get_java_type_from_ffi_type(ffi_type* type) {
964964
switch(type->type) {
965-
// FIXME aliases 'C' on *nix; this will cause problems if anyone
966-
// ever installs a type mapper for char/Character (not a common arg type)
965+
// NOTE 'Z' aliases 'C' on *nix and platforms where sizeof(wchar_t) is 4;
966+
// this will cause problems if anyone ever installs a type mapper for
967+
// char/Character (not a common arg type)
967968
case FFI_TYPE_UINT32: return 'Z';
968969
case FFI_TYPE_SINT8: return 'B';
969970
case FFI_TYPE_SINT16: return 'S';
@@ -1113,11 +1114,11 @@ getFFITypeTypeMapped(JNIEnv* env, jobject converter) {
11131114
}
11141115

11151116
void
1116-
toNative(JNIEnv* env, jobject obj, void* valuep, size_t size, jboolean promote) {
1117+
toNative(JNIEnv* env, jobject obj, void* valuep, size_t size, jboolean promote, const char* encoding) {
11171118
if (obj != NULL) {
11181119
jobject arg = (*env)->CallObjectMethod(env, obj, MID_NativeMapped_toNative);
11191120
if (!(*env)->ExceptionCheck(env)) {
1120-
extract_value(env, arg, valuep, size, promote);
1121+
extract_value(env, arg, valuep, size, promote, encoding);
11211122
}
11221123
}
11231124
else {
@@ -1126,11 +1127,11 @@ toNative(JNIEnv* env, jobject obj, void* valuep, size_t size, jboolean promote)
11261127
}
11271128

11281129
static void
1129-
toNativeTypeMapped(JNIEnv* env, jobject obj, void* valuep, size_t size, jobject to_native) {
1130+
toNativeTypeMapped(JNIEnv* env, jobject obj, void* valuep, size_t size, jobject to_native, const char* encoding) {
11301131
if (obj != NULL) {
11311132
jobject arg = (*env)->CallStaticObjectMethod(env, classNative, MID_Native_toNativeTypeMapped, to_native, obj);
11321133
if (!(*env)->ExceptionCheck(env)) {
1133-
extract_value(env, arg, valuep, size, JNI_FALSE);
1134+
extract_value(env, arg, valuep, size, JNI_FALSE, encoding);
11341135
}
11351136
}
11361137
else {
@@ -1141,11 +1142,11 @@ toNativeTypeMapped(JNIEnv* env, jobject obj, void* valuep, size_t size, jobject
11411142
static void
11421143
fromNativeTypeMapped(JNIEnv* env, jobject from_native,
11431144
void* native_return_value,
1144-
ffi_type* native_return_type,
1145+
int jtype, int size,
11451146
jclass java_return_class,
1146-
void* result_storage) {
1147-
int jtype = get_java_type_from_ffi_type(native_return_type);
1148-
jobject value = new_object(env, (char)jtype, native_return_value, JNI_TRUE);
1147+
void* result_storage,
1148+
const char* encoding) {
1149+
jobject value = new_object(env, (char)jtype, native_return_value, JNI_TRUE, encoding);
11491150
if (!(*env)->ExceptionCheck(env)) {
11501151
jobject obj = (*env)->CallStaticObjectMethod(env, classNative,
11511152
MID_Native_fromNativeTypeMapped,
@@ -1160,7 +1161,7 @@ fromNativeTypeMapped(JNIEnv* env, jobject from_native,
11601161
|| (*env)->IsSameObject(env, java_return_class, classPrimitiveLong)
11611162
|| (*env)->IsSameObject(env, java_return_class, classPrimitiveFloat)
11621163
|| (*env)->IsSameObject(env, java_return_class, classPrimitiveDouble)) {
1163-
extract_value(env, obj, result_storage, native_return_type->size, JNI_TRUE);
1164+
extract_value(env, obj, result_storage, size, JNI_TRUE, encoding);
11641165
}
11651166
else {
11661167
*(jobject*)result_storage = obj;
@@ -1170,9 +1171,9 @@ fromNativeTypeMapped(JNIEnv* env, jobject from_native,
11701171
}
11711172

11721173
jobject
1173-
fromNative(JNIEnv* env, jclass javaClass, ffi_type* type, void* resp, jboolean promote) {
1174+
fromNative(JNIEnv* env, jclass javaClass, ffi_type* type, void* resp, jboolean promote, const char* encoding) {
11741175
int jtype = get_java_type_from_ffi_type(type);
1175-
jobject value = new_object(env, (char)jtype, resp, promote);
1176+
jobject value = new_object(env, (char)jtype, resp, promote, encoding);
11761177
if (!(*env)->ExceptionCheck(env)) {
11771178
return (*env)->CallStaticObjectMethod(env, classNative,
11781179
MID_Native_fromNative,
@@ -1455,9 +1456,12 @@ JNA_init(JNIEnv* env) {
14551456
return NULL;
14561457
}
14571458

1458-
/** Copy value from the given Java object into the given storage buffer. */
1459+
/** Copy value from the given Java object into the given storage buffer.
1460+
* If the value is being extracted from a String or WString, you are
1461+
* responsible for freeing the allocated memory.
1462+
*/
14591463
void
1460-
extract_value(JNIEnv* env, jobject value, void* buffer, size_t size, jboolean promote) {
1464+
extract_value(JNIEnv* env, jobject value, void* buffer, size_t size, jboolean promote, const char* encoding) {
14611465
if (value == NULL) {
14621466
*(void **)buffer = NULL;
14631467
}
@@ -1525,19 +1529,32 @@ extract_value(JNIEnv* env, jobject value, void* buffer, size_t size, jboolean pr
15251529
else if ((*env)->IsInstanceOf(env, value, classPointer)) {
15261530
*(void **)buffer = getNativeAddress(env, value);
15271531
}
1532+
else if ((*env)->IsInstanceOf(env, value, classString)) {
1533+
*(void **)buffer = newCStringEncoding(env, (jstring)value, encoding);
1534+
}
1535+
else if ((*env)->IsInstanceOf(env, value, classWString)) {
1536+
jstring s = (*env)->CallObjectMethod(env, value, MID_Object_toString);
1537+
*(void **)buffer = newWideCString(env, s);
1538+
}
15281539
else {
1529-
fprintf(stderr, "JNA: extract_value: unrecognized return type, size %d\n", (int)size);
1540+
char msg[MSG_SIZE];
1541+
snprintf(msg, sizeof(msg), "Can't convert type to native, native size %d\n", (int)size);
1542+
fprintf(stderr, "JNA: extract_value: %s", msg);
15301543
memset(buffer, 0, size);
1531-
throwByName(env, EError, "Unrecognized return type");
1544+
throwByName(env, EError, msg);
15321545
}
15331546
}
15341547

15351548
/** Construct a new Java object from a native value. */
15361549
jobject
1537-
new_object(JNIEnv* env, char jtype, void* valuep, jboolean promote) {
1550+
new_object(JNIEnv* env, char jtype, void* valuep, jboolean promote, const char* encoding) {
15381551
switch(jtype) {
15391552
case 's':
15401553
return newJavaPointer(env, valuep);
1554+
case 'c':
1555+
return newJavaString(env, *(void**)valuep, encoding);
1556+
case 'w':
1557+
return newJavaString(env, *(void **)valuep, NULL);
15411558
case '*':
15421559
return newJavaPointer(env, *(void**)valuep);
15431560
case 'J':
@@ -1613,6 +1630,8 @@ get_ffi_type(JNIEnv* env, jclass cls, char jtype) {
16131630
return NULL;
16141631
}
16151632
case '*':
1633+
case 'c':
1634+
case 'w':
16161635
default:
16171636
return &ffi_type_pointer;
16181637
}
@@ -1706,22 +1725,27 @@ dispatch_direct(ffi_cif* cif, void* volatile resp, void** argp, void *cdata) {
17061725
*(void **)args[i] = getPointerTypeAddress(env, *(void **)args[i]);
17071726
break;
17081727
case CVT_TYPE_MAPPER:
1728+
case CVT_TYPE_MAPPER_STRING:
1729+
case CVT_TYPE_MAPPER_WSTRING:
17091730
{
17101731
void* valuep = args[i];
17111732
int jtype = get_java_type_from_ffi_type(data->closure_cif.arg_types[i+2]);
17121733
jobject obj = jtype == '*'
17131734
? *(void **)valuep
1714-
: new_object(env, (char)jtype, valuep, JNI_FALSE);
1735+
: new_object(env, (char)jtype, valuep, JNI_FALSE, data->encoding);
17151736
if (cif->arg_types[i+2]->size < data->cif.arg_types[i]->size) {
17161737
args[i] = alloca(data->cif.arg_types[i]->size);
17171738
}
17181739
toNativeTypeMapped(env, obj, args[i],
17191740
data->cif.arg_types[i]->size,
1720-
data->to_native[i]);
1741+
data->to_native[i],
1742+
data->encoding);
17211743
}
17221744
break;
17231745
case CVT_NATIVE_MAPPED:
1724-
toNative(env, *(void **)args[i], args[i], data->cif.arg_types[i]->size, JNI_FALSE);
1746+
case CVT_NATIVE_MAPPED_STRING:
1747+
case CVT_NATIVE_MAPPED_WSTRING:
1748+
toNative(env, *(void **)args[i], args[i], data->cif.arg_types[i]->size, JNI_FALSE, data->encoding);
17251749
break;
17261750
case CVT_POINTER:
17271751
*(void **)args[i] = getNativeAddress(env, *(void **)args[i]);
@@ -1825,12 +1849,22 @@ dispatch_direct(ffi_cif* cif, void* volatile resp, void** argp, void *cdata) {
18251849

18261850
switch(data->rflag) {
18271851
case CVT_TYPE_MAPPER:
1828-
fromNativeTypeMapped(env, data->from_native, resp, data->cif.rtype, data->closure_rclass, oldresp);
1852+
case CVT_TYPE_MAPPER_STRING:
1853+
case CVT_TYPE_MAPPER_WSTRING:
1854+
{
1855+
int jtype = (data->rflag == CVT_TYPE_MAPPER_STRING
1856+
? 'c' : (data->rflag == CVT_TYPE_MAPPER_WSTRING
1857+
? 'w' : get_java_type_from_ffi_type(data->cif.rtype)));
1858+
fromNativeTypeMapped(env, data->from_native, resp, jtype, data->cif.rtype->size,
1859+
data->closure_rclass, oldresp, data->encoding);
1860+
}
18291861
break;
18301862
case CVT_INTEGER_TYPE:
18311863
case CVT_POINTER_TYPE:
18321864
case CVT_NATIVE_MAPPED:
1833-
*(void **)oldresp = fromNative(env, data->closure_rclass, data->cif.rtype, resp, JNI_TRUE);
1865+
case CVT_NATIVE_MAPPED_STRING:
1866+
case CVT_NATIVE_MAPPED_WSTRING:
1867+
*(void **)oldresp = fromNative(env, data->closure_rclass, data->cif.rtype, resp, JNI_TRUE, data->encoding);
18341868
break;
18351869
case CVT_POINTER:
18361870
*(void **)resp = newJavaPointer(env, *(void **)resp);
@@ -1865,6 +1899,10 @@ dispatch_direct(ffi_cif* cif, void* volatile resp, void** argp, void *cdata) {
18651899
break;
18661900
case CVT_STRING:
18671901
case CVT_WSTRING:
1902+
case CVT_TYPE_MAPPER_STRING:
1903+
case CVT_TYPE_MAPPER_WSTRING:
1904+
case CVT_NATIVE_MAPPED_STRING:
1905+
case CVT_NATIVE_MAPPED_WSTRING:
18681906
// Free allocated native strings
18691907
free(*(void **)args[i]);
18701908
break;
@@ -3281,7 +3319,9 @@ Java_com_sun_jna_Native_registerMethod(JNIEnv *env, jclass UNUSED(ncls),
32813319
if (cvts) {
32823320
data->flags[i] = cvts[i];
32833321
// Type mappers only apply to non-primitive arguments
3284-
if (cvts[i] == CVT_TYPE_MAPPER) {
3322+
if (cvts[i] == CVT_TYPE_MAPPER
3323+
|| cvts[i] == CVT_TYPE_MAPPER_STRING
3324+
|| cvts[i] == CVT_TYPE_MAPPER_WSTRING) {
32853325
if (!data->to_native) {
32863326
data->to_native = calloc(argc, sizeof(jweak));
32873327
}

‎native/dispatch.h

+8-4
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,14 @@ enum {
9393
CVT_CALLBACK = com_sun_jna_Native_CVT_CALLBACK,
9494
CVT_FLOAT = com_sun_jna_Native_CVT_FLOAT,
9595
CVT_NATIVE_MAPPED = com_sun_jna_Native_CVT_NATIVE_MAPPED,
96+
CVT_NATIVE_MAPPED_STRING = com_sun_jna_Native_CVT_NATIVE_MAPPED_STRING,
97+
CVT_NATIVE_MAPPED_WSTRING = com_sun_jna_Native_CVT_NATIVE_MAPPED_WSTRING,
9698
CVT_WSTRING = com_sun_jna_Native_CVT_WSTRING,
9799
CVT_INTEGER_TYPE = com_sun_jna_Native_CVT_INTEGER_TYPE,
98100
CVT_POINTER_TYPE = com_sun_jna_Native_CVT_POINTER_TYPE,
99101
CVT_TYPE_MAPPER = com_sun_jna_Native_CVT_TYPE_MAPPER,
102+
CVT_TYPE_MAPPER_STRING = com_sun_jna_Native_CVT_TYPE_MAPPER_STRING,
103+
CVT_TYPE_MAPPER_WSTRING = com_sun_jna_Native_CVT_TYPE_MAPPER_WSTRING,
100104
};
101105

102106
/* callback behavior flags */
@@ -191,8 +195,8 @@ extern callback* create_callback(JNIEnv*, jobject, jobject,
191195
jobjectArray, jclass,
192196
callconv_t, jint, jstring);
193197
extern void free_callback(JNIEnv*, callback*);
194-
extern void extract_value(JNIEnv*, jobject, void*, size_t, jboolean);
195-
extern jobject new_object(JNIEnv*, char, void*, jboolean);
198+
extern void extract_value(JNIEnv*, jobject, void*, size_t, jboolean, const char*);
199+
extern jobject new_object(JNIEnv*, char, void*, jboolean, const char*);
196200
extern jboolean is_protected();
197201
extern int get_conversion_flag(JNIEnv*, jclass);
198202
extern jboolean ffi_error(JNIEnv*,const char*,ffi_status);
@@ -211,8 +215,8 @@ extern jlong getIntegerTypeValue(JNIEnv*, jobject);
211215
extern void* getPointerTypeAddress(JNIEnv*, jobject);
212216
extern void writeStructure(JNIEnv*, jobject);
213217
extern jclass getNativeType(JNIEnv*, jclass);
214-
extern void toNative(JNIEnv*, jobject, void*, size_t, jboolean);
215-
extern jclass fromNative(JNIEnv*, jclass, ffi_type*, void*, jboolean);
218+
extern void toNative(JNIEnv*, jobject, void*, size_t, jboolean, const char*);
219+
extern jclass fromNative(JNIEnv*, jclass, ffi_type*, void*, jboolean, const char*);
216220

217221
typedef struct _AttachOptions {
218222
int daemon;

‎native/testlib.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -755,8 +755,8 @@ callCallbackWithByReferenceArgument(int (*func)(int arg, int* result), int arg,
755755
}
756756

757757
EXPORT char*
758-
callStringCallback(char* (*func)(char* arg), char* arg) {
759-
return (*func)(arg);
758+
callStringCallback(char* (*func)(const char* arg, const char* arg2), const char* arg, const char* arg2) {
759+
return (*func)(arg, arg2);
760760
}
761761

762762
EXPORT char**
@@ -765,8 +765,8 @@ callStringArrayCallback(char** (*func)(char** arg), char** arg) {
765765
}
766766

767767
EXPORT wchar_t*
768-
callWideStringCallback(wchar_t* (*func)(wchar_t* arg), wchar_t* arg) {
769-
return (*func)(arg);
768+
callWideStringCallback(wchar_t* (*func)(const wchar_t* arg, const wchar_t* arg2), const wchar_t* arg, const wchar_t* arg2) {
769+
return (*func)(arg, arg2);
770770
}
771771

772772
struct cbstruct {

‎src/com/sun/jna/CallbackReference.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,14 @@ private CallbackReference(Callback callback, int callingConvention, boolean dire
165165
direct = false;
166166
break;
167167
}
168-
// No TypeMapper support in native callback code
168+
// Direct mode callbacks do not support TypeMapper
169169
if (mapper != null
170170
&& mapper.getFromNativeConverter(ptypes[i]) != null) {
171171
direct = false;
172172
break;
173173
}
174174
}
175+
// Direct mode callbacks do not support TypeMapper
175176
if (mapper != null
176177
&& mapper.getToNativeConverter(m.getReturnType()) != null) {
177178
direct = false;

0 commit comments

Comments
 (0)