| Message ID | 20250203194629.3731895-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Accepted |
| Commit | ab930483eca9f3e816c35824b5868599af0c61d7 |
| Headers | show |
| Series | [v1,1/1] ACPI: property: Fix return value for nval == 0 in acpi_data_prop_read() | expand |
On Mon, Feb 3, 2025 at 8:47 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > While analysing code for software and OF node for the corner case when > caller asks to read zero items in the supposed to be an array of values > I found that ACPI behaves differently to what OF does, i.e. > > 1. It returns -EINVAL when caller asks to read zero items from integer > array, while OF returns 0, if no other errors happened. > > 2. It returns -EINVAL when caller asks to read zero items from string > array, while OF returns -ENODATA, if no other errors happened. > > Amend ACPI implementation to follow what OF does. > > Fixes: b31384fa5de3 ("Driver core: Unified device properties interface for platform firmware") > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/acpi/property.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c > index 1144c2368d89..7d7f4974c5b1 100644 > --- a/drivers/acpi/property.c > +++ b/drivers/acpi/property.c > @@ -1189,8 +1189,6 @@ static int acpi_data_prop_read(const struct acpi_device_data *data, > return -EOVERFLOW; > break; > } > - if (nval == 0) > - return -EINVAL; > > if (obj->type == ACPI_TYPE_BUFFER) { > if (proptype != DEV_PROP_U8) > @@ -1214,9 +1212,10 @@ static int acpi_data_prop_read(const struct acpi_device_data *data, > ret = acpi_copy_property_array_uint(items, (u64 *)val, nval); > break; > case DEV_PROP_STRING: > - ret = acpi_copy_property_array_string( > - items, (char **)val, > - min_t(u32, nval, obj->package.count)); > + nval = min_t(u32, nval, obj->package.count); > + if (nval == 0) > + return -ENODATA; > + ret = acpi_copy_property_array_string(items, (char **)val, nval); > break; > default: > ret = -EINVAL; > -- Applied as 6.14-rc material, thanks!
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index 1144c2368d89..7d7f4974c5b1 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -1189,8 +1189,6 @@ static int acpi_data_prop_read(const struct acpi_device_data *data, return -EOVERFLOW; break; } - if (nval == 0) - return -EINVAL; if (obj->type == ACPI_TYPE_BUFFER) { if (proptype != DEV_PROP_U8) @@ -1214,9 +1212,10 @@ static int acpi_data_prop_read(const struct acpi_device_data *data, ret = acpi_copy_property_array_uint(items, (u64 *)val, nval); break; case DEV_PROP_STRING: - ret = acpi_copy_property_array_string( - items, (char **)val, - min_t(u32, nval, obj->package.count)); + nval = min_t(u32, nval, obj->package.count); + if (nval == 0) + return -ENODATA; + ret = acpi_copy_property_array_string(items, (char **)val, nval); break; default: ret = -EINVAL;
While analysing code for software and OF node for the corner case when caller asks to read zero items in the supposed to be an array of values I found that ACPI behaves differently to what OF does, i.e. 1. It returns -EINVAL when caller asks to read zero items from integer array, while OF returns 0, if no other errors happened. 2. It returns -EINVAL when caller asks to read zero items from string array, while OF returns -ENODATA, if no other errors happened. Amend ACPI implementation to follow what OF does. Fixes: b31384fa5de3 ("Driver core: Unified device properties interface for platform firmware") Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/acpi/property.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)