Peter Wu
2014-Aug-16 11:28 UTC
[Libguestfs] [hivex] [PATCH 0/6] Python fixes for node_set_value
Hi, This patch series is based on a prior patch[1], splitting off changes as requested and incorporating feedback from Richard Jones. It introduces type validation to avoid segmentation faults (instead, it reports an exception) and fixes handling of the bytes type in Python 3. Major changes since that series: - Drop newly introduced support for integer types for DWORD/QWORDS - Reject Unicode string types for Python 3 (and restore bytes support). - Adjusted tests due to new requirements. These changes were tested with the following combinations: - Arch Linux x86_64 - Python 2.7.8, 3.4.1 - Kubuntu 14.04 x86_64 - Python 2.7.6, 3.4.0 - Ubuntu 10.04 i686 - Python 2.6.5, 3.1.2. Note that Python 3.1.2 does not pass the tests (syntax error) unless you remove the use of Unicode literals: `sed 's/u"/"/' -i python/t/*.py` Support for Unicode literals was removed in 3.0 and restored in 3.3 (PEP-414) [1]: https://www.redhat.com/archives/libguestfs/2014-August/msg00051.html Peter Wu (6): python: use errors more specific than RuntimeError python: use PyErr_NoMemory python: check some types for get_value python: fix crash by validating key and value python: add heavier tests for setvalue generator: Fix mixed tabs/spaces configure.ac | 4 - generator/generator.ml | 570 +++++++++++++++++++++-------------------- python/t/200-write.py | 4 +- python/t/210-setvalue.py | 28 +- python/t/300-setvalue-types.py | 80 ++++++ 5 files changed, 386 insertions(+), 300 deletions(-) create mode 100644 python/t/300-setvalue-types.py -- 2.0.4
Peter Wu
2014-Aug-16 11:28 UTC
[Libguestfs] [PATCH 1/6] python: use errors more specific than RuntimeError
KeyError is documented as: "Raised when a mapping (dictionary) key is not found in the set of existing keys." TypeError is documented as: "Raised when an operation or function is applied to an object of inappropriate type." RuntimeError is documented as: "Raised when an error is detected that doesn’t fall in any of the other categories." Let's be more specific with exceptions. The exception all inherit from StandardError (and from Exception), but not from RuntimeError, so this might need some code changes (if users would catch this specific error). However, this behavior was not documented so it is perfectly fine to change it. --- generator/generator.ml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/generator/generator.ml b/generator/generator.ml index 0218b90..c65a3bd 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -2846,7 +2846,7 @@ get_value (PyObject *v, hive_set_value *ret) obj = PyDict_GetItemString (v, \"key\"); if (!obj) { - PyErr_SetString (PyExc_RuntimeError, \"no 'key' element in dictionary\"); + PyErr_SetString (PyExc_KeyError, \"no 'key' element in dictionary\"); return -1; } #ifdef HAVE_PYSTRING_ASSTRING @@ -2858,14 +2858,14 @@ get_value (PyObject *v, hive_set_value *ret) obj = PyDict_GetItemString (v, \"t\"); if (!obj) { - PyErr_SetString (PyExc_RuntimeError, \"no 't' element in dictionary\"); + PyErr_SetString (PyExc_KeyError, \"no 't' element in dictionary\"); return -1; } ret->t = PyLong_AsLong (obj); obj = PyDict_GetItemString (v, \"value\"); if (!obj) { - PyErr_SetString (PyExc_RuntimeError, \"no 'value' element in dictionary\"); + PyErr_SetString (PyExc_KeyError, \"no 'value' element in dictionary\"); return -1; } #ifdef HAVE_PYSTRING_ASSTRING -- 2.0.4
If malloc fails, just throw a NoMemory exception rather than showing "RuntimeError: Cannot allocate memory". --- generator/generator.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generator/generator.ml b/generator/generator.ml index c65a3bd..1c35da9 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -2905,7 +2905,7 @@ get_values (PyObject *v, py_set_values *ret) ret->nr_values = len; ret->values = malloc (len * sizeof (hive_set_value)); if (!ret->values) { - PyErr_SetString (PyExc_RuntimeError, strerror (errno)); + PyErr_NoMemory (); return -1; } -- 2.0.4
Peter Wu
2014-Aug-16 11:28 UTC
[Libguestfs] [PATCH 3/6] python: check some types for get_value
Recognise early that a value passed to node_get_value is not a dict rather than spitting out "no 'key' element in dictionary". Detect errors for invalid `t` elements instead of silently using `-1`. --- generator/generator.ml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/generator/generator.ml b/generator/generator.ml index 1c35da9..44a4d47 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -2844,6 +2844,11 @@ get_value (PyObject *v, hive_set_value *ret) PyObject *bytes; #endif + if (!PyDict_Check (v)) { + PyErr_SetString (PyExc_TypeError, \"expected dictionary type for value\"); + return -1; + } + obj = PyDict_GetItemString (v, \"key\"); if (!obj) { PyErr_SetString (PyExc_KeyError, \"no 'key' element in dictionary\"); @@ -2862,6 +2867,10 @@ get_value (PyObject *v, hive_set_value *ret) return -1; } ret->t = PyLong_AsLong (obj); + if (PyErr_Occurred ()) { + PyErr_SetString (PyExc_TypeError, \"expected int type for 't'\"); + return -1; + } obj = PyDict_GetItemString (v, \"value\"); if (!obj) { -- 2.0.4
Peter Wu
2014-Aug-16 11:28 UTC
[Libguestfs] [PATCH 4/6] python: fix crash by validating key and value
User-visible changes: no crashes if you try to pass a non-str type as key/value in Python 3. Exceptions are thrown rather than silently ignoring type violations (such as passing a Unicode string as value in Python 2). Until now, Python 2 keys and values were assumes to be bytes, in Python 3 these were seen as Unicode strings instead. Remove the compile-check for a deprecated Python 2 function (`PyString_AsString`) and check for byte types instead. For keys, accept both strings and bytes as the common case is to pass a ASCII string name. (The `\u20ac` Unicode character (Euro-sign) is apparently also valid as key name, verified with Python 3 and Windows 7). For values, be more conservative and reject all types except bytes. This better matches the C API and removes the burden of character conversion from the wrapper. Instead, callers should know the correct encoding and terminate strings. Modify tests to specify bytes instead of strings. (Otherwise Python 3 would break while Python 2 still passes.) --- configure.ac | 4 ---- generator/generator.ml | 41 +++++++++++++++++++++++++---------------- python/t/200-write.py | 4 ++-- python/t/210-setvalue.py | 28 ++++++++++------------------ 4 files changed, 37 insertions(+), 40 deletions(-) diff --git a/configure.ac b/configure.ac index f50daff..4566a32 100644 --- a/configure.ac +++ b/configure.ac @@ -345,10 +345,6 @@ AS_IF([test "x$enable_python" != "xno"], [AC_DEFINE([HAVE_PYCAPSULE_NEW],1, [Found PyCapsule_New in libpython])], [],[$PYTHON_BLDLIBRARY]) - AC_CHECK_LIB([c],[PyString_AsString], - [AC_DEFINE([HAVE_PYSTRING_ASSTRING],1, - [Found PyString_AsString in libpython])], - [],[$PYTHON_BLDLIBRARY]) LIBS="$old_LIBS" fi diff --git a/generator/generator.ml b/generator/generator.ml index 44a4d47..3c7b5ab 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -2840,9 +2840,7 @@ static int get_value (PyObject *v, hive_set_value *ret) { PyObject *obj; -#ifndef HAVE_PYSTRING_ASSTRING PyObject *bytes; -#endif if (!PyDict_Check (v)) { PyErr_SetString (PyExc_TypeError, \"expected dictionary type for value\"); @@ -2854,12 +2852,20 @@ get_value (PyObject *v, hive_set_value *ret) PyErr_SetString (PyExc_KeyError, \"no 'key' element in dictionary\"); return -1; } -#ifdef HAVE_PYSTRING_ASSTRING - ret->key = PyString_AsString (obj); -#else - bytes = PyUnicode_AsUTF8String (obj); - ret->key = PyBytes_AS_STRING (bytes); -#endif + if (PyUnicode_Check (obj)) { + /* TODO: use PyUnicode_DecodeASCII or PyUnicode_AsUTF16String instead? */ + bytes = PyUnicode_AsUTF8String (obj); + if (bytes == NULL) { + PyErr_SetString (PyExc_ValueError, \"failed to decode 'key'\"); + return -1; + } + ret->key = PyBytes_AS_STRING (bytes); + } else if (PyBytes_Check (obj)) { + ret->key = PyBytes_AS_STRING (obj); + } else { + PyErr_SetString (PyExc_TypeError, \"expected bytes type for 'key'\"); + return -1; + } obj = PyDict_GetItemString (v, \"t\"); if (!obj) { @@ -2877,14 +2883,17 @@ get_value (PyObject *v, hive_set_value *ret) PyErr_SetString (PyExc_KeyError, \"no 'value' element in dictionary\"); return -1; } -#ifdef HAVE_PYSTRING_ASSTRING - ret->value = PyString_AsString (obj); - ret->len = PyString_Size (obj); -#else - bytes = PyUnicode_AsUTF8String (obj); - ret->value = PyBytes_AS_STRING (bytes); - ret->len = PyBytes_GET_SIZE (bytes); -#endif + /* Support bytes only. As the registry can use multiple character sets, reject + * Unicode str types and let the caller handle conversion to nul-terminated + * UTF-16-LE, ASCII, etc. as necessary. This means that 'x' and b'x' are valid + * in Python 2 (but not u'x') but that in Python 3, only b'x' is valid. */ + if (PyBytes_Check (obj)) { + ret->len = PyBytes_GET_SIZE (obj); + ret->value = PyBytes_AS_STRING (obj); + } else { + PyErr_SetString (PyExc_TypeError, \"expected bytes type for 'value'\"); + return -1; + } return 0; } diff --git a/python/t/200-write.py b/python/t/200-write.py index 0692f11..4759f82 100644 --- a/python/t/200-write.py +++ b/python/t/200-write.py @@ -37,7 +37,7 @@ b = h.node_get_child (root, "B") assert b values = [ - { "key": "Key1", "t": 3, "value": "ABC" }, - { "key": "Key2", "t": 3, "value": "DEF" } + { "key": "Key1", "t": 3, "value": b"ABC" }, + { "key": "Key2", "t": 3, "value": b"DEF" } ] h.node_set_values (b, values) diff --git a/python/t/210-setvalue.py b/python/t/210-setvalue.py index 8e7ed5c..495c55a 100644 --- a/python/t/210-setvalue.py +++ b/python/t/210-setvalue.py @@ -19,14 +19,6 @@ import sys import os import hivex -if sys.version >= '3': - import codecs - def b(x): - return codecs.encode(x) -else: - def b(x): - return x - srcdir = os.environ["srcdir"] if not srcdir: srcdir = "." @@ -44,23 +36,23 @@ B = h.node_get_child (root, "B") assert B values = [ - { "key": "Key1", "t": 3, "value": "ABC" }, - { "key": "Key2", "t": 3, "value": "DEF" } + { "key": "Key1", "t": 3, "value": b"ABC" }, + { "key": "Key2", "t": 3, "value": b"DEF" } ] h.node_set_values (B, values) -value1 = { "key": "Key3", "t": 3, "value": "GHI" } +value1 = { "key": "Key3", "t": 3, "value": b"GHI" } h.node_set_value (B, value1) -value1 = { "key": "Key1", "t": 3, "value": "JKL" } +value1 = { "key": "Key1", "t": 3, "value": b"JKL" } h.node_set_value (B, value1) val = h.node_get_value (B, "Key1") -t_data = h.value_value (val) -assert t_data[0] == 3 -assert t_data[1] == b("JKL") +val_t, val_value = h.value_value (val) +assert val_t == 3 +assert val_value == b"JKL" val = h.node_get_value (B, "Key3") -t_data = h.value_value (val) -assert t_data[0] == 3 -assert t_data[1] == b("GHI") +val_t, val_value = h.value_value (val) +assert val_t == 3 +assert val_value == b"GHI" -- 2.0.4
Peter Wu
2014-Aug-16 11:28 UTC
[Libguestfs] [PATCH 5/6] python: add heavier tests for setvalue
Ensure that invalid types are not silently ignored (and test to avoid segfaults). --- python/t/300-setvalue-types.py | 80 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 python/t/300-setvalue-types.py diff --git a/python/t/300-setvalue-types.py b/python/t/300-setvalue-types.py new file mode 100644 index 0000000..7f9b2bc --- /dev/null +++ b/python/t/300-setvalue-types.py @@ -0,0 +1,80 @@ +# Test various possible types for assignment to setvalue +# Copyright (C) 2014 Peter Wu <peter@lekensteyn.nl> +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + +import hivex +from hivex.hive_types import * +import os + +srcdir = "." +if "srcdir" in os.environ and os.environ["srcdir"]: + srcdir = os.environ["srcdir"] + +h = hivex.Hivex ("%s/../images/minimal" % srcdir, + write = True) + +def set_value (key = "test key", t = REG_BINARY, value = b"Val"): + global h + h.node_set_value (h.root (), { + "key": key, + "t": t, + "value": value + }) + +def test_pass (key = "test key", t = REG_BINARY, value = b"Val"): + global h + set_value (key, t, value) + val = h.node_get_value (h.root (), key) + ret_type, ret_value = h.value_value (val) + assert t == ret_type, \ + "expected type {0}, got {1}".format(t, ret_type) + assert value == ret_value, \ + "expected value {0}, got {1}".format(value, ret_value) + +def test_exception (exception_type, **kwargs): + try: + set_value (**kwargs) + raise AssertionError("expected {0}".format(exception_type)) + except exception_type: + pass + + +# Good weather tests +# Accept either bytes or unicode for ASCII string +# TODO: fix node_get_value to handle UTF-16 string in bytes +#test_pass (t = REG_BINARY, key = b"\x01\x02") +test_pass (t = REG_SZ, key = u"ASCII key") +# Try a byte with upper bit set +test_pass (t = REG_DWORD, value = b"\xaa\xbb\xcc") + + +# Bad weather tests +# Invalid 'key' type +test_exception (TypeError, key = 1) +test_exception (TypeError, key = 1) +# TODO: should non-ASCII strings be rejected? +#test_exception (ValueError, key = u"Euro: \u20ac") + +# Invalid 't' type +test_exception (TypeError, t = b"meh") + +# Invalid 'value' types +test_exception (TypeError, t = REG_BINARY, value = 1) +test_exception (TypeError, t = REG_DWORD, value = 1) +test_exception (TypeError, t = REG_SZ, value = 1) +test_exception (TypeError, t = REG_DWORD, value = None) +# Unicode strings should be rejected, bytes only! +test_exception (TypeError, t = REG_SZ, value = u"some text") -- 2.0.4
vim :set ts=8 :retab --- generator/generator.ml | 512 ++++++++++++++++++++++++------------------------- 1 file changed, 256 insertions(+), 256 deletions(-) diff --git a/generator/generator.ml b/generator/generator.ml index 3c7b5ab..be783ae 100755 --- a/generator/generator.ml +++ b/generator/generator.ml @@ -1069,10 +1069,10 @@ here. Often it's not documented at all. pr "\n"; if List.mem AUnusedFlags (snd style) then - pr "The flags parameter is unused. Always pass 0.\n\n"; + pr "The flags parameter is unused. Always pass 0.\n\n"; if List.mem ASetValues (snd style) then - pr "C<values> is an array of (key, value) pairs. There + pr "C<values> is an array of (key, value) pairs. There should be C<nr_values> elements in this array. Any existing values stored at the node are discarded, and their @@ -1080,7 +1080,7 @@ C<hive_value_h> handles become invalid. Thus you can remove all values stored at C<node> by passing C<nr_values = 0>.\n\n"; if List.mem ASetValue (snd style) then - pr "C<value> is a single (key, value) pair. + pr "C<value> is a single (key, value) pair. Existing C<hive_value_h> handles become invalid.\n\n"; @@ -1791,8 +1791,8 @@ static void raise_closed (const char *) Noreturn; | ASetValues -> pr " int nrvalues = Wosize_val (valuesv);\n"; pr " hive_set_value *values = HiveSetValues_val (valuesv);\n" - | ASetValue -> - pr " hive_set_value *val = HiveSetValue_val (valv);\n" + | ASetValue -> + pr " hive_set_value *val = HiveSetValue_val (valv);\n" ) (snd style); pr "\n"; @@ -1864,9 +1864,9 @@ static void raise_closed (const char *) Noreturn; | ASetValues -> pr " free (values);\n"; pr "\n"; - | ASetValue -> - pr " free (val);\n"; - pr "\n"; + | ASetValue -> + pr " free (val);\n"; + pr "\n"; ) (snd style); (* Check for errors. *) @@ -1903,7 +1903,7 @@ static void raise_closed (const char *) Noreturn; pr " free (r);\n" | RStringList -> pr " rv = caml_copy_string_array ((const char **) r);\n"; - pr " int i;\n"; + pr " int i;\n"; pr " for (i = 0; r[i] != NULL; ++i) free (r[i]);\n"; pr " free (r);\n" | RLenType -> pr " rv = copy_type_len (len, t);\n" @@ -2220,45 +2220,45 @@ sub open { * Therefore we don't generate prototypes for these two calls: *) if fst style <> RErrDispose && List.hd (snd style) = AHive then ( - let longdesc = replace_str longdesc "C<hivex_" "C<" in - pr "=item %s\n\n " name; - generate_perl_prototype name style; - pr "\n\n"; - pr "%s\n\n" longdesc; - - (match fst style with - | RErr - | RErrDispose - | RHive - | RString - | RStringList - | RLenType - | RLenValue - | RLenTypeVal - | RInt32 - | RInt64 -> () - | RSize -> + let longdesc = replace_str longdesc "C<hivex_" "C<" in + pr "=item %s\n\n " name; + generate_perl_prototype name style; + pr "\n\n"; + pr "%s\n\n" longdesc; + + (match fst style with + | RErr + | RErrDispose + | RHive + | RString + | RStringList + | RLenType + | RLenValue + | RLenTypeVal + | RInt32 + | RInt64 -> () + | RSize -> pr "\ This returns a size.\n\n" - | RNode -> - pr "\ + | RNode -> + pr "\ This returns a node handle.\n\n" - | RNodeNotFound -> - pr "\ + | RNodeNotFound -> + pr "\ This returns a node handle, or C<undef> if the node was not found.\n\n" - | RNodeList -> - pr "\ + | RNodeList -> + pr "\ This returns a list of node handles.\n\n" - | RValue -> - pr "\ + | RValue -> + pr "\ This returns a value handle.\n\n" - | RValueList -> - pr "\ + | RValueList -> + pr "\ This returns a list of value handles.\n\n" - ); + ); - if List.mem ASetValues (snd style) then - pr "C<@values> is an array of (keys, value) pairs. + if List.mem ASetValues (snd style) then + pr "C<@values> is an array of (keys, value) pairs. Each element should be a hashref containing C<key>, C<t> (type) and C<data>. @@ -2524,172 +2524,172 @@ DESTROY (h) fun (name, style, _, longdesc) -> (* The close and open calls are handled specially above. *) if fst style <> RErrDispose && List.hd (snd style) = AHive then ( - (match fst style with - | RErr -> pr "void\n" - | RErrDispose -> failwith "perl bindings cannot handle a call which disposes of the handle" - | RHive -> failwith "perl bindings cannot handle a call which returns a handle" - | RSize - | RNode - | RNodeNotFound - | RValue - | RString -> pr "SV *\n" - | RNodeList - | RValueList - | RStringList - | RLenType - | RLenValue - | RLenTypeVal -> pr "void\n" - | RInt32 -> pr "SV *\n" - | RInt64 -> pr "SV *\n" - ); - - (* Call and arguments. *) - let perl_params - filter_map (function - | AUnusedFlags -> None - | arg -> Some (name_of_argt arg)) (snd style) in - - let c_params - List.map (function - | AUnusedFlags -> "0" - | ASetValues -> "values.nr_values, values.values" - | arg -> name_of_argt arg) (snd style) in - - pr "%s (%s)\n" name (String.concat ", " perl_params); - iteri ( + (match fst style with + | RErr -> pr "void\n" + | RErrDispose -> failwith "perl bindings cannot handle a call which disposes of the handle" + | RHive -> failwith "perl bindings cannot handle a call which returns a handle" + | RSize + | RNode + | RNodeNotFound + | RValue + | RString -> pr "SV *\n" + | RNodeList + | RValueList + | RStringList + | RLenType + | RLenValue + | RLenTypeVal -> pr "void\n" + | RInt32 -> pr "SV *\n" + | RInt64 -> pr "SV *\n" + ); + + (* Call and arguments. *) + let perl_params + filter_map (function + | AUnusedFlags -> None + | arg -> Some (name_of_argt arg)) (snd style) in + + let c_params + List.map (function + | AUnusedFlags -> "0" + | ASetValues -> "values.nr_values, values.values" + | arg -> name_of_argt arg) (snd style) in + + pr "%s (%s)\n" name (String.concat ", " perl_params); + iteri ( fun i -> function | AHive -> - pr " hive_h *h;\n" - | ANode n - | AValue n -> - pr " int %s;\n" n - | AString n -> - pr " char *%s;\n" n + pr " hive_h *h;\n" + | ANode n + | AValue n -> + pr " int %s;\n" n + | AString n -> + pr " char *%s;\n" n | AStringNullable n -> - (* http://www.perlmonks.org/?node_id=554277 *) - pr " char *%s = SvOK(ST(%d)) ? SvPV_nolen(ST(%d)) : NULL;\n" n i i - | AOpenFlags -> - pr " int flags;\n" - | AUnusedFlags -> () - | ASetValues -> - pr " pl_set_values values = unpack_pl_set_values (ST(%d));\n" i - | ASetValue -> - pr " hive_set_value *val = unpack_set_value (ST(%d));\n" i - ) (snd style); - - let free_args () - List.iter ( - function - | ASetValues -> - pr " free (values.values);\n" - | ASetValue -> - pr " free (val);\n" - | AHive | ANode _ | AValue _ | AString _ | AStringNullable _ - | AOpenFlags | AUnusedFlags -> () - ) (snd style) - in - - (* Code. *) - (match fst style with - | RErr -> + (* http://www.perlmonks.org/?node_id=554277 *) + pr " char *%s = SvOK(ST(%d)) ? SvPV_nolen(ST(%d)) : NULL;\n" n i i + | AOpenFlags -> + pr " int flags;\n" + | AUnusedFlags -> () + | ASetValues -> + pr " pl_set_values values = unpack_pl_set_values (ST(%d));\n" i + | ASetValue -> + pr " hive_set_value *val = unpack_set_value (ST(%d));\n" i + ) (snd style); + + let free_args () + List.iter ( + function + | ASetValues -> + pr " free (values.values);\n" + | ASetValue -> + pr " free (val);\n" + | AHive | ANode _ | AValue _ | AString _ | AStringNullable _ + | AOpenFlags | AUnusedFlags -> () + ) (snd style) + in + + (* Code. *) + (match fst style with + | RErr -> pr "PREINIT:\n"; pr " int r;\n"; pr " PPCODE:\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == -1)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; + name; - | RErrDispose -> assert false - | RHive -> assert false + | RErrDispose -> assert false + | RHive -> assert false - | RSize - | RNode - | RValue -> + | RSize + | RNode + | RValue -> pr "PREINIT:\n"; - pr " /* hive_node_h = hive_value_h = size_t so we cheat\n"; - pr " here to simplify the generator */\n"; + pr " /* hive_node_h = hive_value_h = size_t so we cheat\n"; + pr " here to simplify the generator */\n"; pr " size_t r;\n"; pr " CODE:\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == 0)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; + name; pr " RETVAL = newSViv (r);\n"; pr " OUTPUT:\n"; pr " RETVAL\n" - | RNodeNotFound -> + | RNodeNotFound -> pr "PREINIT:\n"; pr " hive_node_h r;\n"; pr " CODE:\n"; - pr " errno = 0;\n"; + pr " errno = 0;\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == 0 && errno != 0)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; - pr " if (r == 0)\n"; + name; + pr " if (r == 0)\n"; pr " RETVAL = &PL_sv_undef;\n"; - pr " else\n"; + pr " else\n"; pr " RETVAL = newSViv (r);\n"; pr " OUTPUT:\n"; pr " RETVAL\n" - | RString -> + | RString -> pr "PREINIT:\n"; pr " char *r;\n"; pr " CODE:\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == NULL)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; + name; if f_len_exists name then pr " RETVAL = newSVpvn_utf8 (r, hivex_%s_len (%s), 1);\n" name (String.concat ", " c_params) else pr " RETVAL = newSVpv (r, 0);\n"; - pr " free (r);\n"; + pr " free (r);\n"; pr " OUTPUT:\n"; pr " RETVAL\n" - | RNodeList - | RValueList -> + | RNodeList + | RValueList -> pr "PREINIT:\n"; pr " size_t *r;\n"; pr " int i, n;\n"; pr " PPCODE:\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == NULL)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; + name; pr " for (n = 0; r[n] != 0; ++n) /**/;\n"; pr " EXTEND (SP, n);\n"; pr " for (i = 0; i < n; ++i)\n"; pr " PUSHs (sv_2mortal (newSViv (r[i])));\n"; pr " free (r);\n"; - | RStringList -> + | RStringList -> pr "PREINIT:\n"; pr " char **r;\n"; pr " int i, n;\n"; pr " PPCODE:\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == NULL)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; + name; pr " for (n = 0; r[n] != NULL; ++n) /**/;\n"; pr " EXTEND (SP, n);\n"; pr " for (i = 0; i < n; ++i) {\n"; @@ -2698,86 +2698,86 @@ DESTROY (h) pr " }\n"; pr " free (r);\n"; - | RLenType -> - pr "PREINIT:\n"; - pr " int r;\n"; - pr " size_t len;\n"; - pr " hive_type type;\n"; - pr " PPCODE:\n"; + | RLenType -> + pr "PREINIT:\n"; + pr " int r;\n"; + pr " size_t len;\n"; + pr " hive_type type;\n"; + pr " PPCODE:\n"; pr " r = hivex_%s (%s, &type, &len);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == -1)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; - pr " EXTEND (SP, 2);\n"; - 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"; + name; + pr " EXTEND (SP, 2);\n"; + 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 (); + 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"; - pr " size_t len;\n"; - pr " hive_type type;\n"; - pr " PPCODE:\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"; + pr " size_t len;\n"; + pr " hive_type type;\n"; + pr " PPCODE:\n"; pr " r = hivex_%s (%s, &type, &len);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == NULL)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; - pr " EXTEND (SP, 2);\n"; - pr " PUSHs (sv_2mortal (newSViv (type)));\n"; - pr " PUSHs (sv_2mortal (newSVpvn (r, len)));\n"; - pr " free (r);\n"; + name; + pr " EXTEND (SP, 2);\n"; + pr " PUSHs (sv_2mortal (newSViv (type)));\n"; + pr " PUSHs (sv_2mortal (newSVpvn (r, len)));\n"; + pr " free (r);\n"; - | RInt32 -> + | RInt32 -> pr "PREINIT:\n"; pr " int32_t r;\n"; pr " CODE:\n"; - pr " errno = 0;\n"; + pr " errno = 0;\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == -1 && errno != 0)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; + name; pr " RETVAL = newSViv (r);\n"; pr " OUTPUT:\n"; pr " RETVAL\n" - | RInt64 -> + | RInt64 -> pr "PREINIT:\n"; pr " int64_t r;\n"; pr " CODE:\n"; - pr " errno = 0;\n"; + pr " errno = 0;\n"; pr " r = hivex_%s (%s);\n" - name (String.concat ", " c_params); - free_args (); + name (String.concat ", " c_params); + free_args (); pr " if (r == -1 && errno != 0)\n"; pr " croak (\"%%s: %%s\", \"%s\", strerror (errno));\n" - name; + name; pr " RETVAL = my_newSVll (r);\n"; pr " OUTPUT:\n"; pr " RETVAL\n" - ); - pr "\n" + ); + pr "\n" ) ) functions @@ -3019,10 +3019,10 @@ put_val_type (char *val, size_t len, hive_type t) pr " PyObject *py_r;\n"; let error_code - match fst style with + match fst style with | RErr -> pr " int r;\n"; "-1" - | RErrDispose -> pr " int r;\n"; "-1" - | RHive -> pr " hive_h *r;\n"; "NULL" + | RErrDispose -> pr " int r;\n"; "-1" + | RHive -> pr " hive_h *r;\n"; "NULL" | RSize -> pr " size_t r;\n"; "0" | RNode -> pr " hive_node_h r;\n"; "0" | RNodeNotFound -> @@ -3060,11 +3060,11 @@ put_val_type (char *val, size_t len, hive_type t) (* Call and arguments. *) let c_params - List.map (function - | AUnusedFlags -> "0" - | ASetValues -> "values.nr_values, values.values" + List.map (function + | AUnusedFlags -> "0" + | ASetValues -> "values.nr_values, values.values" | ASetValue -> "&val" - | arg -> name_of_argt arg) (snd style) in + | arg -> name_of_argt arg) (snd style) in let c_params match fst style with | RLenType | RLenTypeVal -> c_params @ ["&t"; "&len"] @@ -3074,23 +3074,23 @@ put_val_type (char *val, size_t len, hive_type t) List.iter ( function | AHive -> - pr " hive_h *h;\n"; + pr " hive_h *h;\n"; pr " PyObject *py_h;\n" - | ANode n - | AValue n -> - pr " long %s;\n" n - | AString n + | ANode n + | AValue n -> + pr " long %s;\n" n + | AString n | AStringNullable n -> - pr " char *%s;\n" n - | AOpenFlags -> - pr " int flags;\n" - | AUnusedFlags -> () - | ASetValues -> - pr " py_set_values values;\n"; - pr " PyObject *py_values;\n" - | ASetValue -> - pr " hive_set_value val;\n"; - pr " PyObject *py_val;\n" + pr " char *%s;\n" n + | AOpenFlags -> + pr " int flags;\n" + | AUnusedFlags -> () + | ASetValues -> + pr " py_set_values values;\n"; + pr " PyObject *py_values;\n" + | ASetValue -> + pr " hive_set_value val;\n"; + pr " PyObject *py_val;\n" ) (snd style); pr "\n"; @@ -3100,19 +3100,19 @@ put_val_type (char *val, size_t len, hive_type t) List.iter ( function | AHive -> - pr "O" - | ANode n - | AValue n -> - pr "l" - | AString n -> - pr "s" + pr "O" + | ANode n + | AValue n -> + pr "l" + | AString n -> + pr "s" | AStringNullable n -> - pr "z" - | AOpenFlags -> - pr "i" - | AUnusedFlags -> () - | ASetValues - | ASetValue -> + pr "z" + | AOpenFlags -> + pr "i" + | AUnusedFlags -> () + | ASetValues + | ASetValue -> pr "O" ) (snd style); @@ -3121,19 +3121,19 @@ put_val_type (char *val, size_t len, hive_type t) List.iter ( function | AHive -> - pr ", &py_h" - | ANode n - | AValue n -> - pr ", &%s" n - | AString n + pr ", &py_h" + | ANode n + | AValue n -> + pr ", &%s" n + | AString n | AStringNullable n -> - pr ", &%s" n - | AOpenFlags -> - pr ", &flags" - | AUnusedFlags -> () - | ASetValues -> + pr ", &%s" n + | AOpenFlags -> + pr ", &flags" + | AUnusedFlags -> () + | ASetValues -> pr ", &py_values" - | ASetValue -> + | ASetValue -> pr ", &py_val" ) (snd style); @@ -3145,16 +3145,16 @@ put_val_type (char *val, size_t len, hive_type t) function | AHive -> pr " h = get_handle (py_h);\n" - | ANode _ - | AValue _ - | AString _ + | ANode _ + | AValue _ + | AString _ | AStringNullable _ - | AOpenFlags - | AUnusedFlags -> () - | ASetValues -> + | AOpenFlags + | AUnusedFlags -> () + | ASetValues -> pr " if (get_values (py_values, &values) == -1)\n"; pr " return NULL;\n" - | ASetValue -> + | ASetValue -> pr " if (get_value (py_val, &val) == -1)\n"; pr " return NULL;\n" ) (snd style); @@ -3165,12 +3165,12 @@ put_val_type (char *val, size_t len, hive_type t) (* Free up arguments. *) List.iter ( function - | AHive | ANode _ | AValue _ + | AHive | ANode _ | AValue _ | AString _ | AStringNullable _ - | AOpenFlags | AUnusedFlags -> () - | ASetValues -> - pr " free (values.values);\n" - | ASetValue -> () + | AOpenFlags | AUnusedFlags -> () + | ASetValues -> + pr " free (values.values);\n" + | ASetValue -> () ) (snd style); (* Check for errors from C library. *) @@ -3359,13 +3359,13 @@ class Hivex(object): fun arg -> pr ", "; match arg with - | AHive -> assert false + | AHive -> assert false | ANode n | AValue n | AString n | AStringNullable n -> pr "%s" n - | AOpenFlags + | AOpenFlags | AUnusedFlags -> assert false - | ASetValues -> pr "values" - | ASetValue -> pr "val" + | ASetValues -> pr "values" + | ASetValue -> pr "val" ) args; pr ")\n"; pr "\n" @@ -3419,9 +3419,9 @@ and generate_ruby_c () #define RSTRING_PTR(r) (RSTRING((r))->ptr) #endif -static VALUE m_hivex; /* hivex module */ -static VALUE c_hivex; /* hive_h handle */ -static VALUE e_Error; /* used for all errors */ +static VALUE m_hivex; /* hivex module */ +static VALUE c_hivex; /* hive_h handle */ +static VALUE e_Error; /* used for all errors */ static void ruby_hivex_free (void *hvp) @@ -3576,10 +3576,10 @@ get_values (VALUE valuesv, size_t *nr_values) pr "\n"; let error_code - match ret with + match ret with | RErr -> pr " int r;\n"; "-1" - | RErrDispose -> pr " int r;\n"; "-1" - | RHive -> pr " hive_h *r;\n"; "NULL" + | RErrDispose -> pr " int r;\n"; "-1" + | RHive -> pr " hive_h *r;\n"; "NULL" | RSize -> pr " size_t r;\n"; "0" | RNode -> pr " hive_node_h r;\n"; "0" | RNodeNotFound -> -- 2.0.4
Richard W.M. Jones
2014-Aug-18 10:22 UTC
Re: [Libguestfs] [hivex] [PATCH 0/6] Python fixes for node_set_value
On Sat, Aug 16, 2014 at 01:28:44PM +0200, Peter Wu wrote:> Hi, > > This patch series is based on a prior patch[1], splitting off changes as > requested and incorporating feedback from Richard Jones. It introduces type > validation to avoid segmentation faults (instead, it reports an exception) and > fixes handling of the bytes type in Python 3.I have pushed the patches upstream. Thanks for your contribution. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Reasonably Related Threads
- [hivex] Segfault for an integer value to node_set_value
- [PATCH] hivex: add hivex_set_value api call and ocaml/perl bindings, tests
- [PATCH 1/2] Add type checking, support integers as value
- [PATCH] hivex: add hivex_set_value api call and perl bindings, tests
- [PATCH] hivex: add hivex_set_value api call