Patchwork [04/31] dt: temporarily disable subtree creation failure check

login
register
mail settings
Submitter Alexander Graf
Date June 5, 2012, 11:52 p.m.
Message ID <1338940402-28502-5-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/163226/
State New
Headers show

Comments

Alexander Graf - June 5, 2012, 11:52 p.m.
Usually we want to know when creating a subtree fails. However, while
introducing this patch set we have to modify the device tree and some
times have the code to create a subtree in both the binary tree and
the dynamically created tree.

So ignore failures about this for now and enable them once we got rid
of the binary device tree.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Peter A. G. Crosthwaite - June 6, 2012, 5:32 a.m.
On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> Usually we want to know when creating a subtree fails. However, while
> introducing this patch set we have to modify the device tree and some
> times have the code to create a subtree in both the binary tree and
> the dynamically created tree.
> 
> So ignore failures about this for now and enable them once we got rid
> of the binary device tree.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@petalogix.com>

> ---
>  device_tree.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 8e9262c..6cbc5af 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -203,11 +203,13 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
>      }
>  
>      retval = fdt_add_subnode(fdt, parent, basename);
> +#if 0
>      if (retval < 0) {
>          fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
>                  fdt_strerror(retval));
>          exit(1);
>      }
> +#endif

Doesnt this illustrate a failure in this functions return path in the
first place?? Should this check be removed altogether and an error code
returned to the caller? That way callers (like you platform under
construction) can choose to ignore/act-on the error as appropriate.

>  
>      g_free(dupname);
>      return retval;
Alexander Graf - June 6, 2012, 3:58 p.m.
On 06/06/2012 07:32 AM, Peter Crosthwaite wrote:
> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>> Usually we want to know when creating a subtree fails. However, while
>> introducing this patch set we have to modify the device tree and some
>> times have the code to create a subtree in both the binary tree and
>> the dynamically created tree.
>>
>> So ignore failures about this for now and enable them once we got rid
>> of the binary device tree.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
> Reviewed-by: Peter Crosthwaite<peter.crosthwaite@petalogix.com>
>
>> ---
>>   device_tree.c |    2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index 8e9262c..6cbc5af 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -203,11 +203,13 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
>>       }
>>
>>       retval = fdt_add_subnode(fdt, parent, basename);
>> +#if 0
>>       if (retval<  0) {
>>           fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
>>                   fdt_strerror(retval));
>>           exit(1);
>>       }
>> +#endif
> Doesnt this illustrate a failure in this functions return path in the
> first place?? Should this check be removed altogether and an error code
> returned to the caller? That way callers (like you platform under
> construction) can choose to ignore/act-on the error as appropriate.

That would mean that we'd have to check every single fdt helper call for 
its return value. At the end of the day, we'd have an insane amount of 
code just for checking for potential errors, making the actual 
functional code unreadable.

So no, the one thing that these helpers buy you vs calling libfdt 
manually is exactly the exit(1) :).


Alex

Patch

diff --git a/device_tree.c b/device_tree.c
index 8e9262c..6cbc5af 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -203,11 +203,13 @@  int qemu_devtree_add_subnode(void *fdt, const char *name)
     }
 
     retval = fdt_add_subnode(fdt, parent, basename);
+#if 0
     if (retval < 0) {
         fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
                 fdt_strerror(retval));
         exit(1);
     }
+#endif
 
     g_free(dupname);
     return retval;