Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 0/7] java: Various fixes to RStruct/RStructList bindings (RHBZ#1073906).
See: https://bugzilla.redhat.com/show_bug.cgi?id=1073906 I'm still running the full tests on these. Rich.
Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 1/7] java: run: Add java/.libs to LD_LIBRARY_PATH so JVM finds the right JNI file.
--- java/run-bindtests | 2 +- java/run-java-tests | 2 +- run.in | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/java/run-bindtests b/java/run-bindtests index a9bda18..0718f12 100755 --- a/java/run-bindtests +++ b/java/run-bindtests @@ -18,5 +18,5 @@ set -e -$JAVA_EXE -Djava.library.path=.libs Bindtests > bindtests.tmp +$JAVA_EXE Bindtests > bindtests.tmp diff -u $srcdir/../bindtests bindtests.tmp diff --git a/java/run-java-tests b/java/run-java-tests index c344396..f824c50 100755 --- a/java/run-java-tests +++ b/java/run-java-tests @@ -20,5 +20,5 @@ set -e for f in $(ls -1 t/*.class | fgrep -v \$); do classname=$(basename $f .class) - $JAVA_EXE -Djava.library.path=.libs -ea $classname + $JAVA_EXE -ea $classname done diff --git a/run.in b/run.in index d09210f..55942c6 100755 --- a/run.in +++ b/run.in @@ -79,9 +79,9 @@ export PATH # Set LD_LIBRARY_PATH to contain library. if [ -z "$LD_LIBRARY_PATH" ]; then - LD_LIBRARY_PATH="$b/src/.libs:$b/gobject/.libs" + LD_LIBRARY_PATH="$b/src/.libs:$b/java/.libs:$b/gobject/.libs" else - LD_LIBRARY_PATH="$b/src/.libs:$b/gobject/.libs:$LD_LIBRARY_PATH" + LD_LIBRARY_PATH="$b/src/.libs:$b/java/.libs:$b/gobject/.libs:$LD_LIBRARY_PATH" fi export LD_LIBRARY_PATH -- 1.8.5.3
Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 2/7] java: Fix bogus construction of all RStructList returned values (RHBZ#1073906).
Thanks Maarten on IRC for spotting the problem.
---
generator/java.ml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/generator/java.ml b/generator/java.ml
index 2477e35..db08956 100644
--- a/generator/java.ml
+++ b/generator/java.ml
@@ -1197,7 +1197,7 @@ and generate_java_struct_list_return typ jtyp cols
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"C\");\n" name;
pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
) cols;
- pr " (*env)->SetObjectArrayElement (env, jfl, i, jfl);\n";
+ pr " (*env)->SetObjectArrayElement (env, jr, i, jfl);\n";
pr " }\n";
pr " guestfs_free_%s_list (r);\n" typ;
pr " return jr;\n"
--
1.8.5.3
Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 3/7] java: Split long lines in generated output, and add other whitespace.
No functional change.
---
generator/java.ml | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/generator/java.ml b/generator/java.ml
index db08956..9f68eb1 100644
--- a/generator/java.ml
+++ b/generator/java.ml
@@ -1160,20 +1160,24 @@ and generate_java_struct_return typ jtyp cols and
generate_java_struct_list_return typ jtyp cols pr " cl =
(*env)->FindClass (env, \"com/redhat/et/libguestfs/%s\");\n"
jtyp;
pr " jr = (*env)->NewObjectArray (env, r->len, cl,
NULL);\n";
+ pr "\n";
pr " for (i = 0; i < r->len; ++i) {\n";
pr " jfl = (*env)->AllocObject (env, cl);\n";
+ pr "\n";
List.iter (
function
| name, FString ->
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"Ljava/lang/String;\");\n" name;
- pr " (*env)->SetObjectField (env, jfl, fl,
(*env)->NewStringUTF (env, r->val[i].%s));\n" name;
+ pr " (*env)->SetObjectField (env, jfl, fl,\n";
+ pr " (*env)->NewStringUTF (env,
r->val[i].%s));\n" name;
| name, FUUID ->
pr " {\n";
pr " char s[33];\n";
pr " memcpy (s, r->val[i].%s, 32);\n" name;
pr " s[32] = 0;\n";
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"Ljava/lang/String;\");\n" name;
- pr " (*env)->SetObjectField (env, jfl, fl,
(*env)->NewStringUTF (env, s));\n";
+ pr " (*env)->SetObjectField (env, jfl, fl,\n";
+ pr " (*env)->NewStringUTF (env,
s));\n";
pr " }\n";
| name, FBuffer ->
pr " {\n";
@@ -1182,7 +1186,8 @@ and generate_java_struct_list_return typ jtyp cols
pr " memcpy (s, r->val[i].%s, len);\n" name;
pr " s[len] = 0;\n";
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"Ljava/lang/String;\");\n" name;
- pr " (*env)->SetObjectField (env, jfl, fl,
(*env)->NewStringUTF (env, s));\n";
+ pr " (*env)->SetObjectField (env, jfl, fl,\n";
+ pr " (*env)->NewStringUTF (env,
s));\n";
pr " }\n";
| name, (FBytes|FUInt64|FInt64) ->
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"J\");\n" name;
@@ -1197,8 +1202,10 @@ and generate_java_struct_list_return typ jtyp cols
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"C\");\n" name;
pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
) cols;
+ pr "\n";
pr " (*env)->SetObjectArrayElement (env, jr, i, jfl);\n";
pr " }\n";
+ pr "\n";
pr " guestfs_free_%s_list (r);\n" typ;
pr " return jr;\n"
--
1.8.5.3
Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 4/7] java: Factor out common field code in RStructList.
No functional change.
---
generator/java.ml | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/generator/java.ml b/generator/java.ml
index 9f68eb1..cce1758 100644
--- a/generator/java.ml
+++ b/generator/java.ml
@@ -1165,41 +1165,46 @@ and generate_java_struct_list_return typ jtyp cols pr
" jfl = (*env)->AllocObject (env, cl);\n";
pr "\n";
List.iter (
- function
- | name, FString ->
- pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"Ljava/lang/String;\");\n" name;
+ fun (name, ftyp) ->
+ (* Get the field ID in 'fl'. *)
+ let java_field_type = match ftyp with
+ | FString | FUUID | FBuffer -> "Ljava/lang/String;"
+ | FBytes | FUInt64 | FInt64 -> "J"
+ | FUInt32 | FInt32 -> "I"
+ | FOptPercent -> "F"
+ | FChar -> "C" in
+ pr " fl = (*env)->GetFieldID (env, cl,
\"%s\",\n" name;
+ pr " \"%s\");\n"
java_field_type;
+
+ (* Assign the value to this field. *)
+ match ftyp with
+ | FString ->
pr " (*env)->SetObjectField (env, jfl, fl,\n";
pr " (*env)->NewStringUTF (env,
r->val[i].%s));\n" name;
- | name, FUUID ->
+ | FUUID ->
pr " {\n";
pr " char s[33];\n";
pr " memcpy (s, r->val[i].%s, 32);\n" name;
pr " s[32] = 0;\n";
- pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"Ljava/lang/String;\");\n" name;
pr " (*env)->SetObjectField (env, jfl, fl,\n";
pr " (*env)->NewStringUTF (env,
s));\n";
pr " }\n";
- | name, FBuffer ->
+ | FBuffer ->
pr " {\n";
pr " size_t len = r->val[i].%s_len;\n" name;
pr " char s[len+1];\n";
pr " memcpy (s, r->val[i].%s, len);\n" name;
pr " s[len] = 0;\n";
- pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"Ljava/lang/String;\");\n" name;
pr " (*env)->SetObjectField (env, jfl, fl,\n";
pr " (*env)->NewStringUTF (env,
s));\n";
pr " }\n";
- | name, (FBytes|FUInt64|FInt64) ->
- pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"J\");\n" name;
+ | FBytes|FUInt64|FInt64 ->
pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
- | name, (FUInt32|FInt32) ->
- pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"I\");\n" name;
+ | FUInt32|FInt32 ->
pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
- | name, FOptPercent ->
- pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"F\");\n" name;
+ | FOptPercent ->
pr " (*env)->SetFloatField (env, jfl, fl,
r->val[i].%s);\n" name;
- | name, FChar ->
- pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"C\");\n" name;
+ | FChar ->
pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
) cols;
pr "\n";
--
1.8.5.3
Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 5/7] java: Use correct Set*Field JNI accessors to set fields of the appropriate type.
Using the wrong accessors (somehow - I have no idea how) caused other
fields in the struct to contain incorrect values.
---
generator/java.ml | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/generator/java.ml b/generator/java.ml
index cce1758..e9c5949 100644
--- a/generator/java.ml
+++ b/generator/java.ml
@@ -1146,13 +1146,13 @@ and generate_java_struct_return typ jtyp cols
pr " (*env)->SetLongField (env, jr, fl, r->%s);\n" name;
| name, (FUInt32|FInt32) ->
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"I\");\n" name;
- pr " (*env)->SetLongField (env, jr, fl, r->%s);\n"
name;
+ pr " (*env)->SetIntField (env, jr, fl, r->%s);\n"
name;
| name, FOptPercent ->
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"F\");\n" name;
pr " (*env)->SetFloatField (env, jr, fl, r->%s);\n"
name;
| name, FChar ->
pr " fl = (*env)->GetFieldID (env, cl, \"%s\",
\"C\");\n" name;
- pr " (*env)->SetLongField (env, jr, fl, r->%s);\n"
name;
+ pr " (*env)->SetCharField (env, jr, fl, r->%s);\n"
name;
) cols;
pr " free (r);\n";
pr " return jr;\n"
@@ -1201,11 +1201,11 @@ and generate_java_struct_list_return typ jtyp cols
| FBytes|FUInt64|FInt64 ->
pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
| FUInt32|FInt32 ->
- pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
+ pr " (*env)->SetIntField (env, jfl, fl,
r->val[i].%s);\n" name;
| FOptPercent ->
pr " (*env)->SetFloatField (env, jfl, fl,
r->val[i].%s);\n" name;
| FChar ->
- pr " (*env)->SetLongField (env, jfl, fl,
r->val[i].%s);\n" name;
+ pr " (*env)->SetCharField (env, jfl, fl,
r->val[i].%s);\n" name;
) cols;
pr "\n";
pr " (*env)->SetObjectArrayElement (env, jr, i, jfl);\n";
--
1.8.5.3
Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 6/7] bindtests: Fill in all fields in dummy lvm_pv struct.
Used for testing RStruct/RStructList return values, but only
in GObject and Java bindings.
---
generator/bindtests.ml | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/generator/bindtests.ml b/generator/bindtests.ml
index 9f459ac..cad123e 100644
--- a/generator/bindtests.ml
+++ b/generator/bindtests.ml
@@ -104,6 +104,28 @@ print_strings (guestfs_h *g, char *const *argv)
fprintf (fp, \"]\\n\");
}
+/* Fill an lvm_pv struct with known data. Used by
+ * guestfs_internal_test_rstruct & guestfs_internal_test_rstructlist.
+ */
+static void
+fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i)
+{
+ pv->pv_name = safe_asprintf (g, \"pv%%zu\", i);
+ memcpy (pv->pv_uuid, \"12345678901234567890123456789012\", 32);
+ pv->pv_fmt = safe_strdup (g, \"unknown\");
+ pv->pv_size = i;
+ pv->dev_size = i;
+ pv->pv_free = i;
+ pv->pv_used = i;
+ pv->pv_attr = safe_asprintf (g, \"attr%%zu\", i);
+ pv->pv_pe_count = i;
+ pv->pv_pe_alloc_count = i;
+ pv->pv_tags = safe_asprintf (g, \"tag%%zu\", i);
+ pv->pe_start = i;
+ pv->pv_mda_count = i;
+ pv->pv_mda_free = i;
+}
+
";
let ptests, rtests @@ -239,7 +261,8 @@ print_strings (guestfs_h *g, char
*const *argv)
pr " return strs;\n"
| RStruct (_, typ) ->
pr " struct guestfs_%s *r;\n" typ;
- pr " r = safe_calloc (g, sizeof *r, 1);\n";
+ pr " r = safe_malloc (g, sizeof *r);\n";
+ pr " fill_lvm_pv (g, r, 0);\n";
pr " return r;\n"
| RStructList (_, typ) ->
pr " struct guestfs_%s_list *r;\n" typ;
@@ -248,12 +271,11 @@ print_strings (guestfs_h *g, char *const *argv)
pr " error (g, \"%%s: expecting uint32
argument\", \"%s\");\n" name;
pr " return NULL;\n";
pr " }\n";
- pr " r = safe_calloc (g, sizeof *r, 1);\n";
+ pr " r = safe_malloc (g, sizeof *r);\n";
pr " r->len = len;\n";
- pr " r->val = safe_calloc (g, r->len, sizeof
*r->val);\n";
- pr " for (size_t i = 0; i < r->len; i++) {\n";
- pr " r->val[i].pv_size = i;\n";
- pr " }\n";
+ pr " r->val = safe_malloc (g, r->len * sizeof
(*r->val));\n";
+ pr " for (size_t i = 0; i < r->len; i++)\n";
+ pr " fill_lvm_pv (g, &r->val[i], i);\n";
pr " return r;\n"
| RHashtable _ ->
pr " char **strs;\n";
--
1.8.5.3
Richard W.M. Jones
2014-Mar-07 15:28 UTC
[Libguestfs] [PATCH 7/7] java: Add regression test for RStruct/RStructList (RHBZ#1073906).
---
java/Makefile.am | 3 +-
java/t/GuestFS800RHBZ1073906.java | 75 +++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+), 1 deletion(-)
create mode 100644 java/t/GuestFS800RHBZ1073906.java
diff --git a/java/Makefile.am b/java/Makefile.am
index 7356685..01836ad 100644
--- a/java/Makefile.am
+++ b/java/Makefile.am
@@ -39,7 +39,8 @@ java_tests = \
t/GuestFS070OptArgs.java \
t/GuestFS100Launch.java \
t/GuestFS410CloseEvent.java \
- t/GuestFS420LogMessages.java
+ t/GuestFS420LogMessages.java \
+ t/GuestFS800RHBZ1073906.java
EXTRA_DIST = \
com/redhat/et/libguestfs/.gitignore \
diff --git a/java/t/GuestFS800RHBZ1073906.java
b/java/t/GuestFS800RHBZ1073906.java
new file mode 100644
index 0000000..ddf2f24
--- /dev/null
+++ b/java/t/GuestFS800RHBZ1073906.java
@@ -0,0 +1,75 @@
+/* libguestfs Java bindings
+ * Copyright (C) 2014 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Regression test for RStruct, RStructList problems.
+ * https://bugzilla.redhat.com/show_bug.cgi?id=1073906
+ */
+
+import java.io.*;
+import java.util.Map;
+import com.redhat.et.libguestfs.*;
+
+public class GuestFS800RHBZ1073906
+{
+ public static void main (String[] argv)
+ {
+ try {
+ GuestFS g = new GuestFS ();
+
+ PV pv = g.internal_test_rstruct ("");
+ assert pv.pv_name.equals ("pv0");
+ assert pv.pv_size == 0;
+ assert pv.pv_fmt.equals ("unknown");
+ assert pv.pv_attr.equals ("attr0");
+ assert pv.pv_tags.equals ("tag0");
+
+ PV[] pva = g.internal_test_rstructlist ("5");
+ assert pva[0].pv_name.equals ("pv0");
+ assert pva[0].pv_size == 0;
+ assert pva[0].pv_fmt.equals ("unknown");
+ assert pva[0].pv_attr.equals ("attr0");
+ assert pva[0].pv_tags.equals ("tag0");
+ assert pva[1].pv_name.equals ("pv1");
+ assert pva[1].pv_size == 1;
+ assert pva[1].pv_fmt.equals ("unknown");
+ assert pva[1].pv_attr.equals ("attr1");
+ assert pva[1].pv_tags.equals ("tag1");
+ assert pva[2].pv_name.equals ("pv2");
+ assert pva[2].pv_size == 2;
+ assert pva[2].pv_fmt.equals ("unknown");
+ assert pva[2].pv_attr.equals ("attr2");
+ assert pva[2].pv_tags.equals ("tag2");
+ assert pva[3].pv_name.equals ("pv3");
+ assert pva[3].pv_size == 3;
+ assert pva[3].pv_fmt.equals ("unknown");
+ assert pva[3].pv_attr.equals ("attr3");
+ assert pva[3].pv_tags.equals ("tag3");
+ assert pva[4].pv_name.equals ("pv4");
+ assert pva[4].pv_size == 4;
+ assert pva[4].pv_fmt.equals ("unknown");
+ assert pva[4].pv_attr.equals ("attr4");
+ assert pva[4].pv_tags.equals ("tag4");
+
+ g.close ();
+ }
+ catch (Exception exn) {
+ System.err.println (exn);
+ System.exit (1);
+ }
+ }
+}
--
1.8.5.3