From: Uwe Hermann <redacted>
Date: Tue, 11 Jul 2017 20:59:52 +0000 (+0200)
Subject: sr_dev_clear(): Always free sdi->priv (devc).
X-Git-Url: https://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=8bf18daabbf3cb0fec6c178b4bc7f6283a314e45

sr_dev_clear(): Always free sdi->priv (devc).

Until now, clear_helper() callbacks for std_dev_clear_with_callback()
were expected to g_free(devc), but not all of them did that.

Have std_dev_clear_with_callback() unconditionally g_free(sdi->priv)
(i.e., devc), regardless of whether a clear_helper() callback was
provided or not. It was doing g_free(sdi->priv) when no callback
was provided already anyway.

This makes the individual drivers' clear_helper() implementations
shorter and prevents errors such as missing g_free(devc) calls.

This works, because all drivers either call std_dev_clear_with_callback()
directly, or indirectly via std_dev_clear().

This also allows us to remove some no-longer needed dev_clear()
and clear_helper() implementations that only did g_free(devc)
in favor of std_dev_clear().
---

diff --git a/src/hardware/demo/api.c b/src/hardware/demo/api.c
index 55b17376..6549b5d7 100644
--- a/src/hardware/demo/api.c
+++ b/src/hardware/demo/api.c
@@ -193,7 +193,6 @@ static void clear_helper(void *priv)
 	while (g_hash_table_iter_next(&iter, NULL, &value))
 		g_free(value);
 	g_hash_table_unref(devc->ch_ag);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/dreamsourcelab-dslogic/api.c b/src/hardware/dreamsourcelab-dslogic/api.c
index 2b84ac3e..7e099f49 100644
--- a/src/hardware/dreamsourcelab-dslogic/api.c
+++ b/src/hardware/dreamsourcelab-dslogic/api.c
@@ -283,19 +283,6 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
 	return std_scan_complete(di, devices);
 }
 
-static void clear_helper(void *priv)
-{
-	struct dev_context *devc;
-
-	devc = priv;
-	g_free(devc);
-}
-
-static int dev_clear(const struct sr_dev_driver *di)
-{
-	return std_dev_clear_with_callback(di, clear_helper);
-}
-
 static int dev_open(struct sr_dev_inst *sdi)
 {
 	struct sr_dev_driver *di = sdi->driver;
@@ -637,7 +624,7 @@ static struct sr_dev_driver dreamsourcelab_dslogic_driver_info = {
 	.cleanup = std_cleanup,
 	.scan = scan,
 	.dev_list = std_dev_list,
-	.dev_clear = dev_clear,
+	.dev_clear = std_dev_clear,
 	.config_get = config_get,
 	.config_set = config_set,
 	.config_list = config_list,
diff --git a/src/hardware/ftdi-la/api.c b/src/hardware/ftdi-la/api.c
index 8e4f5eda..6b545afb 100644
--- a/src/hardware/ftdi-la/api.c
+++ b/src/hardware/ftdi-la/api.c
@@ -246,7 +246,6 @@ static void clear_helper(void *priv)
 
 	devc = priv;
 	g_free(devc->data_buf);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/fx2lafw/api.c b/src/hardware/fx2lafw/api.c
index da27f092..d824b022 100644
--- a/src/hardware/fx2lafw/api.c
+++ b/src/hardware/fx2lafw/api.c
@@ -351,7 +351,6 @@ static void clear_helper(void *priv)
 
 	devc = priv;
 	g_slist_free(devc->enabled_analog_channels);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/hameg-hmo/api.c b/src/hardware/hameg-hmo/api.c
index afdeb456..7e062b8d 100644
--- a/src/hardware/hameg-hmo/api.c
+++ b/src/hardware/hameg-hmo/api.c
@@ -120,8 +120,6 @@ static void clear_helper(void *priv)
 
 	g_free(devc->analog_groups);
 	g_free(devc->digital_groups);
-
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/hantek-6xxx/api.c b/src/hardware/hantek-6xxx/api.c
index ad407a2e..aa0977ad 100644
--- a/src/hardware/hantek-6xxx/api.c
+++ b/src/hardware/hantek-6xxx/api.c
@@ -162,7 +162,6 @@ static void clear_helper(void *priv)
 
 	devc = priv;
 	g_slist_free(devc->enabled_channels);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/ikalogic-scanalogic2/api.c b/src/hardware/ikalogic-scanalogic2/api.c
index 335768eb..2078b801 100644
--- a/src/hardware/ikalogic-scanalogic2/api.c
+++ b/src/hardware/ikalogic-scanalogic2/api.c
@@ -160,7 +160,6 @@ static void clear_helper(void *priv)
 
 	libusb_free_transfer(devc->xfer_in);
 	libusb_free_transfer(devc->xfer_out);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/ikalogic-scanaplus/api.c b/src/hardware/ikalogic-scanaplus/api.c
index 8ea35182..dd5d09ff 100644
--- a/src/hardware/ikalogic-scanaplus/api.c
+++ b/src/hardware/ikalogic-scanaplus/api.c
@@ -52,7 +52,6 @@ static void clear_helper(void *priv)
 	ftdi_free(devc->ftdic);
 	g_free(devc->compressed_buf);
 	g_free(devc->sample_buf);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/lecroy-xstream/api.c b/src/hardware/lecroy-xstream/api.c
index cf9df601..e00154b9 100644
--- a/src/hardware/lecroy-xstream/api.c
+++ b/src/hardware/lecroy-xstream/api.c
@@ -124,8 +124,6 @@ static void clear_helper(void *priv)
 	lecroy_xstream_state_free(devc->model_state);
 
 	g_free(devc->analog_groups);
-
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/rigol-ds/api.c b/src/hardware/rigol-ds/api.c
index 59e29fb9..221b1234 100644
--- a/src/hardware/rigol-ds/api.c
+++ b/src/hardware/rigol-ds/api.c
@@ -288,7 +288,6 @@ static void clear_helper(void *priv)
 	g_free(devc->trigger_source);
 	g_free(devc->trigger_slope);
 	g_free(devc->analog_groups);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/scpi-pps/api.c b/src/hardware/scpi-pps/api.c
index f4514d8e..3642b31e 100644
--- a/src/hardware/scpi-pps/api.c
+++ b/src/hardware/scpi-pps/api.c
@@ -295,7 +295,6 @@ static void clear_helper(void *priv)
 	devc = priv;
 	g_free(devc->channels);
 	g_free(devc->channel_groups);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/hardware/sysclk-lwla/api.c b/src/hardware/sysclk-lwla/api.c
index 6cd38a13..ea98a234 100644
--- a/src/hardware/sysclk-lwla/api.c
+++ b/src/hardware/sysclk-lwla/api.c
@@ -203,25 +203,6 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
 	return std_scan_complete(di, devices);
 }
 
-static void clear_helper(void *priv)
-{
-	struct dev_context *devc;
-
-	devc = priv;
-
-	if (devc->acquisition) {
-		sr_err("Cannot clear device context during acquisition!");
-		return; /* Leak and pray. */
-	}
-
-	g_free(devc);
-}
-
-static int dev_clear(const struct sr_dev_driver *di)
-{
-	return std_dev_clear_with_callback(di, clear_helper);
-}
-
 /* Drain any pending data from the USB transfer buffers on the device.
  * This may be necessary e.g. after a crash or generally to clean up after
  * an abnormal condition.
@@ -750,7 +731,7 @@ static struct sr_dev_driver sysclk_lwla_driver_info = {
 	.cleanup = std_cleanup,
 	.scan = scan,
 	.dev_list = std_dev_list,
-	.dev_clear = dev_clear,
+	.dev_clear = std_dev_clear,
 	.config_get = config_get,
 	.config_set = config_set,
 	.config_channel_set = config_channel_set,
diff --git a/src/hardware/yokogawa-dlm/api.c b/src/hardware/yokogawa-dlm/api.c
index 95a345b9..cace9e58 100644
--- a/src/hardware/yokogawa-dlm/api.c
+++ b/src/hardware/yokogawa-dlm/api.c
@@ -131,7 +131,6 @@ static void clear_helper(void *priv)
 
 	g_free(devc->analog_groups);
 	g_free(devc->digital_groups);
-	g_free(devc);
 }
 
 static int dev_clear(const struct sr_dev_driver *di)
diff --git a/src/std.c b/src/std.c
index 1433f30f..a4f5d883 100644
--- a/src/std.c
+++ b/src/std.c
@@ -333,16 +333,18 @@ SR_PRIV int std_serial_dev_acquisition_stop(struct sr_dev_inst *sdi)
  * This function can be used to implement the dev_clear() driver API
  * callback. dev_close() is called before every sr_dev_inst is cleared.
  *
- * The only limitation is driver-specific device contexts (sdi->priv).
+ * The only limitation is driver-specific device contexts (sdi->priv / devc).
  * These are freed, but any dynamic allocation within structs stored
  * there cannot be freed.
  *
  * @param[in] driver The driver which will have its instances released.
  *                   Must not be NULL.
  * @param[in] clear_private If not NULL, this points to a function called
- *            with sdi->priv as argument. The function can then clear
+ *            with sdi->priv (devc) as argument. The function can then clear
  *            any device instance-specific resources kept there.
- *            It must also clear the struct pointed to by sdi->priv.
+ *            It must NOT clear the struct pointed to by sdi->priv (devc),
+ *            since this function will always free it after clear_private()
+ *            has run.
  *
  * @retval SR_OK Success.
  * @retval SR_ERR_ARG Invalid argument.
@@ -388,12 +390,13 @@ SR_PRIV int std_dev_clear_with_callback(const struct sr_dev_driver *driver,
 			if (sdi->inst_type == SR_INST_MODBUS)
 				sr_modbus_free(sdi->conn);
 		}
+
+		/* Clear driver-specific stuff, if any. */
 		if (clear_private)
-			/* The helper function is responsible for freeing
-			 * its own sdi->priv! */
 			clear_private(sdi->priv);
-		else
-			g_free(sdi->priv);
+
+		/* Clear sdi->priv (devc). */
+		g_free(sdi->priv);
 
 		sr_dev_inst_free(sdi);
 	}