[PATCH 3/7] generator: Add new return types to ABI: RLenNode and RLenValue
RLenNode and RLenValue are similar to RLenType, though with one less argument. This required adding one processing function for OCaml and Python. Signed-off-by: Alex Nelson <ajnelson@cs.ucsc.edu> --- generator/generator.ml | 114 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 114 insertions(+), 0 deletions(-) diff --git a/generator/generator.ml b/generator/generator.ml index 48b6e3a..15ebc44 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -49,8 +49,10 @@ and ret = | RNode (* Returns hive_node_h or 0. *) | RNodeNotFound (* See hivex_node_get_child. *) | RNodeList (* Returns hive_node_h* or NULL. *) + | RLenNode (* See node_struct_length. *) | RValue (* Returns hive_value_h or 0. *) | RValueList (* Returns hive_value_h* or NULL. *) + | RLenValue (* See value_struct_length. *) | RString (* Returns char* or NULL. *) | RStringList (* Returns char** or NULL. *) | RLenType (* See hivex_value_type. *) @@ -878,6 +880,8 @@ and generate_c_prototype ?(extern = false) name style = | RValueList -> pr "hive_value_h *" | RString -> pr "char *" | RStringList -> pr "char **" + | RLenNode -> pr "hive_node_h " + | RLenValue -> pr "hive_value_h " | RLenType -> pr "int " | RLenTypeVal -> pr "char *" | RInt32 -> pr "int32_t " @@ -899,6 +903,8 @@ and generate_c_prototype ?(extern = false) name style = ) (snd style); (match fst style with | RLenType | RLenTypeVal -> pr ", hive_type *t, size_t *len" + | RLenNode -> pr ", size_t *len" + | RLenValue -> pr ", size_t *len" | _ -> () ); pr ");\n" @@ -1101,6 +1107,14 @@ On error this returns NULL and sets errno.\n\n" pr "\ Returns 0 on success. On error this returns -1 and sets errno.\n\n" + | RLenNode -> + pr "\ +Returns a positive number on success. +On error this returns 0 and sets errno.\n\n" + | RLenValue -> + pr "\ +Returns a positive number on success. +On error this returns 0 and sets errno.\n\n" | RLenTypeVal -> pr "\ The value is returned as an array of bytes (of length C<len>). @@ -1612,6 +1626,8 @@ and generate_ocaml_prototype ?(is_external = false) name style = | RString -> pr "string" | RStringList -> pr "string array" | RLenType -> pr "hive_type * int" + | RLenNode -> pr "node" + | RLenValue -> pr "value" | RLenTypeVal -> pr "hive_type * string" | RInt32 -> pr "int32" | RInt64 -> pr "int64" @@ -1697,6 +1713,8 @@ static void raise_closed (const char *) Noreturn; let c_params = match fst style with | RLenType | RLenTypeVal -> c_params @ [["&t"; "&len"]] + | RLenNode -> c_params @ [["&len"]] + | RLenValue -> c_params @ [["&len"]] | _ -> c_params in let c_params = List.concat c_params in @@ -1769,6 +1787,14 @@ static void raise_closed (const char *) Noreturn; pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenNode -> + pr " int r;\n"; + pr " size_t len;\n"; + "0" + | RLenValue -> + pr " int r;\n"; + pr " size_t len;\n"; + "0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -1849,6 +1875,8 @@ static void raise_closed (const char *) Noreturn; pr " for (int i = 0; r[i] != NULL; ++i) free (r[i]);\n"; pr " free (r);\n" | RLenType -> pr " rv = copy_type_len (len, t);\n" + | RLenNode -> pr " rv = copy_len (len);\n" + | RLenValue -> pr " rv = copy_len (len);\n" | RLenTypeVal -> pr " rv = copy_type_value (r, len, t);\n"; pr " free (r);\n" @@ -1971,6 +1999,18 @@ copy_type_len (size_t len, hive_type t) } static value +copy_type (size_t len, hive_type t) +{ + CAMLparam0 (); + CAMLlocal2 (v, rv); + + rv = caml_alloc (1, 0); + v = Val_int (len); + Store_field (rv, 0, v); + CAMLreturn (rv); +} + +static value copy_type_value (const char *r, size_t len, hive_type t) { CAMLparam0 (); @@ -2160,6 +2200,8 @@ sub open { | RString | RStringList | RLenType + | RLenNode + | RLenValue | RLenTypeVal | RInt32 | RInt64 -> () @@ -2234,6 +2276,8 @@ and generate_perl_prototype name style = | RString -> pr "$string = " | RStringList -> pr "@strings = " | RLenType -> pr "($type, $len) = " + | RLenNode -> pr "($len) = " + | RLenValue -> pr "($len) = " | RLenTypeVal -> pr "($type, $data) = " | RInt32 -> pr "$int32 = " | RInt64 -> pr "$int64 = " @@ -2457,6 +2501,8 @@ DESTROY (h) | RValueList | RStringList | RLenType + | RLenNode + | RLenValue | RLenTypeVal -> pr "void\n" | RInt32 -> pr "SV *\n" | RInt64 -> pr "SV *\n" @@ -2629,6 +2675,34 @@ DESTROY (h) pr " PUSHs (sv_2mortal (newSViv (type)));\n"; pr " PUSHs (sv_2mortal (newSViv (len)));\n"; + | RLenNode -> + pr "PREINIT:\n"; + pr " int r;\n"; + pr " size_t len;\n"; + pr " PPCODE:\n"; + pr " r = hivex_%s (%s, &len);\n" + name (String.concat ", " c_params); + free_args (); + pr " if (r == 0)\n"; + pr " croak (\"%%s: \", \"%s\", strerror (errno));\n" + name; + pr " EXTEND (SP, 2);\n"; + pr " PUSHs (sv_2mortal (newSViv (len)));\n"; + + | RLenValue -> + pr "PREINIT:\n"; + pr " int r;\n"; + pr " size_t len;\n"; + pr " PPCODE:\n"; + pr " r = hivex_%s (%s, &len);\n" + name (String.concat ", " c_params); + free_args (); + pr " if (r == 0)\n"; + pr " croak (\"%%s: \", \"%s\", strerror (errno));\n" + name; + pr " EXTEND (SP, 2);\n"; + pr " PUSHs (sv_2mortal (newSViv (len)));\n"; + | RLenTypeVal -> pr "PREINIT:\n"; pr " char *r;\n"; @@ -2867,6 +2941,14 @@ put_len_type (size_t len, hive_type t) } static PyObject * +put_len (size_t len) +{ + PyObject *r = PyTuple_New (1); + PyTuple_SetItem (r, 0, PyLong_FromLongLong ((long) len)); + return r; +} + +static PyObject * put_val_type (char *val, size_t len, hive_type t) { PyObject *r = PyTuple_New (2); @@ -2906,6 +2988,14 @@ put_val_type (char *val, size_t len, hive_type t) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenNode -> + pr " int r;\n"; + pr " size_t len;\n"; + "0" + | RLenValue -> + pr " int r;\n"; + pr " size_t len;\n"; + "0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -2930,6 +3020,7 @@ put_val_type (char *val, size_t len, hive_type t) let c_params = match fst style with | RLenType | RLenTypeVal -> c_params @ ["&t"; "&len"] + | RLenNode | RLenValue -> c_params @ ["&len"] | _ -> c_params in List.iter ( @@ -3074,6 +3165,10 @@ put_val_type (char *val, size_t len, hive_type t) pr " free_strings (r);\n" | RLenType -> pr " py_r = put_len_type (len, t);\n" + | RLenNode -> + pr " py_r = put_len (len);\n" + | RLenValue -> + pr " py_r = put_len (len);\n" | RLenTypeVal -> pr " py_r = put_val_type (r, len, t);\n"; pr " free (r);\n" @@ -3284,6 +3379,8 @@ get_values (VALUE valuesv, size_t *nr_values) | RString -> "string" | RStringList -> "list" | RLenType -> "hash" + | RLenNode -> "integer" + | RLenValue -> "integer" | RLenTypeVal -> "hash" | RInt32 -> "integer" | RInt64 -> "integer" in @@ -3382,6 +3479,14 @@ get_values (VALUE valuesv, size_t *nr_values) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenNode -> + pr " int r;\n"; + pr " size_t len;\n"; + "0" + | RLenValue -> + pr " int r;\n"; + pr " size_t len;\n"; + "0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -3406,6 +3511,7 @@ get_values (VALUE valuesv, size_t *nr_values) let c_params = match ret with | RLenType | RLenTypeVal -> c_params @ [["&t"; "&len"]] + | RLenNode | RLenValue -> c_params @ [["&len"]] | _ -> c_params in let c_params = List.concat c_params in @@ -3487,6 +3593,14 @@ get_values (VALUE valuesv, size_t *nr_values) pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"type\")), INT2NUM (t));\n"; pr " return rv;\n" + | RLenNode -> + pr " VALUE rv = rb_hash_new ();\n"; + pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; + pr " return rv;\n" + | RLenValue -> + pr " VALUE rv = rb_hash_new ();\n"; + pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; + pr " return rv;\n" | RLenTypeVal -> pr " VALUE rv = rb_hash_new ();\n"; pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; -- 1.7.6
On Thu, Sep 01, 2011 at 06:17:31PM -0700, Alex Nelson wrote:
RLenNode and RLenValue are similar to RLenType, though with one less argument. This required adding one processing function for OCaml and Python.
Seems like this patch is partly unnecessary? At least, nothing in any later patch appears to return RLenNode. The two functions that previously used this now return RSize (which is a good thing). As for RLenValue, the new function value_data_cell_offset returns this, so this part of the patch should be included. I think you should rework this patch. 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/
Hi Rich, You're right, I had too many new additions. Please find attached a revision of this patch. I also found I misnamed one of the binding functions and forgot its prototype. That's fixed now. `make check` had all tests pass, 1 test not run. Sorry for not running that earlier. Neither of the other patches are directly affected by this, but if you need me to resubmit them anyway please let me know. I ran `make check` against the branch that also has those two patches' changes. --Alex On Sep 6, 2011, at 06:27 , Richard W.M. Jones wrote:
On Thu, Sep 01, 2011 at 06:17:31PM -0700, Alex Nelson wrote:
RLenNode and RLenValue are similar to RLenType, though with one less argument. This required adding one processing function for OCaml and Python.
Seems like this patch is partly unnecessary?
At least, nothing in any later patch appears to return RLenNode. The two functions that previously used this now return RSize (which is a good thing).
As for RLenValue, the new function value_data_cell_offset returns this, so this part of the patch should be included. I think you should rework this patch.
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/
On Tue, Sep 06, 2011 at 09:40:34AM -0700, Alex Nelson wrote:
Neither of the other patches are directly affected by this, but if you need me to resubmit them anyway please let me know.
No need to resubmit the other patches. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
There are still a few mistakes. See my comments inline.
From 02ce84fd959bf67afc6999eaeecef78ccdf57c71 Mon Sep 17 00:00:00 2001 From: Alex Nelson <ajnelson@cs.ucsc.edu> Date: Tue, 6 Sep 2011 09:27:51 -0700 Subject: [PATCH] generator: Add new return type to ABI: RLenValue
RLenValue is are similar to RLenType, though with one less argument. This required adding one processing function for OCaml and Python.
Signed-off-by: Alex Nelson <ajnelson@cs.ucsc.edu> --- generator/generator.ml | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/generator/generator.ml b/generator/generator.ml index b6fb8b3..2703023 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -51,6 +51,7 @@ and ret = | RNodeList (* Returns hive_node_h* or NULL. *) | RValue (* Returns hive_value_h or 0. *) | RValueList (* Returns hive_value_h* or NULL. *) + | RLenValue (* See value_struct_length. *)
I think the comment should read "see value_data_cell_offset", but I'd prefer it if the comment was "Returns offset and length of value" since that describes what it actually does.
| RString (* Returns char* or NULL. *) | RStringList (* Returns char** or NULL. *) | RLenType (* See hivex_value_type. *) @@ -890,6 +891,7 @@ and generate_c_prototype ?(extern = false) name style = | RValueList -> pr "hive_value_h *" | RString -> pr "char *" | RStringList -> pr "char **" + | RLenValue -> pr "hive_value_h " | RLenType -> pr "int " | RLenTypeVal -> pr "char *" | RInt32 -> pr "int32_t " @@ -911,6 +913,7 @@ and generate_c_prototype ?(extern = false) name style = ) (snd style); (match fst style with | RLenType | RLenTypeVal -> pr ", hive_type *t, size_t *len" + | RLenValue -> pr ", size_t *len" | _ -> () ); pr ");\n" @@ -1113,6 +1116,10 @@ On error this returns NULL and sets errno.\n\n" pr "\ Returns 0 on success. On error this returns -1 and sets errno.\n\n" + | RLenValue -> + pr "\ +Returns a positive number on success. +On error this returns 0 and sets errno.\n\n"
This should be: Returns a value handle. On error this returns 0 and sets errno.\n\n"
@@ -1687,6 +1695,7 @@ static hive_type HiveType_val (value); static value Val_hive_type (hive_type); static value copy_int_array (size_t *); static value copy_type_len (size_t, hive_type); +static value copy_len (size_t); static value copy_type_value (const char *, size_t, hive_type); static void raise_error (const char *) Noreturn; static void raise_closed (const char *) Noreturn; @@ -1709,6 +1718,7 @@ static void raise_closed (const char *) Noreturn; let c_params = match fst style with | RLenType | RLenTypeVal -> c_params @ [["&t"; "&len"]] + | RLenValue -> c_params @ [["&len"]] | _ -> c_params in let c_params = List.concat c_params in
@@ -1781,6 +1791,10 @@ static void raise_closed (const char *) Noreturn; pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " int r;\n"; + pr " size_t len;\n"; + "0"
'r' is not an int. It's a hive_value_h.
| RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -1861,6 +1875,7 @@ static void raise_closed (const char *) Noreturn; pr " for (int i = 0; r[i] != NULL; ++i) free (r[i]);\n"; pr " free (r);\n" | RLenType -> pr " rv = copy_type_len (len, t);\n" + | RLenValue -> pr " rv = copy_len (len);\n"
This call is wrong, and so is copy_len below. What happens to 'r'?
| RLenTypeVal -> pr " rv = copy_type_value (r, len, t);\n"; pr " free (r);\n" @@ -1983,6 +1998,18 @@ copy_type_len (size_t len, hive_type t) }
static value +copy_len (size_t len) +{ + CAMLparam0 (); + CAMLlocal2 (v, rv); + + rv = caml_alloc (1, 0); + v = Val_int (len); + Store_field (rv, 0, v); + CAMLreturn (rv); +}
This creates a tuple with a single element, which OCaml can't actually process. In any case you need to be passing two parameters here: the value and the len, and you need to make a 2-element tuple containing both of them.
+static value copy_type_value (const char *r, size_t len, hive_type t) { CAMLparam0 (); @@ -2172,6 +2199,7 @@ sub open { | RString | RStringList | RLenType + | RLenValue | RLenTypeVal | RInt32 | RInt64 -> () @@ -2246,6 +2274,7 @@ and generate_perl_prototype name style = | RString -> pr "$string = " | RStringList -> pr "@strings = " | RLenType -> pr "($type, $len) = " + | RLenValue -> pr "($len) = "
Just a single length? What about the value?
| RLenTypeVal -> pr "($type, $data) = " | RInt32 -> pr "$int32 = " | RInt64 -> pr "$int64 = " @@ -2469,6 +2498,7 @@ DESTROY (h) | RValueList | RStringList | RLenType + | RLenValue | RLenTypeVal -> pr "void\n" | RInt32 -> pr "SV *\n" | RInt64 -> pr "SV *\n" @@ -2641,6 +2671,20 @@ DESTROY (h) pr " PUSHs (sv_2mortal (newSViv (type)));\n"; pr " PUSHs (sv_2mortal (newSViv (len)));\n";
+ | RLenValue -> + pr "PREINIT:\n"; + pr " int r;\n";
int r? The return from the function is a hive_value_h.
+ pr " size_t len;\n"; + pr " PPCODE:\n"; + pr " r = hivex_%s (%s, &len);\n" + name (String.concat ", " c_params); + free_args (); + pr " if (r == 0)\n"; + pr " croak (\"%%s: \", \"%s\", strerror (errno));\n" + name; + pr " EXTEND (SP, 2);\n"; + pr " PUSHs (sv_2mortal (newSViv (len)));\n";
You extend the stack by two elements, then push one element. This would cause Perl to crash. In any case you need to push both the value (r) and the length. I can't remember in which order you need to push them -- take a look at other examples.
| RLenTypeVal -> pr "PREINIT:\n"; pr " char *r;\n"; @@ -2879,6 +2923,14 @@ put_len_type (size_t len, hive_type t) }
static PyObject * +put_len (size_t len) +{ + PyObject *r = PyTuple_New (1); + PyTuple_SetItem (r, 0, PyLong_FromLongLong ((long) len)); + return r; +}
Same problem in Python as in OCaml and Perl. It's probably going to help if you make some small test programs in all languages, so you can check the returned values are marshalled properly.
+static PyObject * put_val_type (char *val, size_t len, hive_type t) { PyObject *r = PyTuple_New (2); @@ -2918,6 +2970,10 @@ put_val_type (char *val, size_t len, hive_type t) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " int r;\n";
As above.
+ pr " size_t len;\n"; + "0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -2942,6 +2998,7 @@ put_val_type (char *val, size_t len, hive_type t) let c_params = match fst style with | RLenType | RLenTypeVal -> c_params @ ["&t"; "&len"] + | RLenValue -> c_params @ ["&len"] | _ -> c_params in
List.iter ( @@ -3086,6 +3143,8 @@ put_val_type (char *val, size_t len, hive_type t) pr " free_strings (r);\n" | RLenType -> pr " py_r = put_len_type (len, t);\n" + | RLenValue -> + pr " py_r = put_len (len);\n"
As above.
| RLenTypeVal -> pr " py_r = put_val_type (r, len, t);\n"; pr " free (r);\n" @@ -3296,6 +3355,7 @@ get_values (VALUE valuesv, size_t *nr_values) | RString -> "string" | RStringList -> "list" | RLenType -> "hash" + | RLenValue -> "integer" | RLenTypeVal -> "hash" | RInt32 -> "integer" | RInt64 -> "integer" in @@ -3394,6 +3454,10 @@ get_values (VALUE valuesv, size_t *nr_values) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " int r;\n";
As above (Ruby).
+ pr " size_t len;\n"; + "0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -3418,6 +3482,7 @@ get_values (VALUE valuesv, size_t *nr_values) let c_params = match ret with | RLenType | RLenTypeVal -> c_params @ [["&t"; "&len"]] + | RLenValue -> c_params @ [["&len"]] | _ -> c_params in let c_params = List.concat c_params in
@@ -3499,6 +3564,10 @@ get_values (VALUE valuesv, size_t *nr_values) pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"type\")), INT2NUM (t));\n"; pr " return rv;\n" + | RLenValue -> + pr " VALUE rv = rb_hash_new ();\n"; + pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; + pr " return rv;\n"
As above.
| RLenTypeVal -> pr " VALUE rv = rb_hash_new ();\n"; pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n";
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/
Hi Rich, Sorry for letting this one sit for a while. I decided the appropriate way to make sure everything was correct was to just write some unit tests and include them for runs with 'make check' - a bit more than you said to do, but I think it makes sense. Unfortunately, an OCaml type error is biting me, and I'm hung on that for verifying the OCaml bindings. In the attached patch, the test ocaml/t/hivex_120_rlenvalue.ml has an error arising from the type abbreviation (is that the right term here? Or is it "synonym"?) of "value" and "int". From what I can do at the OCaml prompt (pwd=.../hivex/ocaml), this is the confusion: # (2:Hivex.value);; Error: This expression has type int but an expression was expected of type Hivex.value # (The 2 is underlined to specify the error.) I get similar confusion later from the format string and trying to coerce the returned values to vanilla ints. (I semi-understand "coerce" is also the wrong term to use here, since that's apparently a term for strictly-ordered subclass abstraction.) May I please have your help with this type confusion? Aside: The attached patch is not intended to be the final correction of this patch 3/7. It's all the remaining unapplied of the work from the 7-patch series, which I will break up as previously planned. --Alex On Sep 6, 2011, at 10:00 , Richard W.M. Jones wrote:
There are still a few mistakes. See my comments inline.
From 02ce84fd959bf67afc6999eaeecef78ccdf57c71 Mon Sep 17 00:00:00 2001 From: Alex Nelson <ajnelson@cs.ucsc.edu> Date: Tue, 6 Sep 2011 09:27:51 -0700 Subject: [PATCH] generator: Add new return type to ABI: RLenValue
RLenValue is are similar to RLenType, though with one less argument. This required adding one processing function for OCaml and Python.
Signed-off-by: Alex Nelson <ajnelson@cs.ucsc.edu> --- generator/generator.ml | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/generator/generator.ml b/generator/generator.ml index b6fb8b3..2703023 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -51,6 +51,7 @@ and ret = | RNodeList (* Returns hive_node_h* or NULL. *) | RValue (* Returns hive_value_h or 0. *) | RValueList (* Returns hive_value_h* or NULL. *) + | RLenValue (* See value_struct_length. *)
I think the comment should read "see value_data_cell_offset", but I'd prefer it if the comment was "Returns offset and length of value" since that describes what it actually does.
| RString (* Returns char* or NULL. *) | RStringList (* Returns char** or NULL. *) | RLenType (* See hivex_value_type. *) @@ -890,6 +891,7 @@ and generate_c_prototype ?(extern = false) name style = | RValueList -> pr "hive_value_h *" | RString -> pr "char *" | RStringList -> pr "char **" + | RLenValue -> pr "hive_value_h " | RLenType -> pr "int " | RLenTypeVal -> pr "char *" | RInt32 -> pr "int32_t " @@ -911,6 +913,7 @@ and generate_c_prototype ?(extern = false) name style = ) (snd style); (match fst style with | RLenType | RLenTypeVal -> pr ", hive_type *t, size_t *len" + | RLenValue -> pr ", size_t *len" | _ -> () ); pr ");\n" @@ -1113,6 +1116,10 @@ On error this returns NULL and sets errno.\n\n" pr "\ Returns 0 on success. On error this returns -1 and sets errno.\n\n" + | RLenValue -> + pr "\ +Returns a positive number on success. +On error this returns 0 and sets errno.\n\n"
This should be:
Returns a value handle. On error this returns 0 and sets errno.\n\n"
@@ -1687,6 +1695,7 @@ static hive_type HiveType_val (value); static value Val_hive_type (hive_type); static value copy_int_array (size_t *); static value copy_type_len (size_t, hive_type); +static value copy_len (size_t); static value copy_type_value (const char *, size_t, hive_type); static void raise_error (const char *) Noreturn; static void raise_closed (const char *) Noreturn; @@ -1709,6 +1718,7 @@ static void raise_closed (const char *) Noreturn; let c_params = match fst style with | RLenType | RLenTypeVal -> c_params @ [["&t"; "&len"]] + | RLenValue -> c_params @ [["&len"]] | _ -> c_params in let c_params = List.concat c_params in
@@ -1781,6 +1791,10 @@ static void raise_closed (const char *) Noreturn; pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " int r;\n"; + pr " size_t len;\n"; + "0"
'r' is not an int. It's a hive_value_h.
| RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -1861,6 +1875,7 @@ static void raise_closed (const char *) Noreturn; pr " for (int i = 0; r[i] != NULL; ++i) free (r[i]);\n"; pr " free (r);\n" | RLenType -> pr " rv = copy_type_len (len, t);\n" + | RLenValue -> pr " rv = copy_len (len);\n"
This call is wrong, and so is copy_len below. What happens to 'r'?
| RLenTypeVal -> pr " rv = copy_type_value (r, len, t);\n"; pr " free (r);\n" @@ -1983,6 +1998,18 @@ copy_type_len (size_t len, hive_type t) }
static value +copy_len (size_t len) +{ + CAMLparam0 (); + CAMLlocal2 (v, rv); + + rv = caml_alloc (1, 0); + v = Val_int (len); + Store_field (rv, 0, v); + CAMLreturn (rv); +}
This creates a tuple with a single element, which OCaml can't actually process. In any case you need to be passing two parameters here: the value and the len, and you need to make a 2-element tuple containing both of them.
+static value copy_type_value (const char *r, size_t len, hive_type t) { CAMLparam0 (); @@ -2172,6 +2199,7 @@ sub open { | RString | RStringList | RLenType + | RLenValue | RLenTypeVal | RInt32 | RInt64 -> () @@ -2246,6 +2274,7 @@ and generate_perl_prototype name style = | RString -> pr "$string = " | RStringList -> pr "@strings = " | RLenType -> pr "($type, $len) = " + | RLenValue -> pr "($len) = "
Just a single length? What about the value?
| RLenTypeVal -> pr "($type, $data) = " | RInt32 -> pr "$int32 = " | RInt64 -> pr "$int64 = " @@ -2469,6 +2498,7 @@ DESTROY (h) | RValueList | RStringList | RLenType + | RLenValue | RLenTypeVal -> pr "void\n" | RInt32 -> pr "SV *\n" | RInt64 -> pr "SV *\n" @@ -2641,6 +2671,20 @@ DESTROY (h) pr " PUSHs (sv_2mortal (newSViv (type)));\n"; pr " PUSHs (sv_2mortal (newSViv (len)));\n";
+ | RLenValue -> + pr "PREINIT:\n"; + pr " int r;\n";
int r? The return from the function is a hive_value_h.
+ pr " size_t len;\n"; + pr " PPCODE:\n"; + pr " r = hivex_%s (%s, &len);\n" + name (String.concat ", " c_params); + free_args (); + pr " if (r == 0)\n"; + pr " croak (\"%%s: \", \"%s\", strerror (errno));\n" + name; + pr " EXTEND (SP, 2);\n"; + pr " PUSHs (sv_2mortal (newSViv (len)));\n";
You extend the stack by two elements, then push one element. This would cause Perl to crash. In any case you need to push both the value (r) and the length. I can't remember in which order you need to push them -- take a look at other examples.
| RLenTypeVal -> pr "PREINIT:\n"; pr " char *r;\n"; @@ -2879,6 +2923,14 @@ put_len_type (size_t len, hive_type t) }
static PyObject * +put_len (size_t len) +{ + PyObject *r = PyTuple_New (1); + PyTuple_SetItem (r, 0, PyLong_FromLongLong ((long) len)); + return r; +}
Same problem in Python as in OCaml and Perl.
It's probably going to help if you make some small test programs in all languages, so you can check the returned values are marshalled properly.
+static PyObject * put_val_type (char *val, size_t len, hive_type t) { PyObject *r = PyTuple_New (2); @@ -2918,6 +2970,10 @@ put_val_type (char *val, size_t len, hive_type t) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " int r;\n";
As above.
+ pr " size_t len;\n"; + "0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -2942,6 +2998,7 @@ put_val_type (char *val, size_t len, hive_type t) let c_params = match fst style with | RLenType | RLenTypeVal -> c_params @ ["&t"; "&len"] + | RLenValue -> c_params @ ["&len"] | _ -> c_params in
List.iter ( @@ -3086,6 +3143,8 @@ put_val_type (char *val, size_t len, hive_type t) pr " free_strings (r);\n" | RLenType -> pr " py_r = put_len_type (len, t);\n" + | RLenValue -> + pr " py_r = put_len (len);\n"
As above.
| RLenTypeVal -> pr " py_r = put_val_type (r, len, t);\n"; pr " free (r);\n" @@ -3296,6 +3355,7 @@ get_values (VALUE valuesv, size_t *nr_values) | RString -> "string" | RStringList -> "list" | RLenType -> "hash" + | RLenValue -> "integer" | RLenTypeVal -> "hash" | RInt32 -> "integer" | RInt64 -> "integer" in @@ -3394,6 +3454,10 @@ get_values (VALUE valuesv, size_t *nr_values) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " int r;\n";
As above (Ruby).
+ pr " size_t len;\n"; + "0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -3418,6 +3482,7 @@ get_values (VALUE valuesv, size_t *nr_values) let c_params = match ret with | RLenType | RLenTypeVal -> c_params @ [["&t"; "&len"]] + | RLenValue -> c_params @ [["&len"]] | _ -> c_params in let c_params = List.concat c_params in
@@ -3499,6 +3564,10 @@ get_values (VALUE valuesv, size_t *nr_values) pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"type\")), INT2NUM (t));\n"; pr " return rv;\n" + | RLenValue -> + pr " VALUE rv = rb_hash_new ();\n"; + pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n"; + pr " return rv;\n"
As above.
| RLenTypeVal -> pr " VALUE rv = rb_hash_new ();\n"; pr " rb_hash_aset (rv, ID2SYM (rb_intern (\"len\")), INT2NUM (len));\n";
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/
On Wed, Oct 12, 2011 at 08:09:09PM -0700, Alex Nelson wrote:
Hi Rich,
Sorry for letting this one sit for a while.
I decided the appropriate way to make sure everything was correct was to just write some unit tests and include them for runs with 'make check' - a bit more than you said to do, but I think it makes sense. Unfortunately, an OCaml type error is biting me, and I'm hung on that for verifying the OCaml bindings.
In the attached patch, the test ocaml/t/hivex_120_rlenvalue.ml has an error arising from the type abbreviation (is that the right term here? Or is it "synonym"?) of "value" and "int". From what I can do at the OCaml prompt (pwd=.../hivex/ocaml), this is the confusion: # (2:Hivex.value);; Error: This expression has type int but an expression was expected of type Hivex.value
You can't just cast between types in OCaml since that would be unsafe (weak typing like C). For the purpose of this test, it seems acceptable to do an unsafe cast though: (Obj.magic 2 : Hivex.value) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 80 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
participants (2)
-
Alex Nelson -
Richard W.M. Jones