sysfs channels: cleanup device files handling

* Shortcut CLASS_DEV_CREATE/CLASS_DEV_DESTROY. No need to pass repetitive
  data (and NULL) on every call.

* Create/remove fixed device files (ctl, timer, channel, pseudo) via
  generic code (fixed_devfiles_create()/fixed_devfiles_remove()) instead
  of repetitive code and flags.

* Try to make all removal/cleanup functions idempotent, so we can
  safely call them on any failure without the need for multiple goto
  destinations.

* Rename 'device_state_flags' to 'should_cleanup' and its member
  flags to a better/consistent naming.

* Rename dahdi_sysfs_exit() to dahdi_sysfs_cleanup() and call it from
  a new proper dahdi_sysfs_exit()

* In dahdi_sysfs_init(), handle dahdi_sysfs_chan_init() failures

* Add dahdi_dbg() message before creating/removing all DEVICES
  objects.

* Also move two KERN_INFO messages to a more correct locations:
  - The version reporting should be first (in dahdi-base.c)
  - The "Telephony... on major" reporting should be at the end
    of dahdi_sysfs_init()

Signed-off-by: Oron Peled <oron.peled@xorcom.com>
Acked-by: Shaun Ruffell <sruffell@digium.com>
Acked-by: Tzafrir Cohen <tzafrir.cohen@xorcom.com>

git-svn-id: http://svn.asterisk.org/svn/dahdi/linux/trunk@10464 a0bf4364-ded3-4de4-8d8a-66a801d63aff

drivers/dahdi/dahdi-base.c

