Libguestfs - Oct 2011 - [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 at 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/