Re: [Qemu-arm] [Qemu-devel] [RESEND RFC 4/6] device_tree: introduce qemu_fdt_getprop_optional

[Eric Auger]

Hi Peter,
On 11/27/2015 08:38 PM, Peter Crosthwaite wrote:
> On Thu, Nov 19, 2015 at 7:22 AM, Eric Auger <address@hidden> wrote:
>> Current qemu_fdt_getprop exits if the property is not found. It is
>> sometimes needed to read an optional property, in which case we do
>> not wish to exit but simply returns a null value.
>>
>> This is what this new qemu_fdt_getprop_optional function does.
>>
>> Signed-off-by: Eric Auger <address@hidden>
>> ---
>>  device_tree.c                | 17 +++++++++++++++++
>>  include/sysemu/device_tree.h |  2 ++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index f184e3c..a318683 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -280,6 +280,23 @@ const void *qemu_fdt_getprop(void *fdt, const char 
>> *node_path,
>>      return r;
>>  }
>>
>> +const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,
>> +                             const char *property, bool optional, int *lenp)
>> +{
>> +    int len;
>> +    const void *r;
>> +    if (!lenp) {
>> +        lenp = &len;
>> +    }
>> +    r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp);
>> +    if (!r && !optional) {
>> +        error_report("%s: Couldn't get %s/%s: %s", __func__,
>> +                     node_path, property, fdt_strerror(*lenp));
>> +        exit(1);
>> +    }
>> +    return r;
>> +}
>> +
> 
> The real problem here is that the device-tree API is self-asserting.
> This looks the old _nofail system that we removed but just named in
> reverse. The correct solution here is to use the Error API properly.
> Convert qemu_fdt_getprop to accept an Error **, and all existing users
> are converted to pass &error_fatal. This will preserve existing
> behaviour. Then to use the API with your optional semantic your pass
> NULL for the Error **.
OK I will implement your proposal. Thanks for your time.

Best Regards

Eric
> 
> Regards,
> Peter
> 
>>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
>>                                 const char *property)
>>  {
>> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
>> index f9e6e6e..10cbe8e 100644
>> --- a/include/sysemu/device_tree.h
>> +++ b/include/sysemu/device_tree.h
>> @@ -34,6 +34,8 @@ int qemu_fdt_setprop_phandle(void *fdt, const char 
>> *node_path,
>>                               const char *target_node_path);
>>  const void *qemu_fdt_getprop(void *fdt, const char *node_path,
>>                               const char *property, int *lenp);
>> +const void *qemu_fdt_getprop_optional(void *fdt, const char *node_path,
>> +                             const char *property, bool optional, int 
>> *lenp);
>>  uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
>>                                 const char *property);
>>  uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
>> --
>> 1.8.3.2
>>
>>