Re: [PATCH v3 7/8] ACPI: property: Add support for parsing buffer property UUID

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Andy,

On Wed, May 25, 2022 at 08:44:11PM +0300, Andy Shevchenko wrote:
> On Wed, May 25, 2022 at 04:01:22PM +0300, Sakari Ailus wrote:
> > Add support for newly added buffer property UUID, as defined in the DSD
> > guide.
> 
> ...
> 
> > +	if (check_mul_overflow((size_t)properties->package.count,
> 
> Why do you need casting? Any issues on 32-bit compilation?

I think it can be removed. I'll see.

> 
> Looking at the below code snippets, it seems you also can have a local
> copy with needed type and use it everywhere (as outer_package_count or so).
> But first question first...

u32 should be fine.

> 
> > +			       sizeof(*package) + sizeof(void *),
> > +			       &alloc_size) ||
> > +	    check_add_overflow(sizeof(*props) + sizeof(*package), alloc_size,
> > +			       &alloc_size)) {
> > +		acpi_handle_warn(handle,
> > +				 "can't allocate memory for %u buffer props",
> > +				 properties->package.count);
> > +		return;
> > +	}
> 
> ...
> 
> > +		if (ACPI_FAILURE(status)) {
> > +			acpi_handle_warn(handle,
> > +					 "can't evaluate \"%s\" as buffer\n",
> > +					 obj->string.pointer);
> 
> I'm wondering if better to use %*pE here to show the full data of the buffer.

The string is supposed to be printable. If it's not, something is seriously
wrong. There's no harm though to prepare for this possibility. It'll make
backslashes printing twice but that is hardly an issue in practice.

> 
> > +			continue;
> > +		}
> 

-- 
Sakari Ailus