PNPACPI: check return value of pnpacpi_parse_allocated_resource() and pnpacpi_parse_resource_option_data()

PATCH In review Arjun Sreedharan 2 participants 5 messages Jul 22, 2014 – Jul 23, 2014

Subsystems (2)

ACPI Pnp support

lore

Share & tools

lore

Arjun Sreedharan

author

2014-07-22 19:21 UTC #0 lore raw

PATCH 1/1 PNPACPI: check return value of pnpacpi_parse_allocated_resource() and pnpacpi_parse_resource_option_data()

Handle error condition since pnpacpi_parse_allocated_resource() and pnpacpi_parse_resource_option_data() could return -EPERM.

Signed-off-by Arjun Sreedharan <[email protected]>

---
 drivers/pnp/pnpacpi/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/pnp/pnpacpi/core.c +6 -2

index b81448b..c3214e9 100644
@@ -272,10 +272,14 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
 		strncpy(dev->name, acpi_device_bid(device), sizeof(dev->name));
 
 	if (dev->active)
-		pnpacpi_parse_allocated_resource(dev);
+		error = pnpacpi_parse_allocated_resource(dev);
+		if (error)
+			return error;
 
 	if (dev->capabilities & PNP_CONFIGURABLE)
-		pnpacpi_parse_resource_option_data(dev);
+		error = pnpacpi_parse_resource_option_data(dev);
+		if (error)
+			return error;
 
 	list_for_each_entry(id, &device->pnp.ids, list) {
 		if (!strcmp(id->id, pnpid))

Signature · 1 line

1.7.11.7

Rafael J. Wysocki

2014-07-22 21:53 UTC #1 lore raw

Quoted 3 lines · Arjun Sreedharan

Handle error condition since pnpacpi_parse_allocated_resource() and pnpacpi_parse_resource_option_data() could return -EPERM.
Signed-off-by Arjun Sreedharan <[email protected]>

I wonder if this fixes any functional issue you've seen or someone has reported to you or it's just an "Oh, we probably should handle the return values here" kind of thing?

Quoted 26 lines

---
 drivers/pnp/pnpacpi/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/pnp/pnpacpi/core.c +6 -2

index b81448b..c3214e9 100644
@@ -272,10 +272,14 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
 		strncpy(dev->name, acpi_device_bid(device), sizeof(dev->name));
 
 	if (dev->active)
-		pnpacpi_parse_allocated_resource(dev);
+		error = pnpacpi_parse_allocated_resource(dev);
+		if (error)
+			return error;
 
 	if (dev->capabilities & PNP_CONFIGURABLE)
-		pnpacpi_parse_resource_option_data(dev);
+		error = pnpacpi_parse_resource_option_data(dev);
+		if (error)
+			return error;
 
 	list_for_each_entry(id, &device->pnp.ids, list) {
 		if (!strcmp(id->id, pnpid))

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Arjun

author

2014-07-23 08:26 UTC #2 lore raw

ACPI 5.0 resource types like FixedDMA will fail pnpacpi_allocated_resource(). pnpacpi_add_device() has to return error in that case.

Quoted 39 lines · Rafael J. Wysocki

Quoted 3 lines · Arjun Sreedharan
Handle error condition since pnpacpi_parse_allocated_resource() and pnpacpi_parse_resource_option_data() could return -EPERM.
Signed-off-by Arjun Sreedharan <[email protected]>
I wonder if this fixes any functional issue you've seen or someone has reported to you or it's just an "Oh, we probably should handle the return values here" kind of thing?
Quoted 16 lines
---
 drivers/pnp/pnpacpi/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pnp/pnpacpi/core.c +2 -1
index b81448b..c3214e9 100644
@@ -272,10 +272,14 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
              strncpy(dev->name, acpi_device_bid(device), sizeof(dev->name));
 
      if (dev->active)
-             pnpacpi_parse_allocated_resource(dev);
+             error = pnpacpi_parse_allocated_resource(dev);
+             if (error)
drivers/pnp/pnpacpi/core.c static int __init pnpacpi_add_device(struct acpi_device *device) · line 285
Quoted 10 lines
drivers/pnp/pnpacpi/core.c +4 -1
+                     return error;
 
      if (dev->capabilities & PNP_CONFIGURABLE)
-             pnpacpi_parse_resource_option_data(dev);
+             error = pnpacpi_parse_resource_option_data(dev);
+             if (error)
+                     return error;
 
      list_for_each_entry(id, &device->pnp.ids, list) {
              if (!strcmp(id->id, pnpid))
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Rafael J. Wysocki

2014-07-23 11:18 UTC #3 lore raw

Quoted 2 lines · Arjun

ACPI 5.0 resource types like FixedDMA will fail pnpacpi_allocated_resource(). pnpacpi_add_device() has to return error in that case.

Why?

Quoted 40 lines · Rafael J. Wysocki

Quoted 39 lines · Arjun Sreedharan
Quoted 3 lines
Handle error condition since pnpacpi_parse_allocated_resource() and pnpacpi_parse_resource_option_data() could return -EPERM.
Signed-off-by Arjun Sreedharan <[email protected]>
I wonder if this fixes any functional issue you've seen or someone has reported to you or it's just an "Oh, we probably should handle the return values here" kind of thing?
Quoted 16 lines
---
 drivers/pnp/pnpacpi/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pnp/pnpacpi/core.c +2 -1
index b81448b..c3214e9 100644
@@ -272,10 +272,14 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
              strncpy(dev->name, acpi_device_bid(device), sizeof(dev->name));
 
      if (dev->active)
-             pnpacpi_parse_allocated_resource(dev);
+             error = pnpacpi_parse_allocated_resource(dev);
+             if (error)
drivers/pnp/pnpacpi/core.c static int __init pnpacpi_add_device(struct acpi_device *device) · line 285
Quoted 10 lines
drivers/pnp/pnpacpi/core.c +4 -1
+                     return error;
 
      if (dev->capabilities & PNP_CONFIGURABLE)
-             pnpacpi_parse_resource_option_data(dev);
+             error = pnpacpi_parse_resource_option_data(dev);
+             if (error)
+                     return error;
 
      list_for_each_entry(id, &device->pnp.ids, list) {
              if (!strcmp(id->id, pnpid))
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Arjun Sreedharan

author

2014-07-23 19:49 UTC #4 lore raw

Quoted 6 lines · Rafael J. Wysocki

Quoted 2 lines · Arjun
ACPI 5.0 resource types like FixedDMA will fail pnpacpi_allocated_resource(). pnpacpi_add_device() has to return error in that case.
Why?

OK. I presume that for unsupported types like ACPI_RESOURCE_TYPE_FIXED_DMA pnpacpi_set_resources() will fail.

Quoted 44 lines

Quoted 33 lines · Rafael J. Wysocki
Quoted 32 lines · Arjun Sreedharan
Quoted 3 lines
Handle error condition since pnpacpi_parse_allocated_resource() and pnpacpi_parse_resource_option_data() could return -EPERM.
Signed-off-by Arjun Sreedharan <[email protected]>
I wonder if this fixes any functional issue you've seen or someone has reported to you or it's just an "Oh, we probably should handle the return values here" kind of thing?
Quoted 23 lines
---
 drivers/pnp/pnpacpi/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/pnp/pnpacpi/core.c +6 -2
index b81448b..c3214e9 100644
@@ -272,10 +272,14 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
              strncpy(dev->name, acpi_device_bid(device), sizeof(dev->name));
 
      if (dev->active)
-             pnpacpi_parse_allocated_resource(dev);
+             error = pnpacpi_parse_allocated_resource(dev);
+             if (error)
+                     return error;
 
      if (dev->capabilities & PNP_CONFIGURABLE)
-             pnpacpi_parse_resource_option_data(dev);
+             error = pnpacpi_parse_resource_option_data(dev);
+             if (error)
+                     return error;
 
Quoted 7 lines
Quoted 7 lines
drivers/pnp/pnpacpi/core.c static int __init pnpacpi_add_device(struct acpi_device *device) · line 285
Quoted 3 lines
drivers/pnp/pnpacpi/core.c +0 -0
      list_for_each_entry(id, &device->pnp.ids, list) {
              if (!strcmp(id->id, pnpid))
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.