Re: [PATCH 1/3] of: base: add support to get machine compatible string

From: Sudeep Holla
Date: Wed Nov 23 2016 - 07:10:15 EST

On 23/11/16 11:47, Sekhar Nori wrote:
On Wednesday 23 November 2016 03:35 PM, Sudeep Holla wrote:

On 23/11/16 07:49, Sekhar Nori wrote:
On Tuesday 22 November 2016 09:16 PM, Sudeep Holla wrote:
Hi Sekhar,

On 22/11/16 15:06, Sekhar Nori wrote:
Hi Sudeep,

On Tuesday 22 November 2016 04:23 PM, Sudeep Holla wrote:

On 22/11/16 10:41, Bartosz Golaszewski wrote:
Add a function allowing to retrieve the compatible string of the root
node of the device tree.

Rob has queued [1] and it's in -next today. You can reuse that if you
are planning to target this for v4.11 or just use open coding in your
driver for v4.10 and target this move for v4.11 to avoid cross tree
dependencies as I already mentioned in your previous thread.

I dont have your original patch in my mailbox, but I wonder if
returning a pointer to property string for a node whose reference has
already been released is safe to do? Probably not an issue for the root
node, but still feels counter-intuitive.

I am not sure if I understand the issue here. Are you referring a case
where of_root is freed ?

Yes, right, thats what I was hinting at. Since you are giving up the
reference to the device node before the function returns, the user can
be left with a dangling reference.

Yes I agree.

So, the if(!of_node_get()) is just an expensive NULL pointer check. I think
it is better to be explicit about it by not using of_node_get/put() at all.
How about:

Are we planning to use this in any time sensitive paths? Anyways I am
fine removing them.

+int of_machine_get_model_name(const char **model)
+ int error;
+ if (!of_root)
+ return -EINVAL;
+ error = of_property_read_string(of_root, "model", model);
+ if (error)
+ error = of_property_read_string_index(of_root, "compatible",
+ 0, model);
+ return error;

I know the patch is already in -next so I guess it depends on how strongly
Rob feels about this.

Frank expressed his concerns and it may be reverted.