On Wed, Oct 19, 2011 at 04:53:30PM -0700, Alex Nelson wrote:
@@ -1622,6 +1629,7 @@ and generate_ocaml_prototype ?(is_external = false) name style = [...] + | RLenValue -> pr "value * int"
The declared return type is a pair (value * int), but ...
@@ -1981,6 +1997,20 @@ copy_type_len (size_t len, hive_type t) }
static value +copy_len_value (size_t len, hive_value_h r) +{ + CAMLparam0 (); + CAMLlocal2 (v, rv); + + rv = caml_alloc (2, 0); + v = Val_int (len); + Store_field (rv, 0, v); + v = Val_int (r); + Store_field (rv, 1, v); + CAMLreturn (rv); +}
... what you're actually returning is the length (field 0) and the value (field 1). So one of these definitions is backwards. There is a test in a later patch, but it didn't pick up the bug. Not surprising: All of the tests check a cell that returns (0,0), so none of the tests could have picked up this type of bug. I think the tests (all of them) need to be changed to be more thorough.
@@ -2244,6 +2275,7 @@ and generate_perl_prototype name style = | RString -> pr "$string = " | RStringList -> pr "@strings = " | RLenType -> pr "($type, $len) = " + | RLenValue -> pr "($len, $value) = " | RLenTypeVal -> pr "($type, $data) = " | RInt32 -> pr "$int32 = " | RInt64 -> pr "$int64 = " @@ -2467,6 +2499,7 @@ DESTROY (h) | RValueList | RStringList | RLenType + | RLenValue | RLenTypeVal -> pr "void\n" | RInt32 -> pr "SV *\n" | RInt64 -> pr "SV *\n" @@ -2639,6 +2672,22 @@ DESTROY (h) pr " PUSHs (sv_2mortal (newSViv (type)));\n"; pr " PUSHs (sv_2mortal (newSViv (len)));\n";
+ | RLenValue -> + pr "PREINIT:\n"; + pr " hive_value_h r;\n"; + pr " size_t len;\n"; + pr " PPCODE:\n"; + pr " errno = 0;\n"; + pr " r = hivex_%s (%s, &len);\n" + name (String.concat ", " c_params); + free_args (); + pr " if (r == 0 && errno)\n"; + pr " croak (\"%%s: \", \"%s\", strerror (errno));\n" + name; + pr " EXTEND (SP, 2);\n"; + pr " PUSHs (sv_2mortal (newSViv (len)));\n"; + pr " PUSHs (sv_2mortal (newSViv (r)));\n";
I can't remember now the details of which order you have to push return values in Perl, but there may or may not be the same bug here too. The test wouldn't pick it up. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/