| Message ID | 20220902074144.2209674-1-floridsleeves@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] drivers/acpi/acpi_video: check return value of acpi_get_parent() | expand |
On Fri, Sep 2, 2022 at 9:42 AM Li Zhong <floridsleeves@gmail.com> wrote: > > Check return status of acpi_get_parent() to confirm whether it fails. > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > --- > drivers/acpi/acpi_video.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c > index 5cbe2196176d..5fca9a39b1a4 100644 > --- a/drivers/acpi/acpi_video.c > +++ b/drivers/acpi/acpi_video.c > @@ -1,4 +1,4 @@ > -// SPDX-License-Identifier: GPL-2.0-or-later > + // SPDX-License-Identifier: GPL-2.0-or-later This change surely is not intended? > /* > * video.c - ACPI Video Driver > * > @@ -1753,6 +1753,7 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) > int result; > static int count; > char *name; > + acpi_status status; > > result = acpi_video_init_brightness(device); > if (result) > @@ -1766,8 +1767,9 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) > return; > count++; > > - acpi_get_parent(device->dev->handle, &acpi_parent); > - > + status = acpi_get_parent(device->dev->handle, &acpi_parent); > + if (ACPI_FAILURE(status)) > + return; But device->dev->handle is known to be valid, so the only case in which acpi_get_parent() above can fail is when the given namespace object has no parent, in which case acpi_parent will be NULL, so that should be caught my the check below, shouldn't it? > pdev = acpi_get_pci_dev(acpi_parent); > if (pdev) { > parent = &pdev->dev; > --
On Sat, Sep 10, 2022 at 9:17 AM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Fri, Sep 2, 2022 at 9:42 AM Li Zhong <floridsleeves@gmail.com> wrote: > > > > Check return status of acpi_get_parent() to confirm whether it fails. > > > > Signed-off-by: Li Zhong <floridsleeves@gmail.com> > > --- > > drivers/acpi/acpi_video.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c > > index 5cbe2196176d..5fca9a39b1a4 100644 > > --- a/drivers/acpi/acpi_video.c > > +++ b/drivers/acpi/acpi_video.c > > @@ -1,4 +1,4 @@ > > -// SPDX-License-Identifier: GPL-2.0-or-later > > + // SPDX-License-Identifier: GPL-2.0-or-later > > This change surely is not intended? > > > /* > > * video.c - ACPI Video Driver > > * > > @@ -1753,6 +1753,7 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) > > int result; > > static int count; > > char *name; > > + acpi_status status; > > > > result = acpi_video_init_brightness(device); > > if (result) > > @@ -1766,8 +1767,9 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) > > return; > > count++; > > > > - acpi_get_parent(device->dev->handle, &acpi_parent); > > - > > + status = acpi_get_parent(device->dev->handle, &acpi_parent); > > + if (ACPI_FAILURE(status)) > > + return; > > But device->dev->handle is known to be valid, so the only case in > which acpi_get_parent() above can fail is when the given namespace > object has no parent, in which case acpi_parent will be NULL, so that > should be caught my the check below, shouldn't it? > > > pdev = acpi_get_pci_dev(acpi_parent); > > if (pdev) { > > parent = &pdev->dev; > > -- That makes sense. Thank you!
diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c index 5cbe2196176d..5fca9a39b1a4 100644 --- a/drivers/acpi/acpi_video.c +++ b/drivers/acpi/acpi_video.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0-or-later + // SPDX-License-Identifier: GPL-2.0-or-later /* * video.c - ACPI Video Driver * @@ -1753,6 +1753,7 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) int result; static int count; char *name; + acpi_status status; result = acpi_video_init_brightness(device); if (result) @@ -1766,8 +1767,9 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) return; count++; - acpi_get_parent(device->dev->handle, &acpi_parent); - + status = acpi_get_parent(device->dev->handle, &acpi_parent); + if (ACPI_FAILURE(status)) + return; pdev = acpi_get_pci_dev(acpi_parent); if (pdev) { parent = &pdev->dev;
Check return status of acpi_get_parent() to confirm whether it fails. Signed-off-by: Li Zhong <floridsleeves@gmail.com> --- drivers/acpi/acpi_video.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)