Implementing rollback with goto and labels is a common practice that leads to prettier and more maintainable code. FWIW, this design pattern is already being used in alloc_link() a few lines below in this file.
Do the same in setup_xsl_irq().
Signed-off-by: Greg Kurz groug@kaod.org --- drivers/misc/ocxl/link.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c index eed92055184d..659977a17405 100644 --- a/drivers/misc/ocxl/link.c +++ b/drivers/misc/ocxl/link.c @@ -273,9 +273,9 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) spa->irq_name = kasprintf(GFP_KERNEL, "ocxl-xsl-%x-%x-%x", link->domain, link->bus, link->dev); if (!spa->irq_name) { - unmap_irq_registers(spa); dev_err(&dev->dev, "Can't allocate name for xsl interrupt\n"); - return -ENOMEM; + rc = -ENOMEM; + goto err_xsl; } /* * At some point, we'll need to look into allowing a higher @@ -283,11 +283,10 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) */ spa->virq = irq_create_mapping(NULL, hwirq); if (!spa->virq) { - kfree(spa->irq_name); - unmap_irq_registers(spa); dev_err(&dev->dev, "irq_create_mapping failed for translation interrupt\n"); - return -EINVAL; + rc = -EINVAL; + goto err_name; }
dev_dbg(&dev->dev, "hwirq %d mapped to virq %d\n", hwirq, spa->virq); @@ -295,15 +294,21 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) rc = request_irq(spa->virq, xsl_fault_handler, 0, spa->irq_name, link); if (rc) { - irq_dispose_mapping(spa->virq); - kfree(spa->irq_name); - unmap_irq_registers(spa); dev_err(&dev->dev, "request_irq failed for translation interrupt: %d\n", rc); - return -EINVAL; + rc = -EINVAL; + goto err_mapping; } return 0; + +err_mapping: + irq_dispose_mapping(spa->virq); +err_name: + kfree(spa->irq_name); +err_xsl: + unmap_irq_registers(spa); + return rc; }
static void release_xsl_irq(struct link *link)
Le 10/12/2018 à 16:18, Greg Kurz a écrit :
Implementing rollback with goto and labels is a common practice that leads to prettier and more maintainable code. FWIW, this design pattern is already being used in alloc_link() a few lines below in this file.
Do the same in setup_xsl_irq().
Signed-off-by: Greg Kurz groug@kaod.org
This looks good. I don't have a fixed limit when I start using the "goto undo" pattern, so it's likely inconsistent in other places as well. Truth is I'm not too fussed either way.
Acked-by: Frederic Barrat fbarrat@linux.ibm.com
drivers/misc/ocxl/link.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c index eed92055184d..659977a17405 100644 --- a/drivers/misc/ocxl/link.c +++ b/drivers/misc/ocxl/link.c @@ -273,9 +273,9 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) spa->irq_name = kasprintf(GFP_KERNEL, "ocxl-xsl-%x-%x-%x", link->domain, link->bus, link->dev); if (!spa->irq_name) {
dev_err(&dev->dev, "Can't allocate name for xsl interrupt\n");unmap_irq_registers(spa);
return -ENOMEM;
rc = -ENOMEM;
} /*goto err_xsl;
- At some point, we'll need to look into allowing a higher
@@ -283,11 +283,10 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) */ spa->virq = irq_create_mapping(NULL, hwirq); if (!spa->virq) {
kfree(spa->irq_name);
dev_err(&dev->dev, "irq_create_mapping failed for translation interrupt\n");unmap_irq_registers(spa);
return -EINVAL;
rc = -EINVAL;
goto err_name;
}
dev_dbg(&dev->dev, "hwirq %d mapped to virq %d\n", hwirq, spa->virq);
@@ -295,15 +294,21 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) rc = request_irq(spa->virq, xsl_fault_handler, 0, spa->irq_name, link); if (rc) {
irq_dispose_mapping(spa->virq);
kfree(spa->irq_name);
dev_err(&dev->dev, "request_irq failed for translation interrupt: %d\n", rc);unmap_irq_registers(spa);
return -EINVAL;
rc = -EINVAL;
} return 0;goto err_mapping;
+err_mapping:
- irq_dispose_mapping(spa->virq);
+err_name:
- kfree(spa->irq_name);
+err_xsl:
unmap_irq_registers(spa);
return rc; }
static void release_xsl_irq(struct link *link)
On 11/12/18 2:18 am, Greg Kurz wrote:
Implementing rollback with goto and labels is a common practice that leads to prettier and more maintainable code. FWIW, this design pattern is already being used in alloc_link() a few lines below in this file.
Do the same in setup_xsl_irq().
Signed-off-by: Greg Kurz groug@kaod.org
This is good, thanks.
Acked-by: Andrew Donnellan andrew.donnellan@au1.ibm.com
drivers/misc/ocxl/link.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c index eed92055184d..659977a17405 100644 --- a/drivers/misc/ocxl/link.c +++ b/drivers/misc/ocxl/link.c @@ -273,9 +273,9 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) spa->irq_name = kasprintf(GFP_KERNEL, "ocxl-xsl-%x-%x-%x", link->domain, link->bus, link->dev); if (!spa->irq_name) {
dev_err(&dev->dev, "Can't allocate name for xsl interrupt\n");unmap_irq_registers(spa);
return -ENOMEM;
rc = -ENOMEM;
} /*goto err_xsl;
- At some point, we'll need to look into allowing a higher
@@ -283,11 +283,10 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) */ spa->virq = irq_create_mapping(NULL, hwirq); if (!spa->virq) {
kfree(spa->irq_name);
dev_err(&dev->dev, "irq_create_mapping failed for translation interrupt\n");unmap_irq_registers(spa);
return -EINVAL;
rc = -EINVAL;
}goto err_name;
dev_dbg(&dev->dev, "hwirq %d mapped to virq %d\n", hwirq, spa->virq); @@ -295,15 +294,21 @@ static int setup_xsl_irq(struct pci_dev *dev, struct link *link) rc = request_irq(spa->virq, xsl_fault_handler, 0, spa->irq_name, link); if (rc) {
irq_dispose_mapping(spa->virq);
kfree(spa->irq_name);
dev_err(&dev->dev, "request_irq failed for translation interrupt: %d\n", rc);unmap_irq_registers(spa);
return -EINVAL;
rc = -EINVAL;
} return 0;goto err_mapping;
+err_mapping:
- irq_dispose_mapping(spa->virq);
+err_name:
- kfree(spa->irq_name);
+err_xsl:
- unmap_irq_registers(spa);
- return rc; }
static void release_xsl_irq(struct link *link)
On Mon, 2018-12-10 at 15:18:13 UTC, Greg Kurz wrote:
Implementing rollback with goto and labels is a common practice that leads to prettier and more maintainable code. FWIW, this design pattern is already being used in alloc_link() a few lines below in this file.
Do the same in setup_xsl_irq().
Signed-off-by: Greg Kurz groug@kaod.org Acked-by: Frederic Barrat fbarrat@linux.ibm.com Acked-by: Andrew Donnellan andrew.donnellan@au1.ibm.com
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/759bc01586535700fbb1aaf40dd727
cheers
linux-stable-mirror@lists.linaro.org