On Thu, Nov 17, 2016 at 03:53:22PM -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.
Nice idea. Some nits in the details..
>
> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
> ---
>
> pylibfdt/.gitignore | 3 +
> pylibfdt/Makefile.pylibfdt | 21 ++++++
> pylibfdt/libfdt.swig | 157 +++++++++++++++++++++++++++++++++++++++++++++
> pylibfdt/setup.py | 34 ++++++++++
> 4 files changed, 215 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/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..fbdbca5
> --- /dev/null
> +++ b/pylibfdt/Makefile.pylibfdt
> @@ -0,0 +1,21 @@
> +# Makefile.pylibfdt
> +#
> +# This is not a complete Makefile of itself. Instead, it is designed to
> +# be easily embeddable into other systems of Makefiles.
So, Makefile.libfdt is explicitly designed for easily embedding libfdt
in other projecs - even ones with weird build environments, like
bootloaders. It's less clear that that's valuable for Python
wrappers.
> +#
> +
> +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..91887da
> --- /dev/null
> +++ b/pylibfdt/libfdt.swig
> @@ -0,0 +1,157 @@
> +/*
> + * pylibfdt - Flat Device Tree manipulation in Python
> + * Copyright (C) 2016 Google, Inc.
> + * Written by Simon Glass <sjg@xxxxxxxxxxxx>
> + *
> + * 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 %{
> +
> +def Raise(errnum):
> + raise ValueError('Error %s' % fdt_strerror(errnum))
So, first, I believe this and several things below break normal Python
capitalization conventions for functions (capital-followed-by-lower
generally indicates a class).
More importantly, wouldn't it make more sense to have an Exception
subclass for libfdt errors, rather than shuffling everything through
ValueError?
> +def Name(fdt, offset):
> + name, len = fdt_get_name(fdt, offset)
> + return name
> +
> +def String(fdt, offset):
> + offset = fdt32_to_cpu(offset)
> + name = fdt_string(fdt, offset)
> + return name
> +
> +def fdt32_to_cpu(val):
> + return struct.unpack("=I", struct.pack(">I", val))[0]
> +
> +def Data(prop):
> + set_prop(prop)
> + return get_prop_data()
> +%}
> +
> +%include "typemaps.i"
> +%include "cstring.i"
> +
> +%typemap(in) void* = char*;
> +
> +typedef int fdt32_t;
> +
> +struct fdt_property {
> + fdt32_t tag;
> + fdt32_t len;
> + fdt32_t nameoff;
> + char data[0];
> +};
Can't you include this directly from fdt.h? That header is guaranteed
never to have actual function definitions, just data structures and
constants.
> +/*
> + * This is a work-around since I'm not sure of a better way to copy out the
> + * contents of a string. This is used in dtoc/GetProps(). The intent is to
> + * pass in a pointer to a property and access the data field at the end of
> + * it. Ideally the Data() function above would be able to do this directly,
> + * but I'm not sure how to do that. Needs another look.
> + */
> +#pragma SWIG nowarn=454
> +%inline %{
> + static struct fdt_property *cur_prop;
> +
> + void set_prop(struct fdt_property *prop) {
> + cur_prop = prop;
> + }
Eugh... a global variable making this totally non thread safe. You
really need a better solution. Property values you should be able to
represent nicely as Python strings (since unlike C strings, those can
include embedded \0). Uh.. I guess that's 'bytes' for Python3
Of course, in the other direction, node and property names can't
include \0, so you probably need some sort of exception if a Python
string which includes them is passed in.
> +%}
> +
> +%cstring_output_allocate_size(char **s, int *sz, free(*$1));
> +%inline %{
> + void get_prop_data(char **s, int *sz) {
> + *sz = fdt32_to_cpu(cur_prop->len);
> + *s = (char *)malloc(*sz);
> + if (!*s)
> + *sz = 0;
> + else
> + memcpy(*s, cur_prop + 1, *sz);
> + }
> +%}
> +
> +%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);
> +}
> +
> +int fdt_path_offset(const void *fdt, const char *path);
> +int fdt_first_property_offset(const void *fdt, int nodeoffset);
> +int fdt_next_property_offset(const void *fdt, int offset);
> +const char *fdt_get_name(const void *fdt, int nodeoffset, int *OUTPUT);
> +const char *fdt_string(const void *fdt, int stroffset);
> +const struct fdt_property *fdt_get_property_by_offset(const void *fdt,
> + int offset,
> + int *OUTPUT);
> +const char *fdt_strerror(int errval);
> +int fdt_first_subnode(const void *fdt, int offset);
> +int fdt_next_subnode(const void *fdt, int offset);
> +
> +%typemap(in) (void *) {
> + if (!PyByteArray_Check($input)) {
> + SWIG_exception_fail(SWIG_TypeError, "in method '" "$symname" "', argument "
> + "$argnum"" of type '" "$type""'");
> + }
> + $1 = PyByteArray_AsString($input);
> +}
> +
> +int fdt_delprop(void *fdt, int nodeoffset, const char *name);
> +
> +int fdt_pack(void *fdt);
> +
> +int fdt_totalsize(const void *fdt);
> +int fdt_off_dt_struct(const void *fdt);
> 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:
signature.asc
Description: PGP signature
