Skip to contentSkip to conversation

xen/mce: fix up xen_late_init_mcelog() error handling

PATCH Applied Dan Carpenter 3 participants 4 messages Mar 5, 2015 – Mar 16, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xen/mce: fix up xen_late_init_mcelog() error handling

Static checkers complain about the missing call to misc_deregister() if bind_virq_for_mce() fails.

Also I reversed the tests so that we do error handling instead of success handling. That way we just have a series of function calls instead of the more complicated nested if statements in the original code. Let's preserve the error codes as well.

drivers/xen/mcelog.c +18 -7
index 6ab6a79..a493c7315 100644
@@ -393,14 +393,25 @@ static int bind_virq_for_mce(void)
 
 static int __init xen_late_init_mcelog(void)
 {
+	int ret;
+
 	/* Only DOM0 is responsible for MCE logging */
-	if (xen_initial_domain()) {
-		/* register character device /dev/mcelog for xen mcelog */
-		if (misc_register(&xen_mce_chrdev_device))
-			return -ENODEV;
-		return bind_virq_for_mce();
-	}
+	if (!xen_initial_domain())
+		return -ENODEV;
+
+	/* register character device /dev/mcelog for xen mcelog */
+	ret = misc_register(&xen_mce_chrdev_device);
+	if (ret)
+		return ret;
+
+	ret = bind_virq_for_mce();
+	if (ret)
+		goto deregister;
 
-	return -ENODEV;
+	return 0;
+
+deregister:
+	misc_deregister(&xen_mce_chrdev_device);
+	return ret;
 }
 device_initcall(xen_late_init_mcelog);
Reviewed-by on v1 1/1
#1 lore raw
Quoted 9 lines · Dan Carpenter

Static checkers complain about the missing call to misc_deregister() if bind_virq_for_mce() fails.

Also I reversed the tests so that we do error handling instead of success handling. That way we just have a series of function calls instead of the more complicated nested if statements in the original code. Let's preserve the error codes as well.

David, you want me to stash in devel for 4.1?

Quoted 38 lines
drivers/xen/mcelog.c +18 -7
index 6ab6a79..a493c7315 100644
@@ -393,14 +393,25 @@ static int bind_virq_for_mce(void)
 
 static int __init xen_late_init_mcelog(void)
 {
+	int ret;
+
 	/* Only DOM0 is responsible for MCE logging */
-	if (xen_initial_domain()) {
-		/* register character device /dev/mcelog for xen mcelog */
-		if (misc_register(&xen_mce_chrdev_device))
-			return -ENODEV;
-		return bind_virq_for_mce();
-	}
+	if (!xen_initial_domain())
+		return -ENODEV;
+
+	/* register character device /dev/mcelog for xen mcelog */
+	ret = misc_register(&xen_mce_chrdev_device);
+	if (ret)
+		return ret;
+
+	ret = bind_virq_for_mce();
+	if (ret)
+		goto deregister;
 
-	return -ENODEV;
+	return 0;
+
+deregister:
+	misc_deregister(&xen_mce_chrdev_device);
+	return ret;
 }
 device_initcall(xen_late_init_mcelog);
Reviewed-by on v1 1/1
#2 lore raw
In reply to Dan Carpenter
Quoted 9 lines · Dan Carpenter

Static checkers complain about the missing call to misc_deregister() if bind_virq_for_mce() fails.

Also I reversed the tests so that we do error handling instead of success handling. That way we just have a series of function calls instead of the more complicated nested if statements in the original code. Let's preserve the error codes as well.

David, you want me to stash in devel for 4.1?

Quoted 38 lines
drivers/xen/mcelog.c +18 -7
index 6ab6a79..a493c7315 100644
@@ -393,14 +393,25 @@ static int bind_virq_for_mce(void)
 
 static int __init xen_late_init_mcelog(void)
 {
+	int ret;
+
 	/* Only DOM0 is responsible for MCE logging */
-	if (xen_initial_domain()) {
-		/* register character device /dev/mcelog for xen mcelog */
-		if (misc_register(&xen_mce_chrdev_device))
-			return -ENODEV;
-		return bind_virq_for_mce();
-	}
+	if (!xen_initial_domain())
+		return -ENODEV;
+
+	/* register character device /dev/mcelog for xen mcelog */
+	ret = misc_register(&xen_mce_chrdev_device);
+	if (ret)
+		return ret;
+
+	ret = bind_virq_for_mce();
+	if (ret)
+		goto deregister;
 
-	return -ENODEV;
+	return 0;
+
+deregister:
+	misc_deregister(&xen_mce_chrdev_device);
+	return ret;
 }
 device_initcall(xen_late_init_mcelog);
Applied
#3 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Dan Carpenter

Static checkers complain about the missing call to misc_deregister() if bind_virq_for_mce() fails.

Also I reversed the tests so that we do error handling instead of success handling. That way we just have a series of function calls instead of the more complicated nested if statements in the original code. Let's preserve the error codes as well.

Applied to devel/for-linus-4.1, thanks.

David