* [PATCH v7 0/5] Introduce Python bindings for libfdt
@ 2017-02-22 4:33 Simon Glass
[not found] ` <20170222043340.17008-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-02-22 4:33 UTC (permalink / raw)
To: Devicetree Compiler
Cc: Benjamin Bimmermann, Ulrich Langenbach, David Gibson, Simon Glass
At present libfdt consists of only a C implementation. Many scripts are
written using Python so it useful to have Python bindings for libfdt.
Apparently this has never been attempted before, or if so I cannot find a
reference.
This series starts the process of adding this support, with just a
bare-bones set of methods.
The v7 series provides binding that can be used like this:
fdt = libfdt.Fdt(open(fname).read())
node = fdt.path_offset('/subnode@1')
print fdt.get_prop(node, 'compatible')
subnode = fdt.first_subnode(node, quiet=[libfdt.NOTFOUND])
while subnode > 0:
print fdt.get_name(subnode)
subnode = fdt.next_subnode(subnode, quiet=[libfdt.NOTFOUND])
This version includes a simple class for properties.
Changes in v7:
- Add QUIET_ALL to silence all exceptions
- Add a test for QUIET_ALL
Changes in v6:
- Use a tuple instead of list for the default quiert parameter
- Use a tuple instead of list for QUIET_NOTFOUND
- Use 'list' instead of 'tuple' for the comment in check_err_null()
- Return a bytearray from getprop()
- Adjust the Property constructor to accept the name and value
- Use uint8_t for pylibfdt_copy_value
- Adjust tests to avoid checking a hard-coded offset
- Use 0 instead of self.fdt.path_offset('/')
- Adjust the totalsize() test to compare against the file size
- Adjust test result processing to avoid using awk
- Update example to print the node value as an integer
- Update example to print the bytestring as well as the string
Changes in v5:
- Use a 'quiet' parameter instead of quiet versions of functions
- Add a Property object to hold a property's name and value
- Drop the data() and string() functions which are not needed now
- Rename pylibfdt_copy_data() tp pylibfdt_copy_value()
- Change order of libfdt.h inclusion to avoid #ifdef around libfdt macros
- Drop fdt_offset_ptr() and fdt_getprop_namelen() from the swig interface
- Use $(SWIG) to call swig from the Makefile
- Review function comments
- Adjust tests to match new swig bindings
- Use an interactive session to demonstrate pylibfdt
- Mention that more work remains
- Update commit message
- Drop #ifdef around fdt_get_header() macros
- Fix 'possible' typo
Changes in v4:
- Make the library less pythonic to avoid a shaky illusion
- Drop classes for Node and Prop, along with associated methods
- Include libfdt.h instead of repeating it
- Add support for fdt_getprop()
- Bring in all libfdt functions (but Python support is missing for many)
- Add full comments for Python methods
- Drop tests that are no-longer applicable
- Add a get for getprop()
- Add new patch to adjust libfdt.h to work with swig
Changes in v3:
- Make the library more pythonic
- Add classes for Node and Prop along with methods
- Add an exception class
- Use Python to generate exeptions instead of SWIG
- Add some more tests
Changes in v2:
- Add exceptions when functions return an error
- Correct Python naming to following PEP8
- Use a class to encapsulate the various methods
- Include fdt.h instead of redefining struct fdt_property
- Use bytearray to avoid the SWIG warning 454
- Add comments
- Update tests for new pylibfdt
- Add a few more tests to increase coverage
- Add details on how to obtain full help and code coverage
Simon Glass (5):
Add an initial Python library for libfdt
Add tests for pylibfdt
Mention pylibfdt in the documentation
Adjust libfdt.h to work with swig
Build pylibfdt as part of the normal build process
Makefile | 17 +-
README | 47 +++++
libfdt/libfdt.h | 5 +-
pylibfdt/.gitignore | 3 +
pylibfdt/Makefile.pylibfdt | 18 ++
pylibfdt/libfdt.swig | 477 +++++++++++++++++++++++++++++++++++++++++++++
pylibfdt/setup.py | 34 ++++
tests/pylibfdt_tests.py | 275 ++++++++++++++++++++++++++
tests/run_tests.sh | 16 +-
9 files changed, 889 insertions(+), 3 deletions(-)
create mode 100644 pylibfdt/.gitignore
create mode 100644 pylibfdt/Makefile.pylibfdt
create mode 100644 pylibfdt/libfdt.swig
create mode 100644 pylibfdt/setup.py
create mode 100644 tests/pylibfdt_tests.py
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v7 1/5] Add an initial Python library for libfdt
[not found] ` <20170222043340.17008-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2017-02-22 4:33 ` Simon Glass
[not found] ` <20170222043340.17008-2-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-22 4:33 ` [PATCH v7 2/5] Add tests for pylibfdt Simon Glass
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-02-22 4:33 UTC (permalink / raw)
To: Devicetree Compiler
Cc: Benjamin Bimmermann, Ulrich Langenbach, David Gibson, Simon Glass
Add Python bindings for a bare-bones set of libfdt functions. These allow
navigating the tree and reading node names and properties.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v7:
- Add QUIET_ALL to silence all exceptions
Changes in v6:
- Use a tuple instead of list for the default quiert parameter
- Use a tuple instead of list for QUIET_NOTFOUND
- Use 'list' instead of 'tuple' for the comment in check_err_null()
- Return a bytearray from getprop()
- Adjust the Property constructor to accept the name and value
- Use uint8_t for pylibfdt_copy_value
Changes in v5:
- Use a 'quiet' parameter instead of quiet versions of functions
- Add a Property object to hold a property's name and value
- Drop the data() and string() functions which are not needed now
- Rename pylibfdt_copy_data() tp pylibfdt_copy_value()
- Change order of libfdt.h inclusion to avoid #ifdef around libfdt macros
- Drop fdt_offset_ptr() and fdt_getprop_namelen() from the swig interface
- Use $(SWIG) to call swig from the Makefile
- Review function comments
Changes in v4:
- Make the library less pythonic to avoid a shaky illusion
- Drop classes for Node and Prop, along with associated methods
- Include libfdt.h instead of repeating it
- Add support for fdt_getprop()
- Bring in all libfdt functions (but Python support is missing for many)
- Add full comments for Python methods
Changes in v3:
- Make the library more pythonic
- Add classes for Node and Prop along with methods
- Add an exception class
- Use Python to generate exeptions instead of SWIG
Changes in v2:
- Add exceptions when functions return an error
- Correct Python naming to following PEP8
- Use a class to encapsulate the various methods
- Include fdt.h instead of redefining struct fdt_property
- Use bytearray to avoid the SWIG warning 454
- Add comments
Makefile | 1 +
pylibfdt/.gitignore | 3 +
pylibfdt/Makefile.pylibfdt | 18 ++
pylibfdt/libfdt.swig | 477 +++++++++++++++++++++++++++++++++++++++++++++
pylibfdt/setup.py | 34 ++++
5 files changed, 533 insertions(+)
create mode 100644 pylibfdt/.gitignore
create mode 100644 pylibfdt/Makefile.pylibfdt
create mode 100644 pylibfdt/libfdt.swig
create mode 100644 pylibfdt/setup.py
diff --git a/Makefile b/Makefile
index ce05eba..1c48210 100644
--- a/Makefile
+++ b/Makefile
@@ -22,6 +22,7 @@ CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
BISON = bison
LEX = flex
+SWIG = swig
INSTALL = /usr/bin/install
DESTDIR =
diff --git a/pylibfdt/.gitignore b/pylibfdt/.gitignore
new file mode 100644
index 0000000..5e8c5e3
--- /dev/null
+++ b/pylibfdt/.gitignore
@@ -0,0 +1,3 @@
+libfdt.py
+libfdt.pyc
+libfdt_wrap.c
diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
new file mode 100644
index 0000000..0c0b390
--- /dev/null
+++ b/pylibfdt/Makefile.pylibfdt
@@ -0,0 +1,18 @@
+# Makefile.pylibfdt
+#
+
+PYLIBFDT_srcs = $(addprefix $(LIBFDT_srcdir)/,$(LIBFDT_SRCS))
+WRAP = $(PYLIBFDT_objdir)/libfdt_wrap.c
+PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
+
+$(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
+ @$(VECHO) PYMOD $@
+ python $(PYLIBFDT_objdir)/setup.py "$(CPPFLAGS)" $^
+ mv _libfdt.so $(PYMODULE)
+
+$(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
+ @$(VECHO) SWIG $@
+ $(SWIG) -python -o $@ $<
+
+PYLIBFDT_cleanfiles = libfdt_wrap.c libfdt.py libfdt.pyc
+PYLIBFDT_CLEANFILES = $(addprefix $(PYLIBFDT_objdir)/,$(PYLIBFDT_cleanfiles))
diff --git a/pylibfdt/libfdt.swig b/pylibfdt/libfdt.swig
new file mode 100644
index 0000000..03de79e
--- /dev/null
+++ b/pylibfdt/libfdt.swig
@@ -0,0 +1,477 @@
+/*
+ * pylibfdt - Flat Device Tree manipulation in Python
+ * Copyright (C) 2017 Google, Inc.
+ * Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
+ *
+ * libfdt is dual licensed: you can use it either under the terms of
+ * the GPL, or the BSD license, at your option.
+ *
+ * a) This library 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 library 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 library; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ *
+ * Alternatively,
+ *
+ * b) Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer.
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+ * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+%module libfdt
+
+%{
+#define SWIG_FILE_WITH_INIT
+#include "libfdt.h"
+%}
+
+%pythoncode %{
+
+import struct
+
+_NUM_ERRORS = 18
+
+# Error codes, corresponding to FDT_ERR_... in libfdt.h
+(NOTFOUND,
+ EXISTS,
+ NOSPACE,
+ BADOFFSET,
+ BADPATH,
+ BADPHANDLE,
+ BADSTATE,
+ TRUNCATED,
+ BADMAGIC,
+ BADVERSION,
+ BADSTRUCTURE,
+ BADLAYOUT,
+ INTERNAL,
+ BADNCELLS,
+ BADVALUE,
+ BADOVERLAY,
+ NOPHANDLES) = range(1, _NUM_ERRORS)
+
+# Pass this as the 'quiet' parameter to return -ENOTFOUND on NOTFOUND errors,
+# instead of raising an exception.
+QUIET_NOTFOUND = (NOTFOUND,)
+
+# Pass this as the 'quiet' parameter to avoid exceptions altogether. All
+# functions passed this value will return an error instead of raising an
+# exception.
+QUIET_ALL = range(1, _NUM_ERRORS)
+
+
+class FdtException(Exception):
+ """An exception caused by an error such as one of the codes above"""
+ def __init__(self, err):
+ self.err = err
+
+ def __str__(self):
+ return 'pylibfdt error %d: %s' % (self.err, fdt_strerror(self.err))
+
+def fdt32_to_cpu(val):
+ """Convert a device-tree cell value into a native integer"""
+ return struct.unpack("=I", struct.pack(">I", val))[0]
+
+def strerror(fdt_err):
+ """Get the string for an error number
+
+ Args:
+ fdt_err: Error number (-ve)
+
+ Returns:
+ String containing the associated error
+ """
+ return fdt_strerror(fdt_err)
+
+def check_err(val, quiet=()):
+ """Raise an error if the return value is -ve
+
+ This is used to check for errors returned by libfdt C functions.
+
+ Args:
+ val: Return value from a libfdt function
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ val if val >= 0
+
+ Raises
+ FdtException if val < 0
+ """
+ if val < 0:
+ if -val not in quiet:
+ raise FdtException(val)
+ return val
+
+def check_err_null(val, quiet=()):
+ """Raise an error if the return value is NULL
+
+ This is used to check for a NULL return value from certain libfdt C
+ functions
+
+ Args:
+ val: Return value from a libfdt function
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ val if val is a list, None if not
+
+ Raises
+ FdtException if val indicates an error was reported and the error
+ is not in @quiet.
+ """
+ # Normally a list is returned which contains the data and its length.
+ # If we get just an integer error code, it means the function failed.
+ if not isinstance(val, list):
+ if -val not in quiet:
+ raise FdtException(val)
+ return val
+
+class Fdt:
+ """Device tree class, supporting all operations
+
+ The Fdt object is created is created from a device tree binary file,
+ e.g. with something like:
+
+ fdt = Fdt(open("filename.dtb").read())
+
+ Operations can then be performed using the methods in this class. Each
+ method xxx(args...) corresponds to a libfdt function fdt_xxx(fdt, args...).
+
+ All methods raise an FdtException if an error occurs. To avoid this
+ behaviour a 'quiet' parameter is provided for some functions. This
+ defaults to empty, but you can pass a list of errors that you expect.
+ If one of these errors occurs, the function will return an error number
+ (e.g. -NOTFOUND).
+ """
+ def __init__(self, data):
+ self._fdt = bytearray(data)
+ check_err(fdt_check_header(self._fdt));
+
+ def path_offset(self, path, quiet=()):
+ """Get the offset for a given path
+
+ Args:
+ path: Path to the required node, e.g. '/node@3/subnode@1'
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ Node offset
+
+ Raises
+ FdtException if the path is not valid or not found
+ """
+ return check_err(fdt_path_offset(self._fdt, path), quiet)
+
+ def first_property_offset(self, nodeoffset, quiet=()):
+ """Get the offset of the first property in a node offset
+
+ Args:
+ nodeoffset: Offset to the node to check
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ Offset of the first property
+
+ Raises
+ FdtException if the associated node has no properties, or some
+ other error occurred
+ """
+ return check_err(fdt_first_property_offset(self._fdt, nodeoffset),
+ quiet)
+
+ def next_property_offset(self, prop_offset, quiet=()):
+ """Get the next property in a node
+
+ Args:
+ prop_offset: Offset of the previous property
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ Offset of the next property
+
+ Raises:
+ FdtException if the associated node has no more properties, or
+ some other error occurred
+ """
+ return check_err(fdt_next_property_offset(self._fdt, prop_offset),
+ quiet)
+
+ def get_name(self, nodeoffset):
+ """Get the name of a node
+
+ Args:
+ nodeoffset: Offset of node to check
+
+ Returns:
+ Node name
+
+ Raises:
+ FdtException on error (e.g. nodeoffset is invalid)
+ """
+ return check_err_null(fdt_get_name(self._fdt, nodeoffset))[0]
+
+ def get_property_by_offset(self, prop_offset, quiet=()):
+ """Obtains a property that can be examined
+
+ Args:
+ prop_offset: Offset of property (e.g. from first_property_offset())
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ Property object, or None if not found
+
+ Raises:
+ FdtException on error (e.g. invalid prop_offset or device
+ tree format)
+ """
+ pdata = check_err_null(
+ fdt_get_property_by_offset(self._fdt, prop_offset), quiet)
+ if isinstance(pdata, (int)):
+ return pdata
+ name = fdt_string(self._fdt, fdt32_to_cpu(pdata[0].nameoff))
+ value = bytearray(fdt32_to_cpu(pdata[0].len))
+ pylibfdt_copy_value(value, pdata[0])
+ return Property(name, value)
+
+
+ def first_subnode(self, nodeoffset, quiet=()):
+ """Find the first subnode of a parent node
+
+ Args:
+ nodeoffset: Node offset of parent node
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ The offset of the first subnode, if any
+
+ Raises:
+ FdtException if no subnode found or other error occurs
+ """
+ return check_err(fdt_first_subnode(self._fdt, nodeoffset), quiet)
+
+ def next_subnode(self, nodeoffset, quiet=()):
+ """Find the next subnode
+
+ Args:
+ nodeoffset: Node offset of previous subnode
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ The offset of the next subnode, if any
+
+ Raises:
+ FdtException if no more subnode found or other error occurs
+ """
+ return check_err(fdt_next_subnode(self._fdt, nodeoffset), quiet)
+
+ def totalsize(self):
+ """Return the total size of the device tree
+
+ Returns:
+ Total tree size in bytes
+ """
+ return check_err(fdt_totalsize(self._fdt))
+
+ def off_dt_struct(self):
+ """Return the start of the device tree struct area
+
+ Returns:
+ Start offset of struct area
+ """
+ return check_err(fdt_off_dt_struct(self._fdt))
+
+ def pack(self, quiet=()):
+ """Pack the device tree to remove unused space
+
+ This adjusts the tree in place.
+
+ Args:
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Raises:
+ FdtException if any error occurs
+ """
+ return check_err(fdt_pack(self._fdt), quiet)
+
+ def delprop(self, nodeoffset, prop_name):
+ """Delete a property from a node
+
+ Args:
+ nodeoffset: Node offset containing property to delete
+ prop_name: Name of property to delete
+
+ Raises:
+ FdtError if the property does not exist, or another error occurs
+ """
+ return check_err(fdt_delprop(self._fdt, nodeoffset, prop_name))
+
+ def getprop(self, nodeoffset, prop_name, quiet=()):
+ """Get a property from a node
+
+ Args:
+ nodeoffset: Node offset containing property to get
+ prop_name: Name of property to get
+ quiet: Errors to ignore (empty to raise on all errors)
+
+ Returns:
+ Value of property as a bytearray, or -ve error number
+
+ Raises:
+ FdtError if any error occurs (e.g. the property is not found)
+ """
+ pdata = check_err_null(fdt_getprop(self._fdt, nodeoffset, prop_name),
+ quiet)
+ if isinstance(pdata, (int)):
+ return pdata
+ return bytearray(pdata[0])
+
+
+class Property:
+ """Holds a device tree property name and value.
+
+ This holds a copy of a property taken from the device tree. It does not
+ reference the device tree, so if anything changes in the device tree,
+ a Property object will remain valid.
+
+ Properties:
+ name: Property name
+ value: Proper value as a bytearray
+ """
+ def __init__(self, name, value):
+ self.name = name
+ self.value = value
+%}
+
+%rename(fdt_property) fdt_property_func;
+
+typedef int fdt32_t;
+
+%include "libfdt/fdt.h"
+
+%include "typemaps.i"
+
+/*
+ * Unfortunately the defintiion of pybuffer_mutable_binary() in my Python
+ * version appears to be broken:
+ * pylibfdt/libfdt_wrap.c: In function ‘_wrap_pylibfdt_copy_value’:
+ * pylibfdt/libfdt_wrap.c:3603:22: error: ‘size’ undeclared (first use in this
+ * function)
+ * arg2 = (size_t) (size/sizeof(char));
+ *
+ * This version works correctly.
+ */
+%define %mypybuffer_mutable_binary(TYPEMAP, SIZE)
+%typemap(in) (TYPEMAP, SIZE)(int res, Py_ssize_t size = 0, void *buf = 0)
+{
+ res = PyObject_AsWriteBuffer($input, &buf, &size);
+ if (res < 0) {
+ PyErr_Clear();
+ %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
+ }
+ $1 = ($1_ltype)buf;
+ $2 = ($2_ltype)(size1 / sizeof($*1_type));
+}
+%enddef
+
+/* This is used to copy property data into a bytearray */
+%mypybuffer_mutable_binary(uint8_t *str, size_t size);
+void pylibfdt_copy_value(uint8_t *str, size_t size,
+ const struct fdt_property *prop);
+
+/* Most functions don't change the device tree, so use a const void * */
+%typemap(in) (const void *) {
+ if (!PyByteArray_Check($input)) {
+ SWIG_exception_fail(SWIG_TypeError, "in method '" "$symname"
+ "', argument " "$argnum"" of type '" "$type""'");
+ }
+ $1 = (void *)PyByteArray_AsString($input);
+}
+
+/* Some functions do change the device tree, so use void * */
+%typemap(in) (void *) {
+ if (!PyByteArray_Check($input)) {
+ SWIG_exception_fail(SWIG_TypeError, "in method '" "$symname"
+ "', argument " "$argnum"" of type '" "$type""'");
+ }
+ $1 = PyByteArray_AsString($input);
+}
+
+%inline %{
+
+/**
+ * pylibfdt_copy_value() - Copy value from a property to the given buffer
+ *
+ * This is used by the Property class to place the contents of a property
+ * into a bytearray.
+ *
+ * @buf: Destination pointer (typically the start of the bytearray)
+ * @size: Number of bytes to copy (size of bytearray)
+ * @prop: Property to copy
+ */
+void pylibfdt_copy_value(uint8_t *buf, size_t size, const struct fdt_property *prop)
+{
+ memcpy(buf, prop + 1, size);
+}
+
+%}
+
+%apply int *OUTPUT { int *lenp };
+
+/* typemap used for fdt_getprop() */
+%typemap(out) (const void *) {
+ if (!$1)
+ $result = Py_None;
+ else
+ /* TODO(sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org): Can we avoid the 'arg4'? */
+ $result = Py_BuildValue("s#", $1, *arg4);
+}
+
+/* We have both struct fdt_property and a function fdt_property() */
+%warnfilter(302) fdt_property;
+
+/* These are macros in the header so have to be redefined here */
+int fdt_magic(const void *fdt);
+int fdt_totalsize(const void *fdt);
+int fdt_off_dt_struct(const void *fdt);
+int fdt_off_dt_strings(const void *fdt);
+int fdt_off_mem_rsvmap(const void *fdt);
+int fdt_version(const void *fdt);
+int fdt_last_comp_version(const void *fdt);
+int fdt_boot_cpuid_phys(const void *fdt);
+int fdt_size_dt_strings(const void *fdt);
+int fdt_size_dt_struct(const void *fdt);
+
+%include <../libfdt/libfdt.h>
diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py
new file mode 100644
index 0000000..8f8618e
--- /dev/null
+++ b/pylibfdt/setup.py
@@ -0,0 +1,34 @@
+#!/usr/bin/env python
+
+"""
+setup.py file for SWIG libfdt
+"""
+
+from distutils.core import setup, Extension
+import os
+import sys
+
+progname = sys.argv[0]
+cflags = sys.argv[1]
+files = sys.argv[2:]
+
+if cflags:
+ cflags = [flag for flag in cflags.split(' ') if flag]
+else:
+ cflags = None
+
+libfdt_module = Extension(
+ '_libfdt',
+ sources = files,
+ extra_compile_args = cflags
+)
+
+sys.argv = [progname, '--quiet', 'build_ext', '--inplace']
+
+setup (name = 'libfdt',
+ version = '0.1',
+ author = "SWIG Docs",
+ description = """Simple swig libfdt from docs""",
+ ext_modules = [libfdt_module],
+ py_modules = ["libfdt"],
+ )
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 2/5] Add tests for pylibfdt
[not found] ` <20170222043340.17008-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-22 4:33 ` [PATCH v7 1/5] Add an initial Python library " Simon Glass
@ 2017-02-22 4:33 ` Simon Glass
[not found] ` <20170222043340.17008-3-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-22 4:33 ` [PATCH v7 3/5] Mention pylibfdt in the documentation Simon Glass
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-02-22 4:33 UTC (permalink / raw)
To: Devicetree Compiler
Cc: Benjamin Bimmermann, Ulrich Langenbach, David Gibson, Simon Glass
Add a set of tests to cover the functionality in pylibfdt.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v7:
- Add a test for QUIET_ALL
Changes in v6:
- Adjust tests to avoid checking a hard-coded offset
- Use 0 instead of self.fdt.path_offset('/')
- Adjust the totalsize() test to compare against the file size
- Adjust test result processing to avoid using awk
Changes in v5:
- Adjust tests to match new swig bindings
Changes in v4:
- Drop tests that are no-longer applicable
- Add a get for getprop()
Changes in v3:
- Add some more tests
Changes in v2:
- Update tests for new pylibfdt
- Add a few more tests to increase coverage
tests/pylibfdt_tests.py | 275 ++++++++++++++++++++++++++++++++++++++++++++++++
tests/run_tests.sh | 16 ++-
2 files changed, 290 insertions(+), 1 deletion(-)
create mode 100644 tests/pylibfdt_tests.py
diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
new file mode 100644
index 0000000..796684a
--- /dev/null
+++ b/tests/pylibfdt_tests.py
@@ -0,0 +1,275 @@
+# pylibfdt - Tests for Flat Device Tree manipulation in Python
+# Copyright (C) 2017 Google, Inc.
+# Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
+#
+# libfdt is dual licensed: you can use it either under the terms of
+# the GPL, or the BSD license, at your option.
+#
+# a) This library 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 library 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 library; if not, write to the Free
+# Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+# MA 02110-1301 USA
+#
+# Alternatively,
+#
+# b) Redistribution and use in source and binary forms, with or
+# without modification, are permitted provided that the following
+# conditions are met:
+#
+# 1. Redistributions of source code must retain the above
+# copyright notice, this list of conditions and the following
+# disclaimer.
+# 2. Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following
+# disclaimer in the documentation and/or other materials
+# provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+# CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+# INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+# NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+# OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
+# EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+
+import sys
+import unittest
+
+sys.path.append('../pylibfdt')
+import libfdt
+from libfdt import FdtException, QUIET_NOTFOUND, QUIET_ALL
+
+# Offsets of properties in the root node
+ROOT_PROPS = (8, 32, 48, 68, 92, 108)
+
+def get_err(err_code):
+ """Convert an error code into an error message
+
+ Args:
+ err_code: Error code value (FDT_ERR_...)
+
+ Returns:
+ String error code
+ """
+ return 'pylibfdt error %d: %s' % (-err_code, libfdt.fdt_strerror(-err_code))
+
+def _ReadFdt(fname):
+ """Read a device tree file into an Fdt object, ready for use
+
+ Args:
+ fname: Filename to read from
+
+ Returns:
+ Fdt bytearray suitable for passing to libfdt functions
+ """
+ return libfdt.Fdt(open(fname).read())
+
+class PyLibfdtTests(unittest.TestCase):
+ """Test class for pylibfdt
+
+ Properties:
+ fdt: Device tree file used for testing
+ """
+
+ def setUp(self):
+ """Read in the device tree we use for testing"""
+ self.fdt = _ReadFdt('test_tree1.dtb')
+
+ def GetPropList(self, node_path):
+ """Read a list of properties from a node
+
+ Args:
+ node_path: Full path to node, e.g. '/subnode@1/subsubnode'
+
+ Returns:
+ List of property names for that node, e.g. ['compatible', 'reg']
+ """
+ prop_list = []
+ node = self.fdt.path_offset(node_path)
+ poffset = self.fdt.first_property_offset(node, QUIET_NOTFOUND)
+ while poffset > 0:
+ prop = self.fdt.get_property_by_offset(poffset)
+ prop_list.append(prop.name)
+ poffset = self.fdt.next_property_offset(poffset, QUIET_NOTFOUND)
+ return prop_list
+
+ def testImport(self):
+ """Check that we can import the library correctly"""
+ self.assertEquals(type(libfdt), type(sys))
+
+ def testBadFdt(self):
+ """Check that a filename provided accidentally is not accepted"""
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADMAGIC)):
+ fdt = libfdt.Fdt('a string')
+
+ def testPathOffset(self):
+ """Check that we can find the offset of a node"""
+ self.assertEquals(self.fdt.path_offset('/'), 0)
+ self.assertTrue(self.fdt.path_offset('/subnode@1') > 0)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
+ self.fdt.path_offset('/wibble')
+ self.assertEquals(self.fdt.path_offset('/wibble', QUIET_NOTFOUND),
+ -libfdt.NOTFOUND)
+
+ def testPropertyOffset(self):
+ """Walk through all the properties in the root node"""
+ self.assertEquals(self.fdt.first_property_offset(0), ROOT_PROPS[0])
+ for pos in range(len(ROOT_PROPS) - 1):
+ self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[pos]),
+ ROOT_PROPS[pos + 1])
+ self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[-1],
+ QUIET_NOTFOUND),
+ -libfdt.NOTFOUND)
+
+ def testPropertyOffsetExceptions(self):
+ """Check that exceptions are raised as expected"""
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.next_property_offset(107)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.first_property_offset(107, QUIET_NOTFOUND)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.next_property_offset(107, QUIET_NOTFOUND)
+
+ node = self.fdt.path_offset('/subnode@1/ss1')
+ self.assertEquals(self.fdt.first_property_offset(node, QUIET_NOTFOUND),
+ -libfdt.NOTFOUND)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
+ self.fdt.first_property_offset(node)
+
+ def testGetName(self):
+ """Check that we can get the name of a node"""
+ self.assertEquals(self.fdt.get_name(0), '')
+ node = self.fdt.path_offset('/subnode@1/subsubnode')
+ self.assertEquals(self.fdt.get_name(node), 'subsubnode')
+
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.get_name(-2)
+
+ def testGetPropertyByOffset(self):
+ """Check that we can read the name and contents of a property"""
+ root = 0
+ poffset = self.fdt.first_property_offset(root)
+ prop = self.fdt.get_property_by_offset(poffset)
+ self.assertEquals(prop.name, 'compatible')
+ self.assertEquals(prop.value, 'test_tree1\0')
+
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.get_property_by_offset(-2)
+ self.assertEquals(
+ -libfdt.BADOFFSET,
+ self.fdt.get_property_by_offset(-2, [libfdt.BADOFFSET]))
+
+ def testGetProp(self):
+ """Check that we can read the contents of a property by name"""
+ root = self.fdt.path_offset('/')
+ value = self.fdt.getprop(root, "compatible")
+ self.assertEquals(value, 'test_tree1\0')
+ self.assertEquals(-libfdt.NOTFOUND, self.fdt.getprop(root, 'missing',
+ QUIET_NOTFOUND))
+
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
+ self.fdt.getprop(root, 'missing')
+
+ node = self.fdt.path_offset('/subnode@1/subsubnode')
+ value = self.fdt.getprop(node, "compatible")
+ self.assertEquals(value, 'subsubnode1\0subsubnode\0')
+
+ def testStrError(self):
+ """Check that we can get an error string"""
+ self.assertEquals(libfdt.strerror(-libfdt.NOTFOUND),
+ 'FDT_ERR_NOTFOUND')
+
+ def testFirstNextSubnodeOffset(self):
+ """Check that we can walk through subnodes"""
+ node_list = []
+ node = self.fdt.first_subnode(0, QUIET_NOTFOUND)
+ while node >= 0:
+ node_list.append(self.fdt.get_name(node))
+ node = self.fdt.next_subnode(node, QUIET_NOTFOUND)
+ self.assertEquals(node_list, ['subnode@1', 'subnode@2'])
+
+ def testFirstNextSubnodeOffsetExceptions(self):
+ """Check except handling for first/next subnode functions"""
+ node = self.fdt.path_offset('/subnode@1/subsubnode', QUIET_NOTFOUND)
+ self.assertEquals(self.fdt.first_subnode(node, QUIET_NOTFOUND),
+ -libfdt.NOTFOUND)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
+ self.fdt.first_subnode(node)
+
+ node = self.fdt.path_offset('/subnode@1/ss1', QUIET_NOTFOUND)
+ self.assertEquals(self.fdt.next_subnode(node, QUIET_NOTFOUND),
+ -libfdt.NOTFOUND)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
+ self.fdt.next_subnode(node)
+
+ def testDeleteProperty(self):
+ """Test that we can delete a property"""
+ node_name = '/subnode@1'
+ self.assertEquals(self.GetPropList(node_name),
+ ['compatible', 'reg', 'prop-int'])
+ node = self.fdt.path_offset('/%s' % node_name)
+ self.assertEquals(self.fdt.delprop(node, 'reg'), 0)
+ self.assertEquals(self.GetPropList(node_name),
+ ['compatible', 'prop-int'])
+
+ def testHeader(self):
+ """Test that we can access the header values"""
+ self.assertEquals(self.fdt.totalsize(), len(self.fdt._fdt))
+ self.assertEquals(self.fdt.off_dt_struct(), 88)
+
+ def testPack(self):
+ """Test that we can pack the tree after deleting something"""
+ self.assertEquals(self.fdt.totalsize(), 693)
+ node = self.fdt.path_offset('/subnode@2', QUIET_NOTFOUND)
+ self.assertEquals(self.fdt.delprop(node, 'prop-int'), 0)
+ self.assertEquals(self.fdt.totalsize(), 693)
+ self.assertEquals(self.fdt.pack(), 0)
+ self.assertEquals(self.fdt.totalsize(), 677)
+
+ def testBadPropertyOffset(self):
+ """Test that bad property offsets are detected"""
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.get_property_by_offset(13)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.first_property_offset(3)
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
+ self.fdt.next_property_offset(3)
+
+ def testBadPathOffset(self):
+ """Test that bad path names are detected"""
+ with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADPATH)):
+ self.fdt.path_offset('not-present')
+
+ def testEndian(self):
+ """Check that we can convert from FDT (big endian) to native endian"""
+ self.assertEquals(libfdt.fdt32_to_cpu(0x10000000), 0x10)
+
+ def testQuietAll(self):
+ """Check that exceptions can be masked by QUIET_ALL"""
+ self.assertEquals(-libfdt.NOTFOUND,
+ self.fdt.path_offset('/missing', QUIET_ALL))
+ self.assertEquals(-libfdt.BADOFFSET,
+ self.fdt.get_property_by_offset(13, QUIET_ALL))
+ self.assertEquals(-libfdt.BADPATH,
+ self.fdt.path_offset('missing', QUIET_ALL))
+
+
+if __name__ == "__main__":
+ unittest.main()
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index ed489db..76d20d8 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -769,6 +769,17 @@ fdtdump_tests () {
run_fdtdump_test fdtdump.dts
}
+pylibfdt_tests () {
+ TMP=/tmp/tests.stderr.$$
+ python pylibfdt_tests.py 2> ${TMP}
+ result=$(head -1 ${TMP})
+
+ # Extract the test results and add them to our totals
+ tot_fail=$((tot_fail + $(echo $result | tr -d '\n.' | wc -c)))
+ tot_pass=$((tot_pass + $(echo $result | tr -d '\nF' | wc -c)))
+ tot_tests=$((tot_tests + $(echo $result | tr -d '\n' | wc -c)))
+}
+
while getopts "vt:me" ARG ; do
case $ARG in
"v")
@@ -787,7 +798,7 @@ while getopts "vt:me" ARG ; do
done
if [ -z "$TESTSETS" ]; then
- TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump"
+ TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump pylibfdt"
fi
# Make sure we don't have stale blobs lying around
@@ -816,6 +827,9 @@ for set in $TESTSETS; do
"fdtdump")
fdtdump_tests
;;
+ "pylibfdt")
+ pylibfdt_tests
+ ;;
esac
done
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 3/5] Mention pylibfdt in the documentation
[not found] ` <20170222043340.17008-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-22 4:33 ` [PATCH v7 1/5] Add an initial Python library " Simon Glass
2017-02-22 4:33 ` [PATCH v7 2/5] Add tests for pylibfdt Simon Glass
@ 2017-02-22 4:33 ` Simon Glass
[not found] ` <20170222043340.17008-4-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-22 4:33 ` [PATCH v7 4/5] Adjust libfdt.h to work with swig Simon Glass
2017-02-22 4:33 ` [PATCH v7 5/5] Build pylibfdt as part of the normal build process Simon Glass
4 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-02-22 4:33 UTC (permalink / raw)
To: Devicetree Compiler
Cc: Benjamin Bimmermann, Ulrich Langenbach, David Gibson, Simon Glass
Add a note about pylibfdt in the README.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v7: None
Changes in v6:
- Update example to print the node value as an integer
- Update example to print the bytestring as well as the string
Changes in v5:
- Use an interactive session to demonstrate pylibfdt
- Mention that more work remains
Changes in v4: None
Changes in v3: None
Changes in v2:
- Add details on how to obtain full help and code coverage
README | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/README b/README
index f92008f..96d8486 100644
--- a/README
+++ b/README
@@ -7,6 +7,53 @@ DTC and LIBFDT are maintained by:
David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>
+
+Python library
+--------------
+
+A Python library is also available. To build this you will need to install
+swig and Python development files. On Debian distributions:
+
+ sudo apt-get install swig python-dev
+
+The library provides an Fdt class which you can use like this:
+
+$ PYTHONPATH=../pylibfdt python
+>>> import libfdt
+>>> fdt = libfdt.Fdt(open('test_tree1.dtb').read())
+>>> node = fdt.path_offset('/subnode@1')
+>>> print node
+124
+>>> prop_offset = fdt.first_property_offset(node)
+>>> prop = fdt.get_property_by_offset(prop_offset)
+>>> print '%s=%r' % (prop.name, prop.value)
+compatible=bytearray(b'subnode1\x00')
+>>> print '%s=%s' % (prop.name, prop.value)
+compatible=subnode1
+>>> node2 = fdt.path_offset('/')
+>>> print fdt.getprop(node2, 'compatible')
+test_tree1
+
+You will find tests in tests/pylibfdt_tests.py showing how to use each
+method. Help is available using the Python help command, e.g.:
+
+ $ cd pylibfdt
+ $ python -c "import libfdt; help(libfdt)"
+
+If you add new features, please check code coverage:
+
+ $ sudo apt-get install python-pip python-pytest
+ $ sudo pip install coverage
+ $ cd tests
+ $ coverage run pylibfdt_tests.py
+ $ coverage html
+ # Open 'htmlcov/index.html' in your browser
+
+
+More work remains to support all of libfdt, including access to numeric
+values.
+
+
Mailing list
------------
The following list is for discussion about dtc and libfdt implementation
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 4/5] Adjust libfdt.h to work with swig
[not found] ` <20170222043340.17008-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (2 preceding siblings ...)
2017-02-22 4:33 ` [PATCH v7 3/5] Mention pylibfdt in the documentation Simon Glass
@ 2017-02-22 4:33 ` Simon Glass
[not found] ` <20170222043340.17008-5-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-22 4:33 ` [PATCH v7 5/5] Build pylibfdt as part of the normal build process Simon Glass
4 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-02-22 4:33 UTC (permalink / raw)
To: Devicetree Compiler
Cc: Benjamin Bimmermann, Ulrich Langenbach, David Gibson, Simon Glass
There are a few places where libfdt.h cannot be used as is with swig:
- macros like fdt_totalsize() have to be defined as C declarations
- fdt_offset_ptr() and fdt_getprop_namelen() need special treatment due to
a TODO in the wrapper for fdt_getprop(). However they are not useful to
Python so can be removed
Add #ifdefs to work around these problem.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v7: None
Changes in v6: None
Changes in v5:
- Update commit message
- Drop #ifdef around fdt_get_header() macros
Changes in v4:
- Add new patch to adjust libfdt.h to work with swig
Changes in v3: None
Changes in v2: None
libfdt/libfdt.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index c69e918..2aa8ff6 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -143,7 +143,9 @@
/* Low-level functions (you probably don't need these) */
/**********************************************************************/
+#ifndef SWIG /* This functions are not useful in Python */
const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int checklen);
+#endif
static inline void *fdt_offset_ptr_w(void *fdt, int offset, int checklen)
{
return (void *)(uintptr_t)fdt_offset_ptr(fdt, offset, checklen);
@@ -210,7 +212,6 @@ int fdt_next_subnode(const void *fdt, int offset);
/**********************************************************************/
/* General functions */
/**********************************************************************/
-
#define fdt_get_header(fdt, field) \
(fdt32_to_cpu(((const struct fdt_header *)(fdt))->field))
#define fdt_magic(fdt) (fdt_get_header(fdt, magic))
@@ -638,8 +639,10 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
* Identical to fdt_getprop(), but only examine the first namelen
* characters of name for matching the property name.
*/
+#ifndef SWIG /* Not available in Python */
const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
const char *name, int namelen, int *lenp);
+#endif
static inline void *fdt_getprop_namelen_w(void *fdt, int nodeoffset,
const char *name, int namelen,
int *lenp)
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 5/5] Build pylibfdt as part of the normal build process
[not found] ` <20170222043340.17008-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
` (3 preceding siblings ...)
2017-02-22 4:33 ` [PATCH v7 4/5] Adjust libfdt.h to work with swig Simon Glass
@ 2017-02-22 4:33 ` Simon Glass
[not found] ` <20170222043340.17008-6-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
4 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-02-22 4:33 UTC (permalink / raw)
To: Devicetree Compiler
Cc: Benjamin Bimmermann, Ulrich Langenbach, David Gibson, Simon Glass
Possibly this needs to be made optional. For now just hook it up.
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v7: None
Changes in v6: None
Changes in v5:
- Fix 'possible' typo
Changes in v4: None
Changes in v3: None
Changes in v2: None
Makefile | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 1c48210..4adba10 100644
--- a/Makefile
+++ b/Makefile
@@ -116,7 +116,7 @@ BIN += fdtput
SCRIPTS = dtdiff
-all: $(BIN) libfdt
+all: $(BIN) libfdt pylibfdt
ifneq ($(DEPTARGETS),)
@@ -203,6 +203,19 @@ dist:
cat ../dtc-$(dtc_version).tar | \
gzip -9 > ../dtc-$(dtc_version).tar.gz
+
+#
+# Rules for pylibfdt
+#
+PYLIBFDT_srcdir = pylibfdt
+PYLIBFDT_objdir = pylibfdt
+
+include $(PYLIBFDT_srcdir)/Makefile.pylibfdt
+
+.PHONY: pylibfdt
+pylibfdt: $(PYLIBFDT_objdir)/_libfdt.so
+
+
#
# Release signing and uploading
# This is for maintainer convenience, don't try this at home.
@@ -247,6 +260,7 @@ STD_CLEANFILES = *~ *.o *.$(SHAREDLIB_EXT) *.d *.a *.i *.s core a.out vgcore.* \
clean: libfdt_clean tests_clean
@$(VECHO) CLEAN
rm -f $(STD_CLEANFILES)
+ rm -f $(PYLIBFDT_CLEANFILES)
rm -f $(VERSION_FILE)
rm -f $(BIN)
rm -f dtc-*.tar dtc-*.tar.sign dtc-*.tar.asc
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v7 1/5] Add an initial Python library for libfdt
[not found] ` <20170222043340.17008-2-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2017-02-24 2:43 ` David Gibson
[not found] ` <20170224024317.GI17615-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: David Gibson @ 2017-02-24 2:43 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
[-- Attachment #1: Type: text/plain, Size: 23672 bytes --]
On Tue, Feb 21, 2017 at 09:33:36PM -0700, Simon Glass wrote:
> Add Python bindings for a bare-bones set of libfdt functions. These allow
> navigating the tree and reading node names and properties.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>
> Changes in v7:
> - Add QUIET_ALL to silence all exceptions
>
> Changes in v6:
> - Use a tuple instead of list for the default quiert parameter
> - Use a tuple instead of list for QUIET_NOTFOUND
> - Use 'list' instead of 'tuple' for the comment in check_err_null()
> - Return a bytearray from getprop()
> - Adjust the Property constructor to accept the name and value
> - Use uint8_t for pylibfdt_copy_value
>
> Changes in v5:
> - Use a 'quiet' parameter instead of quiet versions of functions
> - Add a Property object to hold a property's name and value
> - Drop the data() and string() functions which are not needed now
> - Rename pylibfdt_copy_data() tp pylibfdt_copy_value()
> - Change order of libfdt.h inclusion to avoid #ifdef around libfdt macros
> - Drop fdt_offset_ptr() and fdt_getprop_namelen() from the swig interface
> - Use $(SWIG) to call swig from the Makefile
> - Review function comments
>
> Changes in v4:
> - Make the library less pythonic to avoid a shaky illusion
> - Drop classes for Node and Prop, along with associated methods
> - Include libfdt.h instead of repeating it
> - Add support for fdt_getprop()
> - Bring in all libfdt functions (but Python support is missing for many)
> - Add full comments for Python methods
>
> Changes in v3:
> - Make the library more pythonic
> - Add classes for Node and Prop along with methods
> - Add an exception class
> - Use Python to generate exeptions instead of SWIG
>
> Changes in v2:
> - Add exceptions when functions return an error
> - Correct Python naming to following PEP8
> - Use a class to encapsulate the various methods
> - Include fdt.h instead of redefining struct fdt_property
> - Use bytearray to avoid the SWIG warning 454
> - Add comments
>
> Makefile | 1 +
> pylibfdt/.gitignore | 3 +
> pylibfdt/Makefile.pylibfdt | 18 ++
> pylibfdt/libfdt.swig | 477 +++++++++++++++++++++++++++++++++++++++++++++
> pylibfdt/setup.py | 34 ++++
> 5 files changed, 533 insertions(+)
> create mode 100644 pylibfdt/.gitignore
> create mode 100644 pylibfdt/Makefile.pylibfdt
> create mode 100644 pylibfdt/libfdt.swig
> create mode 100644 pylibfdt/setup.py
>
> diff --git a/Makefile b/Makefile
> index ce05eba..1c48210 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -22,6 +22,7 @@ CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
>
> BISON = bison
> LEX = flex
> +SWIG = swig
>
> INSTALL = /usr/bin/install
> DESTDIR =
> diff --git a/pylibfdt/.gitignore b/pylibfdt/.gitignore
> new file mode 100644
> index 0000000..5e8c5e3
> --- /dev/null
> +++ b/pylibfdt/.gitignore
> @@ -0,0 +1,3 @@
> +libfdt.py
> +libfdt.pyc
> +libfdt_wrap.c
> diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
> new file mode 100644
> index 0000000..0c0b390
> --- /dev/null
> +++ b/pylibfdt/Makefile.pylibfdt
> @@ -0,0 +1,18 @@
> +# Makefile.pylibfdt
> +#
> +
> +PYLIBFDT_srcs = $(addprefix $(LIBFDT_srcdir)/,$(LIBFDT_SRCS))
> +WRAP = $(PYLIBFDT_objdir)/libfdt_wrap.c
> +PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
> +
> +$(PYMODULE): $(PYLIBFDT_srcs) $(WRAP)
> + @$(VECHO) PYMOD $@
> + python $(PYLIBFDT_objdir)/setup.py "$(CPPFLAGS)" $^
> + mv _libfdt.so $(PYMODULE)
> +
> +$(WRAP): $(PYLIBFDT_srcdir)/libfdt.swig
> + @$(VECHO) SWIG $@
> + $(SWIG) -python -o $@ $<
> +
> +PYLIBFDT_cleanfiles = libfdt_wrap.c libfdt.py libfdt.pyc
> +PYLIBFDT_CLEANFILES = $(addprefix $(PYLIBFDT_objdir)/,$(PYLIBFDT_cleanfiles))
> diff --git a/pylibfdt/libfdt.swig b/pylibfdt/libfdt.swig
> new file mode 100644
> index 0000000..03de79e
> --- /dev/null
> +++ b/pylibfdt/libfdt.swig
> @@ -0,0 +1,477 @@
> +/*
> + * pylibfdt - Flat Device Tree manipulation in Python
> + * Copyright (C) 2017 Google, Inc.
> + * Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> + *
> + * libfdt is dual licensed: you can use it either under the terms of
> + * the GPL, or the BSD license, at your option.
> + *
> + * a) This library 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 library 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 library; if not, write to the Free
> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + *
> + * Alternatively,
> + *
> + * b) Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer.
> + * 2. Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer in the documentation and/or other materials
> + * provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
> + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
> + * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +%module libfdt
> +
> +%{
> +#define SWIG_FILE_WITH_INIT
> +#include "libfdt.h"
> +%}
> +
> +%pythoncode %{
> +
> +import struct
> +
> +_NUM_ERRORS = 18
> +
> +# Error codes, corresponding to FDT_ERR_... in libfdt.h
> +(NOTFOUND,
> + EXISTS,
> + NOSPACE,
> + BADOFFSET,
> + BADPATH,
> + BADPHANDLE,
> + BADSTATE,
> + TRUNCATED,
> + BADMAGIC,
> + BADVERSION,
> + BADSTRUCTURE,
> + BADLAYOUT,
> + INTERNAL,
> + BADNCELLS,
> + BADVALUE,
> + BADOVERLAY,
> + NOPHANDLES) = range(1, _NUM_ERRORS)
Nit: you could actually assign QUIET_ALL right here.
> +# Pass this as the 'quiet' parameter to return -ENOTFOUND on NOTFOUND errors,
> +# instead of raising an exception.
> +QUIET_NOTFOUND = (NOTFOUND,)
> +
> +# Pass this as the 'quiet' parameter to avoid exceptions altogether. All
> +# functions passed this value will return an error instead of raising an
> +# exception.
> +QUIET_ALL = range(1, _NUM_ERRORS)
> +
> +
> +class FdtException(Exception):
> + """An exception caused by an error such as one of the codes above"""
> + def __init__(self, err):
> + self.err = err
> +
> + def __str__(self):
> + return 'pylibfdt error %d: %s' % (self.err, fdt_strerror(self.err))
> +
> +def fdt32_to_cpu(val):
> + """Convert a device-tree cell value into a native integer"""
> + return struct.unpack("=I", struct.pack(">I", val))[0]
I think this is conceptually wrong, although I think it will give the
right results. AIUI this is used when swig's built in type conversion
has incorrectly treated a structure with an integer member as being in
native format when it's actually in BE format.
So rather than storing as BE, then loading as native, you should store
as native - "undoing" the incorrect load within swig - then correctly
loading as BE.
Even better would be configuring swig not to make the mistake in the
first place, but I can accept that that's more trouble than it's
worth.
> +
> +def strerror(fdt_err):
> + """Get the string for an error number
> +
> + Args:
> + fdt_err: Error number (-ve)
> +
> + Returns:
> + String containing the associated error
> + """
> + return fdt_strerror(fdt_err)
> +
> +def check_err(val, quiet=()):
> + """Raise an error if the return value is -ve
> +
> + This is used to check for errors returned by libfdt C functions.
> +
> + Args:
> + val: Return value from a libfdt function
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + val if val >= 0
> +
> + Raises
> + FdtException if val < 0
> + """
> + if val < 0:
> + if -val not in quiet:
> + raise FdtException(val)
> + return val
> +
> +def check_err_null(val, quiet=()):
> + """Raise an error if the return value is NULL
> +
> + This is used to check for a NULL return value from certain libfdt C
> + functions
> +
> + Args:
> + val: Return value from a libfdt function
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + val if val is a list, None if not
> +
> + Raises
> + FdtException if val indicates an error was reported and the error
> + is not in @quiet.
> + """
> + # Normally a list is returned which contains the data and its length.
> + # If we get just an integer error code, it means the function failed.
> + if not isinstance(val, list):
> + if -val not in quiet:
> + raise FdtException(val)
> + return val
> +
> +class Fdt:
> + """Device tree class, supporting all operations
> +
> + The Fdt object is created is created from a device tree binary file,
> + e.g. with something like:
> +
> + fdt = Fdt(open("filename.dtb").read())
> +
> + Operations can then be performed using the methods in this class. Each
> + method xxx(args...) corresponds to a libfdt function fdt_xxx(fdt, args...).
> +
> + All methods raise an FdtException if an error occurs. To avoid this
> + behaviour a 'quiet' parameter is provided for some functions. This
> + defaults to empty, but you can pass a list of errors that you expect.
> + If one of these errors occurs, the function will return an error number
> + (e.g. -NOTFOUND).
> + """
> + def __init__(self, data):
> + self._fdt = bytearray(data)
> + check_err(fdt_check_header(self._fdt));
> +
> + def path_offset(self, path, quiet=()):
> + """Get the offset for a given path
> +
> + Args:
> + path: Path to the required node, e.g. '/node@3/subnode@1'
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + Node offset
> +
> + Raises
> + FdtException if the path is not valid or not found
> + """
> + return check_err(fdt_path_offset(self._fdt, path), quiet)
> +
> + def first_property_offset(self, nodeoffset, quiet=()):
> + """Get the offset of the first property in a node offset
> +
> + Args:
> + nodeoffset: Offset to the node to check
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + Offset of the first property
> +
> + Raises
> + FdtException if the associated node has no properties, or some
> + other error occurred
> + """
> + return check_err(fdt_first_property_offset(self._fdt, nodeoffset),
> + quiet)
> +
> + def next_property_offset(self, prop_offset, quiet=()):
> + """Get the next property in a node
> +
> + Args:
> + prop_offset: Offset of the previous property
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + Offset of the next property
> +
> + Raises:
> + FdtException if the associated node has no more properties, or
> + some other error occurred
> + """
> + return check_err(fdt_next_property_offset(self._fdt, prop_offset),
> + quiet)
> +
> + def get_name(self, nodeoffset):
> + """Get the name of a node
> +
> + Args:
> + nodeoffset: Offset of node to check
> +
> + Returns:
> + Node name
> +
> + Raises:
> + FdtException on error (e.g. nodeoffset is invalid)
> + """
> + return check_err_null(fdt_get_name(self._fdt, nodeoffset))[0]
> +
> + def get_property_by_offset(self, prop_offset, quiet=()):
> + """Obtains a property that can be examined
> +
> + Args:
> + prop_offset: Offset of property (e.g. from first_property_offset())
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + Property object, or None if not found
> +
> + Raises:
> + FdtException on error (e.g. invalid prop_offset or device
> + tree format)
> + """
> + pdata = check_err_null(
> + fdt_get_property_by_offset(self._fdt, prop_offset), quiet)
> + if isinstance(pdata, (int)):
> + return pdata
> + name = fdt_string(self._fdt, fdt32_to_cpu(pdata[0].nameoff))
> + value = bytearray(fdt32_to_cpu(pdata[0].len))
> + pylibfdt_copy_value(value, pdata[0])
> + return Property(name, value)
So.. just to check I understand. swig is translating the the return
value of fdt_get_property_by_offset() into:
If it returns NULL, an integer from the error code in *lenp
Otherwise, a tuple, where the second element is the length
from *lenp, the first element is a Python object with nameoff
and len attributes with those fields from the C structure?
I still find the copy_value() thing ugly, but I think I've finally
understood why it's the easiest way to accomplish what you need. I
take it swig guarantees that if you pass that structure/object back
into a C function it will pass an identical pointer, not a copy of the
C structure (which might no longer have the data in the variable
length field).
> +
> +
> + def first_subnode(self, nodeoffset, quiet=()):
> + """Find the first subnode of a parent node
> +
> + Args:
> + nodeoffset: Node offset of parent node
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + The offset of the first subnode, if any
> +
> + Raises:
> + FdtException if no subnode found or other error occurs
> + """
> + return check_err(fdt_first_subnode(self._fdt, nodeoffset), quiet)
> +
> + def next_subnode(self, nodeoffset, quiet=()):
> + """Find the next subnode
> +
> + Args:
> + nodeoffset: Node offset of previous subnode
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + The offset of the next subnode, if any
> +
> + Raises:
> + FdtException if no more subnode found or other error occurs
> + """
> + return check_err(fdt_next_subnode(self._fdt, nodeoffset), quiet)
> +
> + def totalsize(self):
> + """Return the total size of the device tree
> +
> + Returns:
> + Total tree size in bytes
> + """
> + return check_err(fdt_totalsize(self._fdt))
> +
> + def off_dt_struct(self):
> + """Return the start of the device tree struct area
> +
> + Returns:
> + Start offset of struct area
> + """
> + return check_err(fdt_off_dt_struct(self._fdt))
> +
> + def pack(self, quiet=()):
> + """Pack the device tree to remove unused space
> +
> + This adjusts the tree in place.
> +
> + Args:
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Raises:
> + FdtException if any error occurs
> + """
> + return check_err(fdt_pack(self._fdt), quiet)
> +
> + def delprop(self, nodeoffset, prop_name):
> + """Delete a property from a node
> +
> + Args:
> + nodeoffset: Node offset containing property to delete
> + prop_name: Name of property to delete
> +
> + Raises:
> + FdtError if the property does not exist, or another error occurs
> + """
> + return check_err(fdt_delprop(self._fdt, nodeoffset, prop_name))
> +
> + def getprop(self, nodeoffset, prop_name, quiet=()):
> + """Get a property from a node
> +
> + Args:
> + nodeoffset: Node offset containing property to get
> + prop_name: Name of property to get
> + quiet: Errors to ignore (empty to raise on all errors)
> +
> + Returns:
> + Value of property as a bytearray, or -ve error number
> +
> + Raises:
> + FdtError if any error occurs (e.g. the property is not found)
> + """
> + pdata = check_err_null(fdt_getprop(self._fdt, nodeoffset, prop_name),
> + quiet)
> + if isinstance(pdata, (int)):
> + return pdata
> + return bytearray(pdata[0])
> +
> +
> +class Property:
> + """Holds a device tree property name and value.
> +
> + This holds a copy of a property taken from the device tree. It does not
> + reference the device tree, so if anything changes in the device tree,
> + a Property object will remain valid.
> +
> + Properties:
> + name: Property name
> + value: Proper value as a bytearray
> + """
> + def __init__(self, name, value):
> + self.name = name
> + self.value = value
> +%}
> +
> +%rename(fdt_property) fdt_property_func;
> +
> +typedef int fdt32_t;
> +
> +%include "libfdt/fdt.h"
> +
> +%include "typemaps.i"
> +
> +/*
> + * Unfortunately the defintiion of pybuffer_mutable_binary() in my Python
> + * version appears to be broken:
> + * pylibfdt/libfdt_wrap.c: In function ‘_wrap_pylibfdt_copy_value’:
> + * pylibfdt/libfdt_wrap.c:3603:22: error: ‘size’ undeclared (first use in this
> + * function)
> + * arg2 = (size_t) (size/sizeof(char));
> + *
> + * This version works correctly.
> + */
> +%define %mypybuffer_mutable_binary(TYPEMAP, SIZE)
> +%typemap(in) (TYPEMAP, SIZE)(int res, Py_ssize_t size = 0, void *buf = 0)
> +{
> + res = PyObject_AsWriteBuffer($input, &buf, &size);
> + if (res < 0) {
> + PyErr_Clear();
> + %argument_fail(res, "(TYPEMAP, SIZE)", $symname, $argnum);
> + }
> + $1 = ($1_ltype)buf;
> + $2 = ($2_ltype)(size1 / sizeof($*1_type));
> +}
> +%enddef
> +
> +/* This is used to copy property data into a bytearray */
> +%mypybuffer_mutable_binary(uint8_t *str, size_t size);
> +void pylibfdt_copy_value(uint8_t *str, size_t size,
> + const struct fdt_property *prop);
> +
> +/* Most functions don't change the device tree, so use a const void * */
> +%typemap(in) (const void *) {
> + if (!PyByteArray_Check($input)) {
> + SWIG_exception_fail(SWIG_TypeError, "in method '" "$symname"
> + "', argument " "$argnum"" of type '" "$type""'");
> + }
> + $1 = (void *)PyByteArray_AsString($input);
> +}
> +
> +/* Some functions do change the device tree, so use void * */
> +%typemap(in) (void *) {
> + if (!PyByteArray_Check($input)) {
> + SWIG_exception_fail(SWIG_TypeError, "in method '" "$symname"
> + "', argument " "$argnum"" of type '" "$type""'");
> + }
> + $1 = PyByteArray_AsString($input);
> +}
> +
> +%inline %{
> +
> +/**
> + * pylibfdt_copy_value() - Copy value from a property to the given buffer
> + *
> + * This is used by the Property class to place the contents of a property
> + * into a bytearray.
> + *
> + * @buf: Destination pointer (typically the start of the bytearray)
> + * @size: Number of bytes to copy (size of bytearray)
> + * @prop: Property to copy
> + */
> +void pylibfdt_copy_value(uint8_t *buf, size_t size, const struct fdt_property *prop)
> +{
> + memcpy(buf, prop + 1, size);
Oh.. one more nit. I was recently reminded that memcpy(dst, src, 0)
isn't guaranteed to be a no-op so may invoke undefined behaviour (some
checkers like Coverity complain about it). So it's probably worth
having a check for zero length either here or in the Python caller.
> +}
> +
> +%}
> +
> +%apply int *OUTPUT { int *lenp };
> +
> +/* typemap used for fdt_getprop() */
> +%typemap(out) (const void *) {
> + if (!$1)
> + $result = Py_None;
> + else
> + /* TODO(sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org): Can we avoid the 'arg4'? */
> + $result = Py_BuildValue("s#", $1, *arg4);
What's the arg4 about? This isn't relying on swig internals which are
subject to change is it?
> +}
> +
> +/* We have both struct fdt_property and a function fdt_property() */
> +%warnfilter(302) fdt_property;
> +
> +/* These are macros in the header so have to be redefined here */
> +int fdt_magic(const void *fdt);
> +int fdt_totalsize(const void *fdt);
> +int fdt_off_dt_struct(const void *fdt);
> +int fdt_off_dt_strings(const void *fdt);
> +int fdt_off_mem_rsvmap(const void *fdt);
> +int fdt_version(const void *fdt);
> +int fdt_last_comp_version(const void *fdt);
> +int fdt_boot_cpuid_phys(const void *fdt);
> +int fdt_size_dt_strings(const void *fdt);
> +int fdt_size_dt_struct(const void *fdt);
> +
> +%include <../libfdt/libfdt.h>
> diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py
> new file mode 100644
> index 0000000..8f8618e
> --- /dev/null
> +++ b/pylibfdt/setup.py
> @@ -0,0 +1,34 @@
> +#!/usr/bin/env python
> +
> +"""
> +setup.py file for SWIG libfdt
> +"""
> +
> +from distutils.core import setup, Extension
> +import os
> +import sys
> +
> +progname = sys.argv[0]
> +cflags = sys.argv[1]
> +files = sys.argv[2:]
> +
> +if cflags:
> + cflags = [flag for flag in cflags.split(' ') if flag]
> +else:
> + cflags = None
> +
> +libfdt_module = Extension(
> + '_libfdt',
> + sources = files,
> + extra_compile_args = cflags
> +)
> +
> +sys.argv = [progname, '--quiet', 'build_ext', '--inplace']
> +
> +setup (name = 'libfdt',
> + version = '0.1',
> + author = "SWIG Docs",
> + description = """Simple swig libfdt from docs""",
> + ext_modules = [libfdt_module],
> + py_modules = ["libfdt"],
> + )
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 2/5] Add tests for pylibfdt
[not found] ` <20170222043340.17008-3-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2017-02-24 2:52 ` David Gibson
[not found] ` <20170224025254.GJ17615-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: David Gibson @ 2017-02-24 2:52 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
[-- Attachment #1: Type: text/plain, Size: 16175 bytes --]
On Tue, Feb 21, 2017 at 09:33:37PM -0700, Simon Glass wrote:
> Add a set of tests to cover the functionality in pylibfdt.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>
> Changes in v7:
> - Add a test for QUIET_ALL
>
> Changes in v6:
> - Adjust tests to avoid checking a hard-coded offset
> - Use 0 instead of self.fdt.path_offset('/')
> - Adjust the totalsize() test to compare against the file size
> - Adjust test result processing to avoid using awk
>
> Changes in v5:
> - Adjust tests to match new swig bindings
>
> Changes in v4:
> - Drop tests that are no-longer applicable
> - Add a get for getprop()
>
> Changes in v3:
> - Add some more tests
>
> Changes in v2:
> - Update tests for new pylibfdt
> - Add a few more tests to increase coverage
>
> tests/pylibfdt_tests.py | 275 ++++++++++++++++++++++++++++++++++++++++++++++++
> tests/run_tests.sh | 16 ++-
> 2 files changed, 290 insertions(+), 1 deletion(-)
> create mode 100644 tests/pylibfdt_tests.py
>
> diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
> new file mode 100644
> index 0000000..796684a
> --- /dev/null
> +++ b/tests/pylibfdt_tests.py
> @@ -0,0 +1,275 @@
> +# pylibfdt - Tests for Flat Device Tree manipulation in Python
> +# Copyright (C) 2017 Google, Inc.
> +# Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> +#
> +# libfdt is dual licensed: you can use it either under the terms of
> +# the GPL, or the BSD license, at your option.
> +#
> +# a) This library 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 library 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 library; if not, write to the Free
> +# Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> +# MA 02110-1301 USA
> +#
> +# Alternatively,
> +#
> +# b) Redistribution and use in source and binary forms, with or
> +# without modification, are permitted provided that the following
> +# conditions are met:
> +#
> +# 1. Redistributions of source code must retain the above
> +# copyright notice, this list of conditions and the following
> +# disclaimer.
> +# 2. Redistributions in binary form must reproduce the above
> +# copyright notice, this list of conditions and the following
> +# disclaimer in the documentation and/or other materials
> +# provided with the distribution.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> +# CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
> +# INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> +# MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> +# NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> +# OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
> +# EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +#
> +
> +import sys
> +import unittest
> +
> +sys.path.append('../pylibfdt')
> +import libfdt
> +from libfdt import FdtException, QUIET_NOTFOUND, QUIET_ALL
> +
> +# Offsets of properties in the root node
> +ROOT_PROPS = (8, 32, 48, 68, 92, 108)
As with nodes, relying on exact property offsets in your test is
probably a mistake.
> +
> +def get_err(err_code):
> + """Convert an error code into an error message
> +
> + Args:
> + err_code: Error code value (FDT_ERR_...)
> +
> + Returns:
> + String error code
> + """
> + return 'pylibfdt error %d: %s' % (-err_code, libfdt.fdt_strerror(-err_code))
> +
> +def _ReadFdt(fname):
> + """Read a device tree file into an Fdt object, ready for use
> +
> + Args:
> + fname: Filename to read from
> +
> + Returns:
> + Fdt bytearray suitable for passing to libfdt functions
> + """
> + return libfdt.Fdt(open(fname).read())
> +
> +class PyLibfdtTests(unittest.TestCase):
> + """Test class for pylibfdt
> +
> + Properties:
> + fdt: Device tree file used for testing
> + """
> +
> + def setUp(self):
> + """Read in the device tree we use for testing"""
> + self.fdt = _ReadFdt('test_tree1.dtb')
> +
> + def GetPropList(self, node_path):
> + """Read a list of properties from a node
> +
> + Args:
> + node_path: Full path to node, e.g. '/subnode@1/subsubnode'
> +
> + Returns:
> + List of property names for that node, e.g. ['compatible', 'reg']
> + """
> + prop_list = []
> + node = self.fdt.path_offset(node_path)
> + poffset = self.fdt.first_property_offset(node, QUIET_NOTFOUND)
> + while poffset > 0:
> + prop = self.fdt.get_property_by_offset(poffset)
> + prop_list.append(prop.name)
> + poffset = self.fdt.next_property_offset(poffset, QUIET_NOTFOUND)
> + return prop_list
> +
> + def testImport(self):
> + """Check that we can import the library correctly"""
> + self.assertEquals(type(libfdt), type(sys))
Using types.ModuleType might be a bit clearer than using type(sys).
> +
> + def testBadFdt(self):
> + """Check that a filename provided accidentally is not accepted"""
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADMAGIC)):
> + fdt = libfdt.Fdt('a string')
> +
> + def testPathOffset(self):
> + """Check that we can find the offset of a node"""
> + self.assertEquals(self.fdt.path_offset('/'), 0)
> + self.assertTrue(self.fdt.path_offset('/subnode@1') > 0)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
> + self.fdt.path_offset('/wibble')
Checking the error string by regexp seems a bit indirect. Wouldn't it
be better to add new helper that checks explicitly for an FdtException
with the right fdt error code embedded in it?
> + self.assertEquals(self.fdt.path_offset('/wibble', QUIET_NOTFOUND),
> + -libfdt.NOTFOUND)
> +
> + def testPropertyOffset(self):
> + """Walk through all the properties in the root node"""
> + self.assertEquals(self.fdt.first_property_offset(0), ROOT_PROPS[0])
> + for pos in range(len(ROOT_PROPS) - 1):
> + self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[pos]),
> + ROOT_PROPS[pos + 1])
> + self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[-1],
> + QUIET_NOTFOUND),
> + -libfdt.NOTFOUND)
> +
> + def testPropertyOffsetExceptions(self):
> + """Check that exceptions are raised as expected"""
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.next_property_offset(107)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.first_property_offset(107, QUIET_NOTFOUND)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.next_property_offset(107, QUIET_NOTFOUND)
Isn't this a dupe of the previous statement? This also looks like a
case where using QUIET_ALL could make it neater.
> +
> + node = self.fdt.path_offset('/subnode@1/ss1')
> + self.assertEquals(self.fdt.first_property_offset(node, QUIET_NOTFOUND),
> + -libfdt.NOTFOUND)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
> + self.fdt.first_property_offset(node)
> +
> + def testGetName(self):
> + """Check that we can get the name of a node"""
> + self.assertEquals(self.fdt.get_name(0), '')
> + node = self.fdt.path_offset('/subnode@1/subsubnode')
> + self.assertEquals(self.fdt.get_name(node), 'subsubnode')
> +
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.get_name(-2)
> +
> + def testGetPropertyByOffset(self):
> + """Check that we can read the name and contents of a property"""
> + root = 0
> + poffset = self.fdt.first_property_offset(root)
> + prop = self.fdt.get_property_by_offset(poffset)
> + self.assertEquals(prop.name, 'compatible')
> + self.assertEquals(prop.value, 'test_tree1\0')
> +
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.get_property_by_offset(-2)
> + self.assertEquals(
> + -libfdt.BADOFFSET,
> + self.fdt.get_property_by_offset(-2, [libfdt.BADOFFSET]))
> +
> + def testGetProp(self):
> + """Check that we can read the contents of a property by name"""
> + root = self.fdt.path_offset('/')
> + value = self.fdt.getprop(root, "compatible")
> + self.assertEquals(value, 'test_tree1\0')
> + self.assertEquals(-libfdt.NOTFOUND, self.fdt.getprop(root, 'missing',
> + QUIET_NOTFOUND))
> +
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
> + self.fdt.getprop(root, 'missing')
> +
> + node = self.fdt.path_offset('/subnode@1/subsubnode')
> + value = self.fdt.getprop(node, "compatible")
> + self.assertEquals(value, 'subsubnode1\0subsubnode\0')
> +
> + def testStrError(self):
> + """Check that we can get an error string"""
> + self.assertEquals(libfdt.strerror(-libfdt.NOTFOUND),
> + 'FDT_ERR_NOTFOUND')
> +
> + def testFirstNextSubnodeOffset(self):
> + """Check that we can walk through subnodes"""
> + node_list = []
> + node = self.fdt.first_subnode(0, QUIET_NOTFOUND)
> + while node >= 0:
> + node_list.append(self.fdt.get_name(node))
> + node = self.fdt.next_subnode(node, QUIET_NOTFOUND)
> + self.assertEquals(node_list, ['subnode@1', 'subnode@2'])
> +
> + def testFirstNextSubnodeOffsetExceptions(self):
> + """Check except handling for first/next subnode functions"""
> + node = self.fdt.path_offset('/subnode@1/subsubnode', QUIET_NOTFOUND)
> + self.assertEquals(self.fdt.first_subnode(node, QUIET_NOTFOUND),
> + -libfdt.NOTFOUND)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
> + self.fdt.first_subnode(node)
> +
> + node = self.fdt.path_offset('/subnode@1/ss1', QUIET_NOTFOUND)
> + self.assertEquals(self.fdt.next_subnode(node, QUIET_NOTFOUND),
> + -libfdt.NOTFOUND)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
> + self.fdt.next_subnode(node)
> +
> + def testDeleteProperty(self):
> + """Test that we can delete a property"""
> + node_name = '/subnode@1'
> + self.assertEquals(self.GetPropList(node_name),
> + ['compatible', 'reg', 'prop-int'])
> + node = self.fdt.path_offset('/%s' % node_name)
> + self.assertEquals(self.fdt.delprop(node, 'reg'), 0)
> + self.assertEquals(self.GetPropList(node_name),
> + ['compatible', 'prop-int'])
> +
> + def testHeader(self):
> + """Test that we can access the header values"""
> + self.assertEquals(self.fdt.totalsize(), len(self.fdt._fdt))
> + self.assertEquals(self.fdt.off_dt_struct(), 88)
> +
> + def testPack(self):
> + """Test that we can pack the tree after deleting something"""
> + self.assertEquals(self.fdt.totalsize(), 693)
> + node = self.fdt.path_offset('/subnode@2', QUIET_NOTFOUND)
> + self.assertEquals(self.fdt.delprop(node, 'prop-int'), 0)
> + self.assertEquals(self.fdt.totalsize(), 693)
> + self.assertEquals(self.fdt.pack(), 0)
> + self.assertEquals(self.fdt.totalsize(), 677)
Like exact offsets, exact sizes are probably a mistake here - a new
fdt version which adds a header field would break this, for example.
I think just checking the new size is less than the old should be
sufficient.
> +
> + def testBadPropertyOffset(self):
> + """Test that bad property offsets are detected"""
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.get_property_by_offset(13)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.first_property_offset(3)
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> + self.fdt.next_property_offset(3)
> +
> + def testBadPathOffset(self):
> + """Test that bad path names are detected"""
> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADPATH)):
> + self.fdt.path_offset('not-present')
> +
> + def testEndian(self):
> + """Check that we can convert from FDT (big endian) to native endian"""
> + self.assertEquals(libfdt.fdt32_to_cpu(0x10000000), 0x10)
> +
> + def testQuietAll(self):
> + """Check that exceptions can be masked by QUIET_ALL"""
> + self.assertEquals(-libfdt.NOTFOUND,
> + self.fdt.path_offset('/missing', QUIET_ALL))
> + self.assertEquals(-libfdt.BADOFFSET,
> + self.fdt.get_property_by_offset(13, QUIET_ALL))
> + self.assertEquals(-libfdt.BADPATH,
> + self.fdt.path_offset('missing', QUIET_ALL))
> +
> +
> +if __name__ == "__main__":
> + unittest.main()
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index ed489db..76d20d8 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -769,6 +769,17 @@ fdtdump_tests () {
> run_fdtdump_test fdtdump.dts
> }
>
> +pylibfdt_tests () {
> + TMP=/tmp/tests.stderr.$$
> + python pylibfdt_tests.py 2> ${TMP}
> + result=$(head -1 ${TMP})
> +
> + # Extract the test results and add them to our totals
> + tot_fail=$((tot_fail + $(echo $result | tr -d '\n.' | wc -c)))
> + tot_pass=$((tot_pass + $(echo $result | tr -d '\nF' | wc -c)))
> + tot_tests=$((tot_tests + $(echo $result | tr -d '\n' | wc -c)))
> +}
> +
> while getopts "vt:me" ARG ; do
> case $ARG in
> "v")
> @@ -787,7 +798,7 @@ while getopts "vt:me" ARG ; do
> done
>
> if [ -z "$TESTSETS" ]; then
> - TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump"
> + TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump pylibfdt"
> fi
>
> # Make sure we don't have stale blobs lying around
> @@ -816,6 +827,9 @@ for set in $TESTSETS; do
> "fdtdump")
> fdtdump_tests
> ;;
> + "pylibfdt")
> + pylibfdt_tests
> + ;;
> esac
> done
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 3/5] Mention pylibfdt in the documentation
[not found] ` <20170222043340.17008-4-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2017-02-24 2:53 ` David Gibson
0 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2017-02-24 2:53 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
[-- Attachment #1: Type: text/plain, Size: 2894 bytes --]
On Tue, Feb 21, 2017 at 09:33:38PM -0700, Simon Glass wrote:
> Add a note about pylibfdt in the README.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> ---
>
> Changes in v7: None
> Changes in v6:
> - Update example to print the node value as an integer
> - Update example to print the bytestring as well as the string
>
> Changes in v5:
> - Use an interactive session to demonstrate pylibfdt
> - Mention that more work remains
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - Add details on how to obtain full help and code coverage
>
> README | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/README b/README
> index f92008f..96d8486 100644
> --- a/README
> +++ b/README
> @@ -7,6 +7,53 @@ DTC and LIBFDT are maintained by:
> David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> Jon Loeliger <jdl-CYoMK+44s/E@public.gmane.org>
>
> +
> +Python library
> +--------------
> +
> +A Python library is also available. To build this you will need to install
> +swig and Python development files. On Debian distributions:
> +
> + sudo apt-get install swig python-dev
> +
> +The library provides an Fdt class which you can use like this:
> +
> +$ PYTHONPATH=../pylibfdt python
> +>>> import libfdt
> +>>> fdt = libfdt.Fdt(open('test_tree1.dtb').read())
> +>>> node = fdt.path_offset('/subnode@1')
> +>>> print node
> +124
> +>>> prop_offset = fdt.first_property_offset(node)
> +>>> prop = fdt.get_property_by_offset(prop_offset)
> +>>> print '%s=%r' % (prop.name, prop.value)
> +compatible=bytearray(b'subnode1\x00')
> +>>> print '%s=%s' % (prop.name, prop.value)
> +compatible=subnode1
> +>>> node2 = fdt.path_offset('/')
> +>>> print fdt.getprop(node2, 'compatible')
> +test_tree1
> +
> +You will find tests in tests/pylibfdt_tests.py showing how to use each
> +method. Help is available using the Python help command, e.g.:
> +
> + $ cd pylibfdt
> + $ python -c "import libfdt; help(libfdt)"
> +
> +If you add new features, please check code coverage:
> +
> + $ sudo apt-get install python-pip python-pytest
> + $ sudo pip install coverage
> + $ cd tests
> + $ coverage run pylibfdt_tests.py
> + $ coverage html
> + # Open 'htmlcov/index.html' in your browser
> +
> +
> +More work remains to support all of libfdt, including access to numeric
> +values.
> +
> +
> Mailing list
> ------------
> The following list is for discussion about dtc and libfdt implementation
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 4/5] Adjust libfdt.h to work with swig
[not found] ` <20170222043340.17008-5-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2017-02-24 2:55 ` David Gibson
0 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2017-02-24 2:55 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
[-- Attachment #1: Type: text/plain, Size: 2862 bytes --]
On Tue, Feb 21, 2017 at 09:33:39PM -0700, Simon Glass wrote:
> There are a few places where libfdt.h cannot be used as is with swig:
>
> - macros like fdt_totalsize() have to be defined as C declarations
> - fdt_offset_ptr() and fdt_getprop_namelen() need special treatment due to
> a TODO in the wrapper for fdt_getprop(). However they are not useful to
> Python so can be removed
>
> Add #ifdefs to work around these problem.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>
> Changes in v7: None
> Changes in v6: None
> Changes in v5:
> - Update commit message
> - Drop #ifdef around fdt_get_header() macros
>
> Changes in v4:
> - Add new patch to adjust libfdt.h to work with swig
>
> Changes in v3: None
> Changes in v2: None
>
> libfdt/libfdt.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
> index c69e918..2aa8ff6 100644
> --- a/libfdt/libfdt.h
> +++ b/libfdt/libfdt.h
> @@ -143,7 +143,9 @@
> /* Low-level functions (you probably don't need these) */
> /**********************************************************************/
>
> +#ifndef SWIG /* This functions are not useful in Python */
> const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int checklen);
> +#endif
> static inline void *fdt_offset_ptr_w(void *fdt, int offset, int checklen)
> {
> return (void *)(uintptr_t)fdt_offset_ptr(fdt, offset, checklen);
> @@ -210,7 +212,6 @@ int fdt_next_subnode(const void *fdt, int offset);
> /**********************************************************************/
> /* General functions */
> /**********************************************************************/
> -
This looks to be an unrelated change.
> #define fdt_get_header(fdt, field) \
> (fdt32_to_cpu(((const struct fdt_header *)(fdt))->field))
> #define fdt_magic(fdt) (fdt_get_header(fdt, magic))
> @@ -638,8 +639,10 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
> * Identical to fdt_getprop(), but only examine the first namelen
> * characters of name for matching the property name.
> */
> +#ifndef SWIG /* Not available in Python */
> const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
> const char *name, int namelen, int *lenp);
> +#endif
> static inline void *fdt_getprop_namelen_w(void *fdt, int nodeoffset,
> const char *name, int namelen,
> int *lenp)
You might as well exclude all the rest of the 'namelen' functions
while you're at it.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 5/5] Build pylibfdt as part of the normal build process
[not found] ` <20170222043340.17008-6-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2017-02-24 2:55 ` David Gibson
0 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2017-02-24 2:55 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]
On Tue, Feb 21, 2017 at 09:33:40PM -0700, Simon Glass wrote:
> Possibly this needs to be made optional. For now just hook it up.
>
> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
My one concern is that I don't want to add extra dependencies to the
regular build, particularly not ones as substantial as Python and
swig. Can we rig the Makefile to only build the Python library if
Python headers and swig are available, otherwise just turn it off?
> ---
>
> Changes in v7: None
> Changes in v6: None
> Changes in v5:
> - Fix 'possible' typo
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
> Makefile | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 1c48210..4adba10 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -116,7 +116,7 @@ BIN += fdtput
>
> SCRIPTS = dtdiff
>
> -all: $(BIN) libfdt
> +all: $(BIN) libfdt pylibfdt
>
>
> ifneq ($(DEPTARGETS),)
> @@ -203,6 +203,19 @@ dist:
> cat ../dtc-$(dtc_version).tar | \
> gzip -9 > ../dtc-$(dtc_version).tar.gz
>
> +
> +#
> +# Rules for pylibfdt
> +#
> +PYLIBFDT_srcdir = pylibfdt
> +PYLIBFDT_objdir = pylibfdt
> +
> +include $(PYLIBFDT_srcdir)/Makefile.pylibfdt
> +
> +.PHONY: pylibfdt
> +pylibfdt: $(PYLIBFDT_objdir)/_libfdt.so
> +
> +
> #
> # Release signing and uploading
> # This is for maintainer convenience, don't try this at home.
> @@ -247,6 +260,7 @@ STD_CLEANFILES = *~ *.o *.$(SHAREDLIB_EXT) *.d *.a *.i *.s core a.out vgcore.* \
> clean: libfdt_clean tests_clean
> @$(VECHO) CLEAN
> rm -f $(STD_CLEANFILES)
> + rm -f $(PYLIBFDT_CLEANFILES)
> rm -f $(VERSION_FILE)
> rm -f $(BIN)
> rm -f dtc-*.tar dtc-*.tar.sign dtc-*.tar.asc
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 1/5] Add an initial Python library for libfdt
[not found] ` <20170224024317.GI17615-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2017-03-01 5:40 ` Simon Glass
[not found] ` <CAPnjgZ3p3KfDwJwj=VfnQURL5MQ2tWXBMrb8RViK+tDhj1ikHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-03-01 5:40 UTC (permalink / raw)
To: David Gibson; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
Hi David,
On 23 February 2017 at 19:43, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Tue, Feb 21, 2017 at 09:33:36PM -0700, Simon Glass wrote:
>> Add Python bindings for a bare-bones set of libfdt functions. These allow
>> navigating the tree and reading node names and properties.
>>
>> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>>
>> Changes in v7:
>> - Add QUIET_ALL to silence all exceptions
>>
>> Changes in v6:
>> - Use a tuple instead of list for the default quiert parameter
>> - Use a tuple instead of list for QUIET_NOTFOUND
>> - Use 'list' instead of 'tuple' for the comment in check_err_null()
>> - Return a bytearray from getprop()
>> - Adjust the Property constructor to accept the name and value
>> - Use uint8_t for pylibfdt_copy_value
>>
>> Changes in v5:
>> - Use a 'quiet' parameter instead of quiet versions of functions
>> - Add a Property object to hold a property's name and value
>> - Drop the data() and string() functions which are not needed now
>> - Rename pylibfdt_copy_data() tp pylibfdt_copy_value()
>> - Change order of libfdt.h inclusion to avoid #ifdef around libfdt macros
>> - Drop fdt_offset_ptr() and fdt_getprop_namelen() from the swig interface
>> - Use $(SWIG) to call swig from the Makefile
>> - Review function comments
>>
>> Changes in v4:
>> - Make the library less pythonic to avoid a shaky illusion
>> - Drop classes for Node and Prop, along with associated methods
>> - Include libfdt.h instead of repeating it
>> - Add support for fdt_getprop()
>> - Bring in all libfdt functions (but Python support is missing for many)
>> - Add full comments for Python methods
>>
>> Changes in v3:
>> - Make the library more pythonic
>> - Add classes for Node and Prop along with methods
>> - Add an exception class
>> - Use Python to generate exeptions instead of SWIG
>>
>> Changes in v2:
>> - Add exceptions when functions return an error
>> - Correct Python naming to following PEP8
>> - Use a class to encapsulate the various methods
>> - Include fdt.h instead of redefining struct fdt_property
>> - Use bytearray to avoid the SWIG warning 454
>> - Add comments
>>
>> Makefile | 1 +
>> pylibfdt/.gitignore | 3 +
>> pylibfdt/Makefile.pylibfdt | 18 ++
>> pylibfdt/libfdt.swig | 477 +++++++++++++++++++++++++++++++++++++++++++++
>> pylibfdt/setup.py | 34 ++++
>> 5 files changed, 533 insertions(+)
>> create mode 100644 pylibfdt/.gitignore
>> create mode 100644 pylibfdt/Makefile.pylibfdt
>> create mode 100644 pylibfdt/libfdt.swig
>> create mode 100644 pylibfdt/setup.py
>>
[...]
>> +
>> +_NUM_ERRORS = 18
>> +
>> +# Error codes, corresponding to FDT_ERR_... in libfdt.h
>> +(NOTFOUND,
>> + EXISTS,
>> + NOSPACE,
>> + BADOFFSET,
>> + BADPATH,
>> + BADPHANDLE,
>> + BADSTATE,
>> + TRUNCATED,
>> + BADMAGIC,
>> + BADVERSION,
>> + BADSTRUCTURE,
>> + BADLAYOUT,
>> + INTERNAL,
>> + BADNCELLS,
>> + BADVALUE,
>> + BADOVERLAY,
>> + NOPHANDLES) = range(1, _NUM_ERRORS)
>
> Nit: you could actually assign QUIET_ALL right here.
it means the comment stands alone, but OK.
>
>> +# Pass this as the 'quiet' parameter to return -ENOTFOUND on NOTFOUND errors,
>> +# instead of raising an exception.
>> +QUIET_NOTFOUND = (NOTFOUND,)
>> +
>> +# Pass this as the 'quiet' parameter to avoid exceptions altogether. All
>> +# functions passed this value will return an error instead of raising an
>> +# exception.
>> +QUIET_ALL = range(1, _NUM_ERRORS)
>> +
>> +
>> +class FdtException(Exception):
>> + """An exception caused by an error such as one of the codes above"""
>> + def __init__(self, err):
>> + self.err = err
>> +
>> + def __str__(self):
>> + return 'pylibfdt error %d: %s' % (self.err, fdt_strerror(self.err))
>> +
>> +def fdt32_to_cpu(val):
>> + """Convert a device-tree cell value into a native integer"""
>> + return struct.unpack("=I", struct.pack(">I", val))[0]
>
> I think this is conceptually wrong, although I think it will give the
> right results. AIUI this is used when swig's built in type conversion
> has incorrectly treated a structure with an integer member as being in
> native format when it's actually in BE format.
>
> So rather than storing as BE, then loading as native, you should store
> as native - "undoing" the incorrect load within swig - then correctly
> loading as BE.
>
> Even better would be configuring swig not to make the mistake in the
> first place, but I can accept that that's more trouble than it's
> worth.
Well I can drop this function and incorporate this code into the typemap.
[...]
>> +
>> + def get_property_by_offset(self, prop_offset, quiet=()):
>> + """Obtains a property that can be examined
>> +
>> + Args:
>> + prop_offset: Offset of property (e.g. from first_property_offset())
>> + quiet: Errors to ignore (empty to raise on all errors)
>> +
>> + Returns:
>> + Property object, or None if not found
>> +
>> + Raises:
>> + FdtException on error (e.g. invalid prop_offset or device
>> + tree format)
>> + """
>> + pdata = check_err_null(
>> + fdt_get_property_by_offset(self._fdt, prop_offset), quiet)
>> + if isinstance(pdata, (int)):
>> + return pdata
>> + name = fdt_string(self._fdt, fdt32_to_cpu(pdata[0].nameoff))
>> + value = bytearray(fdt32_to_cpu(pdata[0].len))
>> + pylibfdt_copy_value(value, pdata[0])
>> + return Property(name, value)
>
> So.. just to check I understand. swig is translating the the return
> value of fdt_get_property_by_offset() into:
> If it returns NULL, an integer from the error code in *lenp
> Otherwise, a tuple, where the second element is the length
> from *lenp, the first element is a Python object with nameoff
> and len attributes with those fields from the C structure?
>
> I still find the copy_value() thing ugly, but I think I've finally
> understood why it's the easiest way to accomplish what you need. I
> take it swig guarantees that if you pass that structure/object back
> into a C function it will pass an identical pointer, not a copy of the
> C structure (which might no longer have the data in the variable
> length field).
Yes, but I can do it with a typemap so I'll go with that for the next version.
[...]
>> +%inline %{
>> +
>> +/**
>> + * pylibfdt_copy_value() - Copy value from a property to the given buffer
>> + *
>> + * This is used by the Property class to place the contents of a property
>> + * into a bytearray.
>> + *
>> + * @buf: Destination pointer (typically the start of the bytearray)
>> + * @size: Number of bytes to copy (size of bytearray)
>> + * @prop: Property to copy
>> + */
>> +void pylibfdt_copy_value(uint8_t *buf, size_t size, const struct fdt_property *prop)
>> +{
>> + memcpy(buf, prop + 1, size);
>
> Oh.. one more nit. I was recently reminded that memcpy(dst, src, 0)
> isn't guaranteed to be a no-op so may invoke undefined behaviour (some
> checkers like Coverity complain about it). So it's probably worth
> having a check for zero length either here or in the Python caller.
I think I can drop this.
>
>> +}
>> +
>> +%}
>> +
>> +%apply int *OUTPUT { int *lenp };
>> +
>> +/* typemap used for fdt_getprop() */
>> +%typemap(out) (const void *) {
>> + if (!$1)
>> + $result = Py_None;
>> + else
>> + /* TODO(sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org): Can we avoid the 'arg4'? */
>> + $result = Py_BuildValue("s#", $1, *arg4);
>
> What's the arg4 about? This isn't relying on swig internals which are
> subject to change is it?
Possibly, as it is not documented. I'm not sure if there is a good
way share input parameters with an output typemap. The typemap.i file
is quite mystical to me, but it does seem to be a rule that parameters
are called 'arg' and numbered. If you have any better solutions I am
all ears.
I will ask on the swig-users mailing list two as I cannot seem to find
a solution already mentioned.
[...]
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 2/5] Add tests for pylibfdt
[not found] ` <20170224025254.GJ17615-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
@ 2017-03-01 5:40 ` Simon Glass
[not found] ` <CAPnjgZ0SB4j1QyD7TrnJf=WTt3To0SvUBT5p81vzfnDMxz1aGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Simon Glass @ 2017-03-01 5:40 UTC (permalink / raw)
To: David Gibson; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
Hi David,
On 23 February 2017 at 19:52, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Tue, Feb 21, 2017 at 09:33:37PM -0700, Simon Glass wrote:
>> Add a set of tests to cover the functionality in pylibfdt.
>>
>> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>>
>> Changes in v7:
>> - Add a test for QUIET_ALL
>>
>> Changes in v6:
>> - Adjust tests to avoid checking a hard-coded offset
>> - Use 0 instead of self.fdt.path_offset('/')
>> - Adjust the totalsize() test to compare against the file size
>> - Adjust test result processing to avoid using awk
>>
>> Changes in v5:
>> - Adjust tests to match new swig bindings
>>
>> Changes in v4:
>> - Drop tests that are no-longer applicable
>> - Add a get for getprop()
>>
>> Changes in v3:
>> - Add some more tests
>>
>> Changes in v2:
>> - Update tests for new pylibfdt
>> - Add a few more tests to increase coverage
>>
>> tests/pylibfdt_tests.py | 275 ++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/run_tests.sh | 16 ++-
>> 2 files changed, 290 insertions(+), 1 deletion(-)
>> create mode 100644 tests/pylibfdt_tests.py
>>
>> + def testPathOffset(self):
>> + """Check that we can find the offset of a node"""
>> + self.assertEquals(self.fdt.path_offset('/'), 0)
>> + self.assertTrue(self.fdt.path_offset('/subnode@1') > 0)
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
>> + self.fdt.path_offset('/wibble')
>
> Checking the error string by regexp seems a bit indirect. Wouldn't it
> be better to add new helper that checks explicitly for an FdtException
> with the right fdt error code embedded in it?
I'm not sure how to do a helper simply since there is a context
manager involved. I'll use the normal method of checking the exception
afterwards.
>
>> + self.assertEquals(self.fdt.path_offset('/wibble', QUIET_NOTFOUND),
>> + -libfdt.NOTFOUND)
>> +
>> + def testPropertyOffset(self):
>> + """Walk through all the properties in the root node"""
>> + self.assertEquals(self.fdt.first_property_offset(0), ROOT_PROPS[0])
>> + for pos in range(len(ROOT_PROPS) - 1):
>> + self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[pos]),
>> + ROOT_PROPS[pos + 1])
>> + self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[-1],
>> + QUIET_NOTFOUND),
>> + -libfdt.NOTFOUND)
>> +
>> + def testPropertyOffsetExceptions(self):
>> + """Check that exceptions are raised as expected"""
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
>> + self.fdt.next_property_offset(107)
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
>> + self.fdt.first_property_offset(107, QUIET_NOTFOUND)
>> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
>> + self.fdt.next_property_offset(107, QUIET_NOTFOUND)
>
> Isn't this a dupe of the previous statement? This also looks like a
> case where using QUIET_ALL could make it neater.
All three statements are slightly different.
The intent here is to make sure that quietening one error doesn't mask another.
Regards,
Simon
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 1/5] Add an initial Python library for libfdt
[not found] ` <CAPnjgZ3p3KfDwJwj=VfnQURL5MQ2tWXBMrb8RViK+tDhj1ikHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-03-03 4:21 ` David Gibson
0 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2017-03-03 4:21 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
[-- Attachment #1: Type: text/plain, Size: 9289 bytes --]
On Tue, Feb 28, 2017 at 10:40:16PM -0700, Simon Glass wrote:
> Hi David,
>
> On 23 February 2017 at 19:43, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Tue, Feb 21, 2017 at 09:33:36PM -0700, Simon Glass wrote:
> >> Add Python bindings for a bare-bones set of libfdt functions. These allow
> >> navigating the tree and reading node names and properties.
> >>
> >> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >> ---
> >>
> >> Changes in v7:
> >> - Add QUIET_ALL to silence all exceptions
> >>
> >> Changes in v6:
> >> - Use a tuple instead of list for the default quiert parameter
> >> - Use a tuple instead of list for QUIET_NOTFOUND
> >> - Use 'list' instead of 'tuple' for the comment in check_err_null()
> >> - Return a bytearray from getprop()
> >> - Adjust the Property constructor to accept the name and value
> >> - Use uint8_t for pylibfdt_copy_value
> >>
> >> Changes in v5:
> >> - Use a 'quiet' parameter instead of quiet versions of functions
> >> - Add a Property object to hold a property's name and value
> >> - Drop the data() and string() functions which are not needed now
> >> - Rename pylibfdt_copy_data() tp pylibfdt_copy_value()
> >> - Change order of libfdt.h inclusion to avoid #ifdef around libfdt macros
> >> - Drop fdt_offset_ptr() and fdt_getprop_namelen() from the swig interface
> >> - Use $(SWIG) to call swig from the Makefile
> >> - Review function comments
> >>
> >> Changes in v4:
> >> - Make the library less pythonic to avoid a shaky illusion
> >> - Drop classes for Node and Prop, along with associated methods
> >> - Include libfdt.h instead of repeating it
> >> - Add support for fdt_getprop()
> >> - Bring in all libfdt functions (but Python support is missing for many)
> >> - Add full comments for Python methods
> >>
> >> Changes in v3:
> >> - Make the library more pythonic
> >> - Add classes for Node and Prop along with methods
> >> - Add an exception class
> >> - Use Python to generate exeptions instead of SWIG
> >>
> >> Changes in v2:
> >> - Add exceptions when functions return an error
> >> - Correct Python naming to following PEP8
> >> - Use a class to encapsulate the various methods
> >> - Include fdt.h instead of redefining struct fdt_property
> >> - Use bytearray to avoid the SWIG warning 454
> >> - Add comments
> >>
> >> Makefile | 1 +
> >> pylibfdt/.gitignore | 3 +
> >> pylibfdt/Makefile.pylibfdt | 18 ++
> >> pylibfdt/libfdt.swig | 477 +++++++++++++++++++++++++++++++++++++++++++++
> >> pylibfdt/setup.py | 34 ++++
> >> 5 files changed, 533 insertions(+)
> >> create mode 100644 pylibfdt/.gitignore
> >> create mode 100644 pylibfdt/Makefile.pylibfdt
> >> create mode 100644 pylibfdt/libfdt.swig
> >> create mode 100644 pylibfdt/setup.py
> >>
> [...]
>
> >> +
> >> +_NUM_ERRORS = 18
> >> +
> >> +# Error codes, corresponding to FDT_ERR_... in libfdt.h
> >> +(NOTFOUND,
> >> + EXISTS,
> >> + NOSPACE,
> >> + BADOFFSET,
> >> + BADPATH,
> >> + BADPHANDLE,
> >> + BADSTATE,
> >> + TRUNCATED,
> >> + BADMAGIC,
> >> + BADVERSION,
> >> + BADSTRUCTURE,
> >> + BADLAYOUT,
> >> + INTERNAL,
> >> + BADNCELLS,
> >> + BADVALUE,
> >> + BADOVERLAY,
> >> + NOPHANDLES) = range(1, _NUM_ERRORS)
> >
> > Nit: you could actually assign QUIET_ALL right here.
>
> it means the comment stands alone, but OK.
>
> >
> >> +# Pass this as the 'quiet' parameter to return -ENOTFOUND on NOTFOUND errors,
> >> +# instead of raising an exception.
> >> +QUIET_NOTFOUND = (NOTFOUND,)
> >> +
> >> +# Pass this as the 'quiet' parameter to avoid exceptions altogether. All
> >> +# functions passed this value will return an error instead of raising an
> >> +# exception.
> >> +QUIET_ALL = range(1, _NUM_ERRORS)
> >> +
> >> +
> >> +class FdtException(Exception):
> >> + """An exception caused by an error such as one of the codes above"""
> >> + def __init__(self, err):
> >> + self.err = err
> >> +
> >> + def __str__(self):
> >> + return 'pylibfdt error %d: %s' % (self.err, fdt_strerror(self.err))
> >> +
> >> +def fdt32_to_cpu(val):
> >> + """Convert a device-tree cell value into a native integer"""
> >> + return struct.unpack("=I", struct.pack(">I", val))[0]
> >
> > I think this is conceptually wrong, although I think it will give the
> > right results. AIUI this is used when swig's built in type conversion
> > has incorrectly treated a structure with an integer member as being in
> > native format when it's actually in BE format.
> >
> > So rather than storing as BE, then loading as native, you should store
> > as native - "undoing" the incorrect load within swig - then correctly
> > loading as BE.
> >
> > Even better would be configuring swig not to make the mistake in the
> > first place, but I can accept that that's more trouble than it's
> > worth.
>
> Well I can drop this function and incorporate this code into the typemap.
Ok, that sounds good.
>
> [...]
>
> >> +
> >> + def get_property_by_offset(self, prop_offset, quiet=()):
> >> + """Obtains a property that can be examined
> >> +
> >> + Args:
> >> + prop_offset: Offset of property (e.g. from first_property_offset())
> >> + quiet: Errors to ignore (empty to raise on all errors)
> >> +
> >> + Returns:
> >> + Property object, or None if not found
> >> +
> >> + Raises:
> >> + FdtException on error (e.g. invalid prop_offset or device
> >> + tree format)
> >> + """
> >> + pdata = check_err_null(
> >> + fdt_get_property_by_offset(self._fdt, prop_offset), quiet)
> >> + if isinstance(pdata, (int)):
> >> + return pdata
> >> + name = fdt_string(self._fdt, fdt32_to_cpu(pdata[0].nameoff))
> >> + value = bytearray(fdt32_to_cpu(pdata[0].len))
> >> + pylibfdt_copy_value(value, pdata[0])
> >> + return Property(name, value)
> >
> > So.. just to check I understand. swig is translating the the return
> > value of fdt_get_property_by_offset() into:
> > If it returns NULL, an integer from the error code in *lenp
> > Otherwise, a tuple, where the second element is the length
> > from *lenp, the first element is a Python object with nameoff
> > and len attributes with those fields from the C structure?
> >
> > I still find the copy_value() thing ugly, but I think I've finally
> > understood why it's the easiest way to accomplish what you need. I
> > take it swig guarantees that if you pass that structure/object back
> > into a C function it will pass an identical pointer, not a copy of the
> > C structure (which might no longer have the data in the variable
> > length field).
>
> Yes, but I can do it with a typemap so I'll go with that for the next version.
Right, the version in the latest posting looks much better, thanks.
> [...]
>
> >> +%inline %{
> >> +
> >> +/**
> >> + * pylibfdt_copy_value() - Copy value from a property to the given buffer
> >> + *
> >> + * This is used by the Property class to place the contents of a property
> >> + * into a bytearray.
> >> + *
> >> + * @buf: Destination pointer (typically the start of the bytearray)
> >> + * @size: Number of bytes to copy (size of bytearray)
> >> + * @prop: Property to copy
> >> + */
> >> +void pylibfdt_copy_value(uint8_t *buf, size_t size, const struct fdt_property *prop)
> >> +{
> >> + memcpy(buf, prop + 1, size);
> >
> > Oh.. one more nit. I was recently reminded that memcpy(dst, src, 0)
> > isn't guaranteed to be a no-op so may invoke undefined behaviour (some
> > checkers like Coverity complain about it). So it's probably worth
> > having a check for zero length either here or in the Python caller.
>
> I think I can drop this.
>
> >
> >> +}
> >> +
> >> +%}
> >> +
> >> +%apply int *OUTPUT { int *lenp };
> >> +
> >> +/* typemap used for fdt_getprop() */
> >> +%typemap(out) (const void *) {
> >> + if (!$1)
> >> + $result = Py_None;
> >> + else
> >> + /* TODO(sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org): Can we avoid the 'arg4'? */
> >> + $result = Py_BuildValue("s#", $1, *arg4);
> >
> > What's the arg4 about? This isn't relying on swig internals which are
> > subject to change is it?
>
> Possibly, as it is not documented. I'm not sure if there is a good
> way share input parameters with an output typemap. The typemap.i file
> is quite mystical to me, but it does seem to be a rule that parameters
> are called 'arg' and numbered. If you have any better solutions I am
> all ears.
>
> I will ask on the swig-users mailing list two as I cannot seem to find
> a solution already mentioned.
>
> [...]
>
> Regards,
> Simon
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 2/5] Add tests for pylibfdt
[not found] ` <CAPnjgZ0SB4j1QyD7TrnJf=WTt3To0SvUBT5p81vzfnDMxz1aGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-03-03 4:22 ` David Gibson
0 siblings, 0 replies; 15+ messages in thread
From: David Gibson @ 2017-03-03 4:22 UTC (permalink / raw)
To: Simon Glass; +Cc: Devicetree Compiler, Benjamin Bimmermann, Ulrich Langenbach
[-- Attachment #1: Type: text/plain, Size: 4091 bytes --]
On Tue, Feb 28, 2017 at 10:40:32PM -0700, Simon Glass wrote:
> Hi David,
>
> On 23 February 2017 at 19:52, David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > On Tue, Feb 21, 2017 at 09:33:37PM -0700, Simon Glass wrote:
> >> Add a set of tests to cover the functionality in pylibfdt.
> >>
> >> Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >> ---
> >>
> >> Changes in v7:
> >> - Add a test for QUIET_ALL
> >>
> >> Changes in v6:
> >> - Adjust tests to avoid checking a hard-coded offset
> >> - Use 0 instead of self.fdt.path_offset('/')
> >> - Adjust the totalsize() test to compare against the file size
> >> - Adjust test result processing to avoid using awk
> >>
> >> Changes in v5:
> >> - Adjust tests to match new swig bindings
> >>
> >> Changes in v4:
> >> - Drop tests that are no-longer applicable
> >> - Add a get for getprop()
> >>
> >> Changes in v3:
> >> - Add some more tests
> >>
> >> Changes in v2:
> >> - Update tests for new pylibfdt
> >> - Add a few more tests to increase coverage
> >>
> >> tests/pylibfdt_tests.py | 275 ++++++++++++++++++++++++++++++++++++++++++++++++
> >> tests/run_tests.sh | 16 ++-
> >> 2 files changed, 290 insertions(+), 1 deletion(-)
> >> create mode 100644 tests/pylibfdt_tests.py
> >>
>
> >> + def testPathOffset(self):
> >> + """Check that we can find the offset of a node"""
> >> + self.assertEquals(self.fdt.path_offset('/'), 0)
> >> + self.assertTrue(self.fdt.path_offset('/subnode@1') > 0)
> >> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.NOTFOUND)):
> >> + self.fdt.path_offset('/wibble')
> >
> > Checking the error string by regexp seems a bit indirect. Wouldn't it
> > be better to add new helper that checks explicitly for an FdtException
> > with the right fdt error code embedded in it?
>
> I'm not sure how to do a helper simply since there is a context
> manager involved. I'll use the normal method of checking the exception
> afterwards.
Ah.. yeah.. Python with statements always confuse me.
>
> >
> >> + self.assertEquals(self.fdt.path_offset('/wibble', QUIET_NOTFOUND),
> >> + -libfdt.NOTFOUND)
> >> +
> >> + def testPropertyOffset(self):
> >> + """Walk through all the properties in the root node"""
> >> + self.assertEquals(self.fdt.first_property_offset(0), ROOT_PROPS[0])
> >> + for pos in range(len(ROOT_PROPS) - 1):
> >> + self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[pos]),
> >> + ROOT_PROPS[pos + 1])
> >> + self.assertEquals(self.fdt.next_property_offset(ROOT_PROPS[-1],
> >> + QUIET_NOTFOUND),
> >> + -libfdt.NOTFOUND)
> >> +
> >> + def testPropertyOffsetExceptions(self):
> >> + """Check that exceptions are raised as expected"""
> >> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> >> + self.fdt.next_property_offset(107)
> >> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> >> + self.fdt.first_property_offset(107, QUIET_NOTFOUND)
> >> + with self.assertRaisesRegexp(FdtException, get_err(libfdt.BADOFFSET)):
> >> + self.fdt.next_property_offset(107, QUIET_NOTFOUND)
> >
> > Isn't this a dupe of the previous statement? This also looks like a
> > case where using QUIET_ALL could make it neater.
>
> All three statements are slightly different.
>
> The intent here is to make sure that quietening one error doesn't mask another.
Ah.. ok. Maybe a comment to that effect - from the test name, I
thought it was just about the underlying function returning the right
error codes.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-03-03 4:22 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-22 4:33 [PATCH v7 0/5] Introduce Python bindings for libfdt Simon Glass
[not found] ` <20170222043340.17008-1-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-22 4:33 ` [PATCH v7 1/5] Add an initial Python library " Simon Glass
[not found] ` <20170222043340.17008-2-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-24 2:43 ` David Gibson
[not found] ` <20170224024317.GI17615-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2017-03-01 5:40 ` Simon Glass
[not found] ` <CAPnjgZ3p3KfDwJwj=VfnQURL5MQ2tWXBMrb8RViK+tDhj1ikHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-03 4:21 ` David Gibson
2017-02-22 4:33 ` [PATCH v7 2/5] Add tests for pylibfdt Simon Glass
[not found] ` <20170222043340.17008-3-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-24 2:52 ` David Gibson
[not found] ` <20170224025254.GJ17615-K0bRW+63XPQe6aEkudXLsA@public.gmane.org>
2017-03-01 5:40 ` Simon Glass
[not found] ` <CAPnjgZ0SB4j1QyD7TrnJf=WTt3To0SvUBT5p81vzfnDMxz1aGA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-03 4:22 ` David Gibson
2017-02-22 4:33 ` [PATCH v7 3/5] Mention pylibfdt in the documentation Simon Glass
[not found] ` <20170222043340.17008-4-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-24 2:53 ` David Gibson
2017-02-22 4:33 ` [PATCH v7 4/5] Adjust libfdt.h to work with swig Simon Glass
[not found] ` <20170222043340.17008-5-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-24 2:55 ` David Gibson
2017-02-22 4:33 ` [PATCH v7 5/5] Build pylibfdt as part of the normal build process Simon Glass
[not found] ` <20170222043340.17008-6-sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2017-02-24 2:55 ` David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).