Hi David, On 4 June 2018 at 20:10, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > On Wed, May 23, 2018 at 10:03:41PM -0600, Simon Glass wrote: >> Add support for fdt_open_init() and fdt_create_empty_tree() from the >> Python library. >> >> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx> >> --- >> >> pylibfdt/libfdt.i | 33 +++++++++++++++++++++++++++++++++ >> tests/pylibfdt_tests.py | 18 +++++++++++++++++- >> 2 files changed, 50 insertions(+), 1 deletion(-) >> >> diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i >> index c0cd096..129f000 100644 >> --- a/pylibfdt/libfdt.i >> +++ b/pylibfdt/libfdt.i >> @@ -432,6 +432,39 @@ class Fdt: >> return pdata >> return Property(pdata[0], pdata[1]) >> >> + @staticmethod >> + def create_empty_tree(size, quiet=()): >> + """Create an empty device tree ready for use >> + >> + Args: >> + size: Size of device tree in bytes >> + >> + Returns: >> + Fdt object containing the device tree >> + """ >> + data = bytearray(size) >> + err = check_err(fdt_create_empty_tree(data, size), quiet) >> + if err: >> + return err >> + return Fdt(data) >> + >> + def open_into(self, size, quiet=()): > > So, usually keeping the same names here as in C is a good idea. > However, when Python is managing the buffer, rather than the caller > having to thing about it explicitly the "open_into" name is kind of > misleading. Yes it is I suppose. > > I can see two ways to go: > 1) Call it resize() instead (which would probably need to do > either fdt_open_into() or fdt_resize) depending on if you're in > sw or rw mode) This seems best, although not that the sw interface uses a different class entirely, which already has a resize() method. > 2) Change the interface so that rather than changing the buffer > "in place", it leaves the existing Fdt object as is and returns > a new Fdt object with the "opened" buffer. This is a bit ugly, since the caller then has to manage a new Fdt appearing and the old one going away. It breaks the idea of encapsulation of the Fdt operations I think. This is necessary in C (unless we changed whole interface to use some sort of struct to hold the FDT pointer) but not in Python. So I much prefer option 1. > > >> + """Move the device tree into a larger or smaller space >> + >> + This creates a new device tree of size @size and moves the existing >> + device tree contents over to that. It can be used to create more space >> + in a device tree. >> + >> + Args: >> + size: Required new size of device tree in bytes >> + """ >> + fdt = bytearray(size) >> + fdt[:len(self._fdt)] = self._fdt > > You don't need this - fdt_open_into() will already copy data from the > old buffer into the new one. OK, will drop. > >> + err = check_err(fdt_open_into(self._fdt, fdt, size), quiet) >> + if err: >> + return err >> + self._fdt = fdt >> + [...] Regards, Simon -- To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
