Re: [PATCH v6 4/5] device property: Constify fwnode_handle_get()

[Rafael J. Wysocki]

On Thu, Apr 14, 2022 at 3:09 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, Apr 14, 2022 at 12:47:20AM +0300, Sakari Ailus wrote:
> > On Wed, Apr 13, 2022 at 09:23:17PM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 13, 2022 at 09:19:28PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Apr 13, 2022 at 08:10:22PM +0200, Rafael J. Wysocki wrote:
> > > > > On Fri, Apr 8, 2022 at 8:49 PM Andy Shevchenko
> > > > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> ...
>
> > > > > > -struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode)
> > > > > > +struct fwnode_handle *fwnode_handle_get(const struct fwnode_handle *fwnode)
> > > > > >  {
> > > >
> > > > > >         if (!fwnode_has_op(fwnode, get))
> > > > > >                 return fwnode;
> > > >
> > > > ^^^^, so it needs a casting, but then we have to comment why is so.
> > >
> > > Note, it means that the fwnode parameter either invalid or has no given option.
> > > It's not a problem to drop casting in the first case, but the second one should
> > > be justified and Sakari wants to be sure that the initial container is not
> > > const, which seems can't be achieved even with the original code.
> >
> > I wonder if I'm missing something. The fwnode argument originally was not
> > const here.
>
> Yes, and our discussion went to the direction of what const qualifier implies
> here. I assume that the const means that we do not modify the fwnode object,
> while its container is another story which we have no influence on. You, if
> I read your messages correctly, insisting that const here implies that the
> container object is const as well.
>
> Reading current implementation I see now, that with children APIs we have
> two pointers passed, while with parent APIs only a single one. In children
> API due to above is easy to use const qualifier for the first argument.
> Parent APIs missed that and hence have this problem that we can't constify
> their parameters.
>
> to_of_node() expects const parameter while returns non-const container.
> Is it a subtle issue there? (I believe it should be consistent then)

This is fine AFAICS.

The const parameter means that to_of_node() will not update the memory
pointed to by it, which is correct.

The value returned by it may be used by its caller in whatever way
they like, because the caller has no obligation to preserve the memory
pointed to by it, unless they've also received that pointer with the
const qualifier, but then they need to know how it is related to the
to_of_node() return value and what to do with it.

IOW, to_of_node() has no information on its caller's obligations with
respect to the memory pointed to by its argument, so it is OK for it
to return a non-const result.  Moreover, if it had done otherwise, it
might have created an obligation for the caller that didn't exist
before.

> This patch and the followed one can be moved without understanding why
> we need the non-const parameter there.

I'm not sure what you mean here, sorry.