Re: [PATCH v2 2/2] pylibfdt: Allow reading integer values from properties

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hi David,

On 13 September 2017 at 06:42, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Aug 31, 2017 at 04:42:00AM -0600, Simon Glass wrote:
>> Extend the Properties class with some functions to read a single integer
>> property. Add a new getprop_obj() function to return a Property object
>> instead of the raw data.
>>
>> This suggested approach can be extended to handle other types, as well as
>> arrays.
>>
>> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx>
>
> Sorry again I've taken so long to respond.
>
> This way of doing it really seems awkward to me.  You're introducing a
> clunky variant getter just to wrap the bytearray in an object,
> essentially just to get conversion methods.
>
> It seems to be simpler to just have as_cell() and whatnot as plain
> functions that go straight from bytestring/bytearray to an int or
> whatever.

I don't really understand this. Which class would as_cell() be a member of?

Surely it makes sense for these functions to be members of the
Property class, since they act on properties.

You seem to be suggesting that we have things like:

def as_cell(self, nodeoffset, prop_name, quiet=())

so as to bypass the concept of a Property. We already have the
Property object and I created it specifically to encapsulate a
device-tree property. To me this is preferable to adding lots of
property-access methods to the Fdt class.

I suppose you could argue that each property probably only gets used
once, so we end up with:

    getprop_obj(nodeoffset, prop_name).as_int32()

but that still seems better to me than:

    as_int32(nodeoffset, prop_name)

>
>
> Longer term, I did have an idea that might resolve the
> propery-as-object or property-as-bytestring conundrum a bit more
> satisfactorily.  But, I'm not entirely sure how you'd go about
> implementing it with swig in the picture: it should be possible in
> (modern) Python to make a Property object that's a sub-class of the
> (byte)string type.  That way it can be used directly as though it were
> a bytestring, but you can also add properties to do extra stuff.

Yes good idea. I'll add a patch for that.

>
> [Which raises a minor detail, most of the functions which return
> bytearray()s now should probably be returning strings - there doesn't
> seem to be a reason to return a mutable object]

I added a patch for the one I found. The way SWIG is set up I have to
pass bytearrays to the libfdt functions though.

[...]

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



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux