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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux