[hivex][PATCH 2/8] generator: Add new return type to ABI: RLenValue
RLenValue is similar to RLenType, though with one less argument. This required adding additional conversion functions for several languages' bindings. Signed-off-by: Alex Nelson <ajnelson@cs.ucsc.edu> --- generator/generator.ml | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 78 insertions(+), 0 deletions(-) diff --git a/generator/generator.ml b/generator/generator.ml index def516f..7ece245 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 (* Returns offset and length of value. *) | RString (* Returns char* or NULL. *) | RStringList (* Returns char** or NULL. *) | RLenType (* See hivex_value_type. *) @@ -888,6 +889,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 " @@ -909,6 +911,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" @@ -1111,6 +1114,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 value handle. +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>). @@ -1622,6 +1629,7 @@ and generate_ocaml_prototype ?(is_external = false) name style = | RString -> pr "string" | RStringList -> pr "string array" | RLenType -> pr "hive_type * int" + | RLenValue -> pr "value * int" | RLenTypeVal -> pr "hive_type * string" | RInt32 -> pr "int32" | RInt64 -> pr "int64" @@ -1685,6 +1693,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_value (size_t, hive_value_h); 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; @@ -1707,6 +1716,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 @@ -1779,6 +1789,11 @@ static void raise_closed (const char *) Noreturn; pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " errno = 0;"; + pr " hive_value_h r;\n"; + pr " size_t len;\n"; + "0 && errno != 0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -1859,6 +1874,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_value (len, r);\n" | RLenTypeVal -> pr " rv = copy_type_value (r, len, t);\n"; pr " free (r);\n" @@ -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); +} + +static value copy_type_value (const char *r, size_t len, hive_type t) { CAMLparam0 (); @@ -2170,6 +2200,7 @@ sub open { | RString | RStringList | RLenType + | RLenValue | RLenTypeVal | RInt32 | RInt64 -> () @@ -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"; + | RLenTypeVal -> pr "PREINIT:\n"; pr " char *r;\n"; @@ -2877,6 +2926,15 @@ put_len_type (size_t len, hive_type t) } static PyObject * +put_len_val (size_t len, hive_value_h value) +{ + PyObject *r = PyTuple_New (2); + PyTuple_SetItem (r, 0, PyLong_FromLongLong ((long) len)); + PyTuple_SetItem (r, 1, PyLong_FromLongLong ((long) value)); + return r; +} + +static PyObject * put_val_type (char *val, size_t len, hive_type t) { PyObject *r = PyTuple_New (2); @@ -2916,6 +2974,11 @@ put_val_type (char *val, size_t len, hive_type t) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " errno = 0;\n"; + pr " int r;\n"; + pr " size_t len;\n"; + "0 && errno != 0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -2940,6 +3003,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 ( @@ -3084,6 +3148,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_val (len, r);\n" | RLenTypeVal -> pr " py_r = put_val_type (r, len, t);\n"; pr " free (r);\n" @@ -3294,6 +3360,7 @@ get_values (VALUE valuesv, size_t *nr_values) | RString -> "string" | RStringList -> "list" | RLenType -> "hash" + | RLenValue -> "integer" | RLenTypeVal -> "hash" | RInt32 -> "integer" | RInt64 -> "integer" in @@ -3392,6 +3459,11 @@ get_values (VALUE valuesv, size_t *nr_values) pr " size_t len;\n"; pr " hive_type t;\n"; "-1" + | RLenValue -> + pr " errno = 0;\n"; + pr " hive_value_h r;\n"; + pr " size_t len;\n"; + "0 && errno != 0" | RLenTypeVal -> pr " char *r;\n"; pr " size_t len;\n"; @@ -3416,6 +3488,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 @@ -3497,6 +3570,11 @@ 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 " rb_hash_aset (rv, ID2SYM (rb_intern (\"off\")), ULL2NUM (r));\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.4.4
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/
On Thu, Oct 20, 2011 at 12:37:54PM +0100, Richard W.M. Jones wrote:
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.
And while you're doing that, is there a way to avoid the hard-coded '297680'? I suggest two possibilities: create another test hive (derived from 'large') which will be specific to this test and we will never change in future. Since this won't change, we can go ahead and use hard-coded offsets. Or properly iterate through the hive using the standard APIs, to pick out the nodes we want to test. Second is probably better, but obviously more work. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
On Oct 20, 2011, at 04:46 , Richard W.M. Jones wrote:
On Thu, Oct 20, 2011 at 12:37:54PM +0100, Richard W.M. Jones wrote:
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.
And while you're doing that, is there a way to avoid the hard-coded '297680'?
I suggest two possibilities: create another test hive (derived from 'large') which will be specific to this test and we will never change in future. Since this won't change, we can go ahead and use hard-coded offsets. I see two options for the test hive.
The first, which I would prefer, is to use an openly available hive from a research disk image. For example, digitalcorpora.org has several Windows computer disk images immediately available for analysis. Any hive from there could be included as a sample in the images/ directory. Unfortunately, I don't know how well this would work with the GPL. Alternatively, there is the attached patch, which adds a not-too-creatively named hive generated from ''minimal'. (I didn't see a benefit to deriving it from 'large,' but it's easy enough to modify that.)
Or properly iterate through the hive using the standard APIs, to pick out the nodes we want to test.
Second is probably better, but obviously more work.
The second may be better if a "walk" function in the fashion of Python's os.walk could be used. However, there is no other API mechanism to check for the remotely stored value offsets, which the hard-coded offset tests verify. --Alex
On Sun, Dec 04, 2011 at 04:45:27PM -0800, Alex Nelson wrote:
On Oct 20, 2011, at 04:46 , Richard W.M. Jones wrote:
On Thu, Oct 20, 2011 at 12:37:54PM +0100, Richard W.M. Jones wrote:
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.
And while you're doing that, is there a way to avoid the hard-coded '297680'?
I suggest two possibilities: create another test hive (derived from 'large') which will be specific to this test and we will never change in future. Since this won't change, we can go ahead and use hard-coded offsets.
I see two options for the test hive.
The first, which I would prefer, is to use an openly available hive from a research disk image. For example, digitalcorpora.org has several Windows computer disk images immediately available for analysis. Any hive from there could be included as a sample in the images/ directory. Unfortunately, I don't know how well this would work with the GPL.
I checked this out, and it's not possible. Our test hive 'minimal' was specially constructed so it does not contain anything copyrightable. 'large' was constructed from 'minimal' by a mechanical process. Nevertheless Digital Corpora looks interesting and useful. I need to see if we can add tests (including for libguestfs) that would use that for people who had downloaded the corpus themselves.
Alternatively, there is the attached patch, which adds a not-too-creatively named hive generated from ''minimal'. (I didn't see a benefit to deriving it from 'large,' but it's easy enough to modify that.)
OK. The patch is the sort of thing I was suggesting. This is going to be combined with the revised patch series? [...] Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On Dec 5, 2011, at 00:45 , Richard W.M. Jones wrote:
On Sun, Dec 04, 2011 at 04:45:27PM -0800, Alex Nelson wrote:
On Oct 20, 2011, at 04:46 , Richard W.M. Jones wrote:
On Thu, Oct 20, 2011 at 12:37:54PM +0100, Richard W.M. Jones wrote:
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.
And while you're doing that, is there a way to avoid the hard-coded '297680'?
I suggest two possibilities: create another test hive (derived from 'large') which will be specific to this test and we will never change in future. Since this won't change, we can go ahead and use hard-coded offsets.
I see two options for the test hive.
The first, which I would prefer, is to use an openly available hive from a research disk image. For example, digitalcorpora.org has several Windows computer disk images immediately available for analysis. Any hive from there could be included as a sample in the images/ directory. Unfortunately, I don't know how well this would work with the GPL.
I checked this out, and it's not possible. Our test hive 'minimal' was specially constructed so it does not contain anything copyrightable. 'large' was constructed from 'minimal' by a mechanical process. Thought so.
Nevertheless Digital Corpora looks interesting and useful. I need to see if we can add tests (including for libguestfs) that would use that for people who had downloaded the corpus themselves.
Alternatively, there is the attached patch, which adds a not-too-creatively named hive generated from ''minimal'. (I didn't see a benefit to deriving it from 'large,' but it's easy enough to modify that.)
OK. The patch is the sort of thing I was suggesting. This is going to be combined with the revised patch series? Yes. Thanks!
--Alex
[...]
Rich.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
participants (2)
-
Alex Nelson -
Richard W.M. Jones