Skip to contentSkip to conversation

iio: trigger: clean up viio_trigger_alloc()

PATCH In review Dan Carpenter 2 participants 2 messages Jan 21, 2017 – Jan 22, 2017

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

PATCH 1/1 iio: trigger: clean up viio_trigger_alloc()

1) Reverse the test for kmalloc() failure so we can pull everything back one tab.
2) Use gotos for unwinding.
3) Some of the extra line breaks for the 80 character limit are no longer needed so we can remove them.

drivers/iio/industrialio-trigger.c +34 -35
index 978729f..372faef 100644
@@ -513,46 +513,45 @@ static void iio_trig_subirqunmask(struct irq_data *d)
 static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
 {
 	struct iio_trigger *trig;
+	int i;
+
 	trig = kzalloc(sizeof *trig, GFP_KERNEL);
-	if (trig) {
-		int i;
-		trig->dev.type = &iio_trig_type;
-		trig->dev.bus = &iio_bus_type;
-		device_initialize(&trig->dev);
-
-		mutex_init(&trig->pool_lock);
-		trig->subirq_base
-			= irq_alloc_descs(-1, 0,
-					  CONFIG_IIO_CONSUMERS_PER_TRIGGER,
-					  0);
-		if (trig->subirq_base < 0) {
-			kfree(trig);
-			return NULL;
-		}
+	if (!trig)
+		return NULL;
 
-		trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
-		if (trig->name = NULL) {
-			irq_free_descs(trig->subirq_base,
-				       CONFIG_IIO_CONSUMERS_PER_TRIGGER);
-			kfree(trig);
-			return NULL;
-		}
-		trig->subirq_chip.name = trig->name;
-		trig->subirq_chip.irq_mask = &iio_trig_subirqmask;
-		trig->subirq_chip.irq_unmask = &iio_trig_subirqunmask;
-		for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
-			irq_set_chip(trig->subirq_base + i,
-				     &trig->subirq_chip);
-			irq_set_handler(trig->subirq_base + i,
-					&handle_simple_irq);
-			irq_modify_status(trig->subirq_base + i,
-					  IRQ_NOREQUEST | IRQ_NOAUTOEN,
-					  IRQ_NOPROBE);
-		}
-		get_device(&trig->dev);
+	trig->dev.type = &iio_trig_type;
+	trig->dev.bus = &iio_bus_type;
+	device_initialize(&trig->dev);
+
+	mutex_init(&trig->pool_lock);
+	trig->subirq_base = irq_alloc_descs(-1, 0,
+					    CONFIG_IIO_CONSUMERS_PER_TRIGGER,
+					    0);
+	if (trig->subirq_base < 0)
+		goto free_trig;
+
+	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
+	if (trig->name = NULL)
+		goto free_descs;
+
+	trig->subirq_chip.name = trig->name;
+	trig->subirq_chip.irq_mask = &iio_trig_subirqmask;
+	trig->subirq_chip.irq_unmask = &iio_trig_subirqunmask;
+	for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
+		irq_set_chip(trig->subirq_base + i, &trig->subirq_chip);
+		irq_set_handler(trig->subirq_base + i, &handle_simple_irq);
+		irq_modify_status(trig->subirq_base + i,
+				  IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
 	}
+	get_device(&trig->dev);
 
 	return trig;
+
+free_descs:
+	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
+free_trig:
+	kfree(trig);
+	return NULL;
 }
 
 struct iio_trigger *iio_trigger_alloc(const char *fmt, ...)
maintainer
#1 lore raw
Quoted 7 lines · Dan Carpenter

1) Reverse the test for kmalloc() failure so we can pull everything back one tab.
2) Use gotos for unwinding.
3) Some of the extra line breaks for the 80 character limit are no longer needed so we can remove them.

Ouch. Ugly indeed. Good little clean up. Thanks.

Applied ot the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

Quoted 91 lines
drivers/iio/industrialio-trigger.c +34 -36
index 978729f..372faef 100644
@@ -513,46 +513,45 @@ static void iio_trig_subirqunmask(struct irq_data *d)
 static struct iio_trigger *viio_trigger_alloc(const char *fmt, va_list vargs)
 {
 	struct iio_trigger *trig;
+	int i;
+
 	trig = kzalloc(sizeof *trig, GFP_KERNEL);
-	if (trig) {
-		int i;
-		trig->dev.type = &iio_trig_type;
-		trig->dev.bus = &iio_bus_type;
-		device_initialize(&trig->dev);
-
-		mutex_init(&trig->pool_lock);
-		trig->subirq_base
-			= irq_alloc_descs(-1, 0,
-					  CONFIG_IIO_CONSUMERS_PER_TRIGGER,
-					  0);
-		if (trig->subirq_base < 0) {
-			kfree(trig);
-			return NULL;
-		}
+	if (!trig)
+		return NULL;
 
-		trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
-		if (trig->name = NULL) {
-			irq_free_descs(trig->subirq_base,
-				       CONFIG_IIO_CONSUMERS_PER_TRIGGER);
-			kfree(trig);
-			return NULL;
-		}
-		trig->subirq_chip.name = trig->name;
-		trig->subirq_chip.irq_mask = &iio_trig_subirqmask;
-		trig->subirq_chip.irq_unmask = &iio_trig_subirqunmask;
-		for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
-			irq_set_chip(trig->subirq_base + i,
-				     &trig->subirq_chip);
-			irq_set_handler(trig->subirq_base + i,
-					&handle_simple_irq);
-			irq_modify_status(trig->subirq_base + i,
-					  IRQ_NOREQUEST | IRQ_NOAUTOEN,
-					  IRQ_NOPROBE);
-		}
-		get_device(&trig->dev);
+	trig->dev.type = &iio_trig_type;
+	trig->dev.bus = &iio_bus_type;
+	device_initialize(&trig->dev);
+
+	mutex_init(&trig->pool_lock);
+	trig->subirq_base = irq_alloc_descs(-1, 0,
+					    CONFIG_IIO_CONSUMERS_PER_TRIGGER,
+					    0);
+	if (trig->subirq_base < 0)
+		goto free_trig;
+
+	trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
+	if (trig->name = NULL)
+		goto free_descs;
+
+	trig->subirq_chip.name = trig->name;
+	trig->subirq_chip.irq_mask = &iio_trig_subirqmask;
+	trig->subirq_chip.irq_unmask = &iio_trig_subirqunmask;
+	for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
+		irq_set_chip(trig->subirq_base + i, &trig->subirq_chip);
+		irq_set_handler(trig->subirq_base + i, &handle_simple_irq);
+		irq_modify_status(trig->subirq_base + i,
+				  IRQ_NOREQUEST | IRQ_NOAUTOEN, IRQ_NOPROBE);
 	}
+	get_device(&trig->dev);
 
 	return trig;
+
+free_descs:
+	irq_free_descs(trig->subirq_base, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
+free_trig:
+	kfree(trig);
+	return NULL;
 }
 
 struct iio_trigger *iio_trigger_alloc(const char *fmt, ...)
--

To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html