On Fri, May 27, 2022 at 10:59 PM Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
>
> Hi Rafael,
>
> On Fri, May 27, 2022 at 07:04:39PM +0200, Rafael J. Wysocki wrote:
> > On Fri, May 27, 2022 at 11:02 AM Sakari Ailus
> > <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> > >
> > > Hi Rafael,
> > >
> > > On Thu, May 26, 2022 at 09:19:17PM +0200, Rafael J. Wysocki wrote:
> > > > > +static int acpi_tie_nondev_subnodes(struct acpi_device_data *data)
> > > > > +{
> > > > > + struct acpi_data_node *dn;
> > > > > +
> > > > > + list_for_each_entry(dn, &data->subnodes, sibling) {
> > > > > + acpi_status status;
> > > > > + int ret;
> > > > > +
> > > > > + status = acpi_attach_data(dn->handle, acpi_nondev_subnode_tag, dn);
> > > > > + if (ACPI_FAILURE(status)) {
> > > > > + acpi_handle_err(dn->handle, "Can't tag data node\n");
> > > > > + return 0;
> > > > > + }
> > > > > +
> > > > > + ret = acpi_tie_nondev_subnodes(&dn->data);
> > > > > + if (ret)
> > > > > + return ret;
> > > >
> > > > Is it actually possible that this returns anything different from 0?
> > >
> > > acpi_attach_data() involves allocating memory and resolving a reference.
> > > Both can fail.
> >
> > Yes, they can, but the value returned by acpi_attach_data() is
> > effectively ignored above (except for printing the error message,
> > which BTW could be "info" and provide more information).
>
> Oops. Good point.
>
> I intended this to return an error here. I don't have strong opinion on
> which way to go though. How about changing that to -ENOMEM?
It might as well return bool and let the caller worry about the error handling.
> I think this is basically a decision on whether any subnodes could be
> referenced if ore or more of them could not. I don't expect this to happen
> in practice.
So is having a partial description of something useful? I guess it
may or may not be, depending on the use case.
If there's any use case in which it may be useful, I would ignore the
attach errors and address missing stuff elsewhere.
