Re: [PATCH v2 2/8] ACPI: property: Parse _CRS CSI-2 descriptor

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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?

...

> > > +	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.

> > > +	}

-- 
With Best Regards,
Andy Shevchenko