On 2023-07-17 11:47 AM, Andy Shevchenko wrote:
On Mon, Jul 17, 2023 at 10:29:17AM +0200, Cezary Rojewski wrote:
On 2023-07-12 5:48 PM, Andy Shevchenko wrote:
On Wed, Jul 12, 2023 at 11:10:48AM +0200, Cezary Rojewski wrote:
...
+struct acpi_nhlt_endpoint *
+acpi_nhlt_find_endpoint(const struct acpi_table_nhlt *tb,
+ int link_type, int dev_type, int dir, int bus_id)
+{
+ struct acpi_nhlt_endpoint *ep;
+ if (!tb)
+ return ERR_PTR(-EINVAL);
Just wondering how often we will have a caller that supplies NULL for tb.
Depends on kernel's philosophy on public API. In general, public API should
defend themselves from harmful and illegal callers. However, in low level
areas 'illegal' is sometimes mistaken with illogical. In such cases double
safety can be dropped.
What do you put under "public"? ABI? Or just internally available API?
If the latter, we don't do defensive programming there, we usually just
add NULL(invalid data)-awareness to the free()-like functions.
Or, perhaps you were discussing return value here and ERR_PTR(-EINVAL) could
be replaced by something else or even NULL.
I prefer to get rid of those.
Decided to do some manual tests on more exotic setups that are not part
of our daily CI/CD routine and, completely getting rid of those ifs
causes problems. Those setups are part of the market, expose DSP
capabilities but have invalid BIOS configurations.
Rather than just bringing back the if-statement, different solution came
to my mind:
static struct acpi_table_nhlt empty_nhlt = {
.header = {
.signature = ACPI_SIG_NHLT,
},
};
struct acpi_table_nhlt *acpi_gbl_NHLT;
EXPORT_SYMBOL_GPL(acpi_gbl_NHLT);
acpi_status acpi_nhlt_get_gbl_table(void)
{
acpi_status status;
status = acpi_get_table(ACPI_SIG_NHLT, 0, (struct acpi_table_header
**)(&acpi_gbl_NHLT));
if (!acpi_gbl_NHLT)
acpi_gbl_NHLT = &empty_nhlt;
return status;
}
EXPORT_SYMBOL_GPL(acpi_nhlt_get_gbl_table);
What do you think?
