Hi Andy,
On Mon, Jan 23, 2023 at 07:25:25PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 23, 2023 at 06:07:43PM +0200, Sakari Ailus wrote:
> > On Mon, Jan 23, 2023 at 05:07:09PM +0200, Andy Shevchenko wrote:
> > > On Mon, Jan 23, 2023 at 03:46:11PM +0200, Sakari Ailus wrote:
>
> ...
>
> > > > + memcpy(inst->remote_name, csi2->resource_source.string_ptr,
> > > > + csi2->resource_source.string_length);
> > >
> > > Why don't we use strscpy()? Is it really strings? Or is it some abuse of
> > > the ACPI object type?
> >
> > I didn't find a guarantee it would be nil terminated. Albeit I'm fine
> > switching to strscpy() if there's such a guarantee.
>
> Following this logic neither memcpy() (and especially memcpy()!) call
> guarantees that. But hold on, have you actually read strscpy() documentation?
Yes. And there is such a guarantee, too. The string_length contains the
length of the string, including the terminating nil character. I have no
problem with strscpy() but it won't affect the end result in any way. :-)
>
> ...
>
> > > > + list_replace(&inst_context.res_list, &csi2->buses);
> > > > + list_add(&csi2->list, list);
> > >
> > > Hmm... Can list_swap() be used here?
> >
> > We're replacing an entry in a list and then adding an entry to another. How
> > would you use list_swap() here?
>
> I see, so it is about two different lists.
>
> ...
>
> > > > + /*
> > > > + * Figure out how much temporary storage we need for counting
> > > > + * connections in each device.
> > > > + */
> > > > + list_for_each_entry(csi2, &crs_csi2_handles, list) {
> > > > + struct crs_csi2_instance *inst;
> > > > +
> > > > + handle_count++;
> > >
> > > > + list_for_each_entry(inst, &csi2->buses, list)
> > > > + handle_count++;
> > >
> > > list_count_nodes()?
> >
> > Are you suggesting adding a new list API function or using one that's not
> > in the linux-acpi/testing branch yet?
>
> It's in USB tree.
> I'm fine if you switch your code later on.
Ack, I'll post a patch once this hits the linux-acpi tree.
>
> > > > + }
>
--
Kind regards,
Sakari Ailus