@@ -10021,6 +10021,7 @@ static int __init dahdi_init(void)
 {
 	int res = 0;
 
+	module_printk(KERN_INFO, "Version: %s\n", dahdi_version);
 #ifdef CONFIG_PROC_FS
 	root_proc_entry = proc_mkdir("dahdi", NULL);
 	if (!root_proc_entry) {

drivers/dahdi/dahdi-sysfs-chan.c

@@ -11,6 +11,7 @@
 #include "dahdi.h"
 #include "dahdi-sysfs.h"
 
+/* shortcuts, for code readability */
 #define MAKE_DAHDI_DEV(num, name) \
 	CLASS_DEV_CREATE(dahdi_class, MKDEV(DAHDI_MAJOR, num), NULL, name)
 #define DEL_DAHDI_DEV(num) \
@@ -35,9 +36,7 @@ int chan_sysfs_create(struct dahdi_chan *chan)
 	if (test_bit(DAHDI_FLAGBIT_DEVFILE, &chan->flags))
 		return 0;
 	snprintf(chan_name, sizeof(chan_name), "dahdi!%d", chan->channo);
-	dummy = (void *)CLASS_DEV_CREATE(dahdi_class,
+	dummy = (void *)MAKE_DAHDI_DEV(chan->channo, chan_name);
-		MKDEV(DAHDI_MAJOR, chan->channo),
-		NULL, chan_name);
 	if (IS_ERR(dummy)) {
 		res = PTR_ERR(dummy);
 		chan_err(chan, "Failed creating sysfs device: %d\n",
@@ -52,10 +51,13 @@ void chan_sysfs_remove(struct dahdi_chan *chan)
 {
 	if (!test_bit(DAHDI_FLAGBIT_DEVFILE, &chan->flags))
 		return;
-	CLASS_DEV_DESTROY(dahdi_class, MKDEV(DAHDI_MAJOR, chan->channo));
+	DEL_DAHDI_DEV(chan->channo);
 	clear_bit(DAHDI_FLAGBIT_DEVFILE, &chan->flags);
 }
 
+/*
+ * Used by dahdi_transcode.c
+ */
 int dahdi_register_chardev(struct dahdi_chardev *dev)
 {
 	static const char *DAHDI_STRING = "dahdi!";
@@ -68,33 +70,115 @@ int dahdi_register_chardev(struct dahdi_chardev *dev)
 
 	strcpy(udevname, DAHDI_STRING);
 	strcat(udevname, dev->name);
-	CLASS_DEV_CREATE(dahdi_class,
+	MAKE_DAHDI_DEV(dev->minor, udevname);
-		MKDEV(DAHDI_MAJOR, dev->minor), NULL, udevname);
 	kfree(udevname);
 	return 0;
 }
 EXPORT_SYMBOL(dahdi_register_chardev);
 
+/*
+ * Used by dahdi_transcode.c
+ */
 int dahdi_unregister_chardev(struct dahdi_chardev *dev)
 {
-	CLASS_DEV_DESTROY(dahdi_class, MKDEV(DAHDI_MAJOR, dev->minor));
+	DEL_DAHDI_DEV(dev->minor);
-
 	return 0;
 }
 EXPORT_SYMBOL(dahdi_unregister_chardev);
 
-/* Only used to flag that the device exists: */
+/*--------- Sysfs Device handling ----*/
+
+/*
+ * Describe fixed device files and maintain their
+ * pointer so fixed_devfiles_remove() can always be called
+ * and work cleanly
+ */
 static struct {
-	unsigned int ctl:1;
+	int	minor;
-	unsigned int timer:1;
+	char	*name;
-	unsigned int channel:1;
+	void	*dev;	/* FIXME: wrong type because of old kernels */
-	unsigned int pseudo:1;
+} fixed_minors[] = {
-} dummy_dev;
+	{ DAHDI_CTL,		"dahdi!ctl",	},
+	{ DAHDI_TIMER,		"dahdi!timer",	},
+	{ DAHDI_CHANNEL,	"dahdi!channel",},
+	{ DAHDI_PSEUDO,		"dahdi!pseudo",	},
+};
+
+/*
+ * Removes /dev/dahdi/{ctl,timer,channel,pseudo}
+ *
+ * It is safe to call it during initialization error handling,
+ * as it skips non existing objects.
+ */
+static void fixed_devfiles_remove(void)
+{
+	int i;
+
+	if (!dahdi_class)
+		return;
+	for (i = 0; i < ARRAY_SIZE(fixed_minors); i++) {
+		void *d = fixed_minors[i].dev;
+		if (d && !IS_ERR(d))
+			dahdi_dbg(DEVICES, "Removing fixed device file %s\n",
+				fixed_minors[i].name);
+			DEL_DAHDI_DEV(fixed_minors[i].minor);
+	}
+}
+
+/*
+ * Creates /dev/dahdi/{ctl,timer,channel,pseudo}
+ */
+static int fixed_devfiles_create(void)
+{
+	int i;
+	int res = 0;
+
+	if (!dahdi_class) {
+		dahdi_err("%s: dahdi_class is not initialized yet!\n",
+				__func__);
+		res = -ENODEV;
+		goto cleanup;
+	}
+	for (i = 0; i < ARRAY_SIZE(fixed_minors); i++) {
+		char *name = fixed_minors[i].name;
+		int minor = fixed_minors[i].minor;
+		void *dummy;
+
+		dahdi_dbg(DEVICES, "Making fixed device file %s\n", name);
+		dummy = (void *)MAKE_DAHDI_DEV(minor, name);
+		if (IS_ERR(dummy)) {
+			int res = PTR_ERR(dummy);
+
+			dahdi_err("%s: failed (%d: %s). Error: %d\n",
+					__func__, minor, name, res);
+			goto cleanup;
+		}
+		fixed_minors[i].dev = dummy;
+	}
+	return 0;
+cleanup:
+	fixed_devfiles_remove();
+	return res;
+}
+
+/*
+ * Called during driver unload and while handling any error during
+ * driver load.
+ * Always clean any (and only) objects that were initialized (invariant)
+ */
+static void sysfs_channels_cleanup(void)
+{
+	fixed_devfiles_remove();
+	if (dahdi_class) {
+		dahdi_dbg(DEVICES, "Destroying DAHDI class:\n");
+		class_destroy(dahdi_class);
+		dahdi_class = NULL;
+	}
+}
 
 int __init dahdi_sysfs_chan_init(const struct file_operations *fops)
 {
 	int res = 0;
-	void *dev;
 
 	dahdi_class = class_create(THIS_MODULE, "dahdi");
 	if (IS_ERR(dahdi_class)) {
@@ -103,68 +187,16 @@ int __init dahdi_sysfs_chan_init(const struct file_operations *fops)
 			__func__, res);
 		goto cleanup;
 	}
-	dahdi_dbg(DEVICES, "Creating /dev/dahdi/timer:\n");
+	res = fixed_devfiles_create();
-	dev = MAKE_DAHDI_DEV(DAHDI_TIMER, "dahdi!timer");
+	if (res)
-	if (IS_ERR(dev)) {
-		res = PTR_ERR(dev);
 		goto cleanup;
-	}
-	dummy_dev.timer = 1;
-
-	dahdi_dbg(DEVICES, "Creating /dev/dahdi/channel:\n");
-	dev = MAKE_DAHDI_DEV(DAHDI_CHANNEL, "dahdi!channel");
-	if (IS_ERR(dev)) {
-		res = PTR_ERR(dev);
-		goto cleanup;
-	}
-	dummy_dev.channel = 1;
-
-	dahdi_dbg(DEVICES, "Creating /dev/dahdi/pseudo:\n");
-	dev = MAKE_DAHDI_DEV(DAHDI_PSEUDO, "dahdi!pseudo");
-	if (IS_ERR(dev)) {
-		res = PTR_ERR(dev);
-		goto cleanup;
-	}
-	dummy_dev.pseudo = 1;
-
-	dahdi_dbg(DEVICES, "Creating /dev/dahdi/ctl:\n");
-	dev = MAKE_DAHDI_DEV(DAHDI_CTL, "dahdi!ctl");
-	if (IS_ERR(dev)) {
-		res = PTR_ERR(dev);
-		goto cleanup;
-	}
-	dummy_dev.ctl = 1;
 	return 0;
 cleanup:
-	dahdi_sysfs_chan_exit();
+	sysfs_channels_cleanup();
 	return res;
 }
 
 void dahdi_sysfs_chan_exit(void)
 {
-	if (dummy_dev.pseudo) {
+	sysfs_channels_cleanup();
-		dahdi_dbg(DEVICES, "Removing /dev/dahdi/pseudo:\n");
-		DEL_DAHDI_DEV(DAHDI_PSEUDO);
-		dummy_dev.pseudo = 0;
-	}
-	if (dummy_dev.channel) {
-		dahdi_dbg(DEVICES, "Removing /dev/dahdi/channel:\n");
-		DEL_DAHDI_DEV(DAHDI_CHANNEL);
-		dummy_dev.channel = 0;
-	}
-	if (dummy_dev.timer) {
-		dahdi_dbg(DEVICES, "Removing /dev/dahdi/timer:\n");
-		DEL_DAHDI_DEV(DAHDI_TIMER);
-		dummy_dev.timer = 0;
-	}
-	if (dummy_dev.ctl) {
-		dahdi_dbg(DEVICES, "Removing /dev/dahdi/ctl:\n");
-		DEL_DAHDI_DEV(DAHDI_CTL);
-		dummy_dev.ctl = 0;
-	}
-	if (dahdi_class && !IS_ERR(dahdi_class)) {
-		dahdi_dbg(DEVICES, "Destroying DAHDI class\n");
-		class_destroy(dahdi_class);
-		dahdi_class = NULL;
-	}
 }

drivers/dahdi/dahdi-sysfs.c

@@ -349,7 +349,7 @@ int span_sysfs_create(struct dahdi_span *span)
 
 	for (x = 0; x < span->channels; x++) {
 		res = chan_sysfs_create(span->chans[x]);
-		if (res < 0)
+		if (res)
 			goto cleanup;
 	}
 	return 0;
@@ -361,10 +361,11 @@ cleanup:
 
 /* Only used to flag that the device exists: */
 static struct {
-	unsigned int sysfs_driver_registered:1;
+	unsigned int clean_dahdi_driver:1;
-	unsigned int sysfs_spans_bus_type:1;
+	unsigned int clean_span_bus_type:1;
-	unsigned int dahdi_device_bus_registered:1;
+	unsigned int clean_device_bus:1;
-} device_state_flags;
+	unsigned int clean_chardev:1;
+} should_cleanup;
 
 static inline struct dahdi_device *to_ddev(struct device *dev)
 {
@@ -603,25 +604,30 @@ static struct bus_type dahdi_device_bus = {
 	.dev_attrs = dahdi_device_attrs,
 };
 
-void dahdi_sysfs_exit(void)
+static void dahdi_sysfs_cleanup(void)
 {
 	dahdi_dbg(DEVICES, "SYSFS\n");
-	dahdi_sysfs_chan_exit();
+	if (should_cleanup.clean_dahdi_driver) {
-	if (device_state_flags.sysfs_driver_registered) {
 		dahdi_dbg(DEVICES, "Unregister driver\n");
 		driver_unregister(&dahdi_driver);
-		device_state_flags.sysfs_driver_registered = 0;
+		should_cleanup.clean_dahdi_driver = 0;
 	}
-	if (device_state_flags.sysfs_spans_bus_type) {
+	if (should_cleanup.clean_span_bus_type) {
 		dahdi_dbg(DEVICES, "Unregister span bus type\n");
 		bus_unregister(&spans_bus_type);
-		device_state_flags.sysfs_spans_bus_type = 0;
+		should_cleanup.clean_span_bus_type = 0;
+	}
+	dahdi_sysfs_chan_exit();
+	if (should_cleanup.clean_chardev) {
+		dahdi_dbg(DEVICES, "Unregister character device\n");
+		unregister_chrdev(DAHDI_MAJOR, "dahdi");
+		should_cleanup.clean_chardev = 0;
 	}
-	unregister_chrdev(DAHDI_MAJOR, "dahdi");
 
-	if (device_state_flags.dahdi_device_bus_registered) {
+	if (should_cleanup.clean_device_bus) {
+		dahdi_dbg(DEVICES, "Unregister DAHDI device bus\n");
 		bus_unregister(&dahdi_device_bus);
-		device_state_flags.dahdi_device_bus_registered = 0;
+		should_cleanup.clean_device_bus = 0;
 	}
 }
 
@@ -674,12 +680,14 @@ int __init dahdi_sysfs_init(const struct file_operations *dahdi_fops)
 {
 	int res = 0;
 
+	dahdi_dbg(DEVICES, "Registering DAHDI device bus\n");
 	res = bus_register(&dahdi_device_bus);
 	if (res)
 		return res;
+	should_cleanup.clean_device_bus = 1;
 
-	device_state_flags.dahdi_device_bus_registered = 1;
+	dahdi_dbg(DEVICES,
-
+		"Registering character device (major=%d)\n", DAHDI_MAJOR);
 	res = register_chrdev(DAHDI_MAJOR, "dahdi", dahdi_fops);
 	if (res) {
 		module_printk(KERN_ERR,
@@ -687,30 +695,38 @@ int __init dahdi_sysfs_init(const struct file_operations *dahdi_fops)
 			"handler on %d\n", DAHDI_MAJOR);
 		return res;
 	}
-	module_printk(KERN_INFO, "Telephony Interface Registered on major %d\n",
+	should_cleanup.clean_chardev = 1;
-			DAHDI_MAJOR);
+
-	module_printk(KERN_INFO, "Version: %s\n", dahdi_version);
+	res = dahdi_sysfs_chan_init(dahdi_fops);
+	if (res)
+		goto cleanup;
 
-	dahdi_sysfs_chan_init(dahdi_fops);
 	res = bus_register(&spans_bus_type);
-	if (res != 0) {
+	if (res) {
 		dahdi_err("%s: bus_register(%s) failed. Error number %d",
 			__func__, spans_bus_type.name, res);
 		goto cleanup;
 	}
-	device_state_flags.sysfs_spans_bus_type = 1;
+	should_cleanup.clean_span_bus_type = 1;
+
 	res = driver_register(&dahdi_driver);
-	if (res < 0) {
+	if (res) {
 		dahdi_err("%s: driver_register(%s) failed. Error number %d",
 			__func__, dahdi_driver.name, res);
 		goto cleanup;
 	}
-	device_state_flags.sysfs_driver_registered = 1;
+	should_cleanup.clean_dahdi_driver = 1;
 
+	module_printk(KERN_INFO, "Telephony Interface Registered on major %d\n",
+			DAHDI_MAJOR);
 	return 0;
 
 cleanup:
-	dahdi_sysfs_exit();
+	dahdi_sysfs_cleanup();
 	return res;
 }
 
+void __exit dahdi_sysfs_exit(void)
+{
+	dahdi_sysfs_cleanup();
+}