[PATCH 2/8] use g_slice for hotplug events

Rob Taylor rob.taylor at codethink.co.uk
Thu Mar 6 15:42:01 PST 2008


Using g_slice should be better (and makes for cleaner code) than our current
HotplugEvent pool.
---
  hald/linux/acpi.c     |    8 +++---
  hald/linux/apm.c      |    8 +++---
  hald/linux/blockdev.c |   12 ++++++----
  hald/linux/coldplug.c |   52 
+++++++++++-------------------------------------
  hald/linux/device.c   |    6 ++--
  hald/linux/hotplug.c  |    6 +----
  hald/linux/hotplug.h  |    3 --
  hald/linux/osspec.c   |    4 +-
  8 files changed, 33 insertions(+), 66 deletions(-)

diff --git a/hald/linux/acpi.c b/hald/linux/acpi.c
index d8d526c..af5145c 100644
--- a/hald/linux/acpi.c
+++ b/hald/linux/acpi.c
@@ -727,7 +727,7 @@ acpi_synthesize_item (const gchar *fullpath, int 
acpi_type)
  {
  	HotplugEvent *hotplug_event;
  	HAL_INFO (("Processing %s", fullpath));
-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_ACPI;
  	g_strlcpy (hotplug_event->acpi.acpi_path, fullpath, sizeof 
(hotplug_event->acpi.acpi_path));
@@ -855,7 +855,7 @@ acpi_synthesize_sonypi_display (void)
  	if (!found)
  		return;

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_ACPI;
  	g_strlcpy (hotplug_event->acpi.acpi_path, path, sizeof 
(hotplug_event->acpi.acpi_path));
@@ -1271,7 +1271,7 @@ acpi_generate_add_hotplug_event (HalDevice *d)
  	acpi_path = hal_device_property_get_string (d, "linux.acpi_path");
  	acpi_type = hal_device_property_get_int (d, "linux.acpi_type");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_ACPI;
  	g_strlcpy (hotplug_event->acpi.acpi_path, acpi_path, sizeof 
(hotplug_event->acpi.acpi_path));
@@ -1289,7 +1289,7 @@ acpi_generate_remove_hotplug_event (HalDevice *d)
  	acpi_path = hal_device_property_get_string (d, "linux.acpi_path");
  	acpi_type = hal_device_property_get_int (d, "linux.acpi_type");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_REMOVE;
  	hotplug_event->type = HOTPLUG_EVENT_ACPI;
  	g_strlcpy (hotplug_event->acpi.acpi_path, acpi_path, sizeof 
(hotplug_event->acpi.acpi_path));
diff --git a/hald/linux/apm.c b/hald/linux/apm.c
index 57d75c7..7b49fb2 100644
--- a/hald/linux/apm.c
+++ b/hald/linux/apm.c
@@ -297,14 +297,14 @@ apm_synthesize_hotplug_events (void)
  	/* Set appropriate properties on the computer object */
  	hal_device_property_set_string (computer, "power_management.type", 
"apm");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_APM;
  	g_strlcpy (hotplug_event->apm.apm_path, "/proc/apm", sizeof 
(hotplug_event->apm.apm_path));
  	hotplug_event->apm.apm_type = APM_TYPE_BATTERY;
  	hotplug_event_enqueue (hotplug_event);

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->type = HOTPLUG_EVENT_APM;
  	g_strlcpy (hotplug_event->apm.apm_path, "/proc/apm", sizeof 
(hotplug_event->apm.apm_path));
  	hotplug_event->apm.apm_type = APM_TYPE_AC_ADAPTER;
@@ -520,7 +520,7 @@ apm_generate_add_hotplug_event (HalDevice *d)
  	apm_path = hal_device_property_get_string (d, "linux.apm_path");
  	apm_type = hal_device_property_get_int (d, "linux.apm_type");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_APM;
  	g_strlcpy (hotplug_event->apm.apm_path, apm_path, sizeof 
(hotplug_event->apm.apm_path));
@@ -538,7 +538,7 @@ apm_generate_remove_hotplug_event (HalDevice *d)
  	apm_path = hal_device_property_get_string (d, "linux.apm_path");
  	apm_type = hal_device_property_get_int (d, "linux.apm_type");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_REMOVE;
  	hotplug_event->type = HOTPLUG_EVENT_APM;
  	g_strlcpy (hotplug_event->apm.apm_path, apm_path, sizeof 
(hotplug_event->apm.apm_path));
diff --git a/hald/linux/blockdev.c b/hald/linux/blockdev.c
index 47c95c5..decacff 100644
--- a/hald/linux/blockdev.c
+++ b/hald/linux/blockdev.c
@@ -361,7 +361,7 @@ 
generate_fakevolume_hotplug_event_add_for_storage_device (HalDevice *d)

  	snprintf (fake_sysfs_path, sizeof(fake_sysfs_path), "%s/fakevolume", 
sysfs_path);

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_SYSFS_BLOCK;
  	g_strlcpy (hotplug_event->sysfs.subsystem, "block", sizeof 
(hotplug_event->sysfs.subsystem));
@@ -955,6 +955,8 @@ hotplug_event_begin_add_blockdev (const gchar 
*sysfs_path, const gchar *device_f
  								is_device_mapper = TRUE;
  							}
  						}
+					} else {
+						HAL_INFO(("Couldn't find slave volume in devices"));
  					}
  				}
  				g_free (target);
@@ -1705,7 +1707,7 @@ blockdev_generate_add_hotplug_event (HalDevice *d)
  	serial      = hal_device_property_get_string (d, "storage.serial");
  	revision    = hal_device_property_get_string (d, 
"storage.firmware_revision");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_SYSFS;
  	g_strlcpy (hotplug_event->sysfs.subsystem, "block", sizeof 
(hotplug_event->sysfs.subsystem));
@@ -1730,7 +1732,7 @@ blockdev_generate_remove_hotplug_event (HalDevice *d)

  	sysfs_path = hal_device_property_get_string (d, "linux.sysfs_path");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_REMOVE;
  	hotplug_event->type = HOTPLUG_EVENT_SYSFS;
  	g_strlcpy (hotplug_event->sysfs.subsystem, "block", sizeof 
(hotplug_event->sysfs.subsystem));
@@ -1858,7 +1860,7 @@ blockdev_process_mdstat (void)
                          } else {
                                  HAL_INFO (("Adding md device at '%s' 
('%s')", sysfs_path, device_file));

-                                hotplug_event = g_new0 (HotplugEvent, 1);
+                                hotplug_event = g_slice_new0 
(HotplugEvent);
                                  hotplug_event->action = 
HOTPLUG_ACTION_ADD;
                                  hotplug_event->type = 
HOTPLUG_EVENT_SYSFS_BLOCK;
                                  g_strlcpy 
(hotplug_event->sysfs.subsystem, "block", sizeof 
(hotplug_event->sysfs.subsystem));
@@ -1911,7 +1913,7 @@ blockdev_process_mdstat (void)

                                  HAL_INFO (("Removing md device at '%s' 
('%s')", sysfs_path, device_file));

-                                hotplug_event = g_new0 (HotplugEvent, 1);
+                                hotplug_event = g_slice_new0 
(HotplugEvent);
                                  hotplug_event->action = 
HOTPLUG_ACTION_REMOVE;
                                  hotplug_event->type = 
HOTPLUG_EVENT_SYSFS_BLOCK;
                                  g_strlcpy 
(hotplug_event->sysfs.subsystem, "block", sizeof 
(hotplug_event->sysfs.subsystem));
diff --git a/hald/linux/coldplug.c b/hald/linux/coldplug.c
index 30949ca..5538aff 100644
--- a/hald/linux/coldplug.c
+++ b/hald/linux/coldplug.c
@@ -56,6 +56,11 @@ static GHashTable *sysfs_to_udev_map;
  static GSList *device_list;
  static char dev_root[HAL_PATH_MAX];

+static void hotplug_event_free (HotplugEvent *ev)
+{
+	g_slice_free(HotplugEvent, ev);
+}
+
  static gboolean
  hal_util_init_sysfs_to_udev_map (void)
  {
@@ -66,7 +71,7 @@ hal_util_init_sysfs_to_udev_map (void)
  	HotplugEvent *hotplug_event = NULL;
  	char *p;

-	sysfs_to_udev_map = g_hash_table_new_full (g_str_hash, g_str_equal, 
g_free, g_free);
+	sysfs_to_udev_map = g_hash_table_new_full (g_str_hash, g_str_equal, 
g_free, hotplug_event_free);

  	/* get udevroot */
  	if (g_spawn_sync ("/", udevroot_argv, NULL, 0, NULL, NULL,
@@ -132,7 +137,7 @@ hal_util_init_sysfs_to_udev_map (void)

  		/* new device */
  		if (strncmp(line, "P: ", 3) == 0) {
-			hotplug_event = g_new0 (HotplugEvent, 1);
+			hotplug_event = g_slice_new0 (HotplugEvent);
  			g_strlcpy (hotplug_event->sysfs.sysfs_path, "/sys", 
sizeof(hotplug_event->sysfs.sysfs_path));
  			g_strlcat (hotplug_event->sysfs.sysfs_path, &line[3], 
sizeof(hotplug_event->sysfs.sysfs_path));
  			continue;
@@ -203,22 +208,6 @@ error:
  	return FALSE;
  }

-static HotplugEvent *pool = NULL;
-static int pool_next_free = 0;
-static int pool_num_freed = 0;
-static int pool_size = 1000;
-
-static void
-pool_free (gpointer data)
-{
-	HAL_INFO (("pool_num_freed = %d (of %d)", pool_num_freed, 
pool_next_free));
-	pool_num_freed++;
-	if (pool_num_freed == pool_next_free) {
-		HAL_INFO (("Freeing whole pool"));
-		g_free (pool);
-	}
-}
-
  static HotplugEvent
  *coldplug_get_hotplug_event(const gchar *sysfs_path, const gchar 
*subsystem, HotplugEventType type)
  {
@@ -226,30 +215,13 @@ static HotplugEvent
  	const char *pos;
  	gchar path[HAL_PATH_MAX];
  	struct stat statbuf;
-	gboolean from_pool = FALSE;
-
-	/* TODO: FIXME: this is experimental code */
-	if (pool == NULL) {
-		pool = g_new0 (HotplugEvent, pool_size);
-		pool_next_free = 0;
-		pool_num_freed = 0;
-	}

-	if (pool_next_free >= pool_size) {
-		hotplug_event = g_new0 (HotplugEvent, 1);
-	} else {
-		from_pool = TRUE;
-		hotplug_event = pool + pool_next_free++;
-		hotplug_event->free_function = pool_free;
-	}
+	hotplug_event = g_slice_new0 (HotplugEvent);

  	/* lookup if udev has something stored in its database */
  	hotplug_event_udev = (HotplugEvent *) g_hash_table_lookup 
(sysfs_to_udev_map, sysfs_path);
  	if (hotplug_event_udev != NULL) {
  		memcpy(hotplug_event, hotplug_event_udev, sizeof(HotplugEvent));
-		if (from_pool) {
-			hotplug_event->free_function = pool_free;
-		}
  		HAL_INFO (("new event (dev node from udev) '%s' '%s'", 
hotplug_event->sysfs.sysfs_path, hotplug_event->sysfs.device_file));
  	} else {
  		/* device is not in udev database */
@@ -301,7 +273,7 @@ static int device_list_insert(const char *path, 
const char *subsystem,
  	if (!(statbuf.st_mode & S_IWUSR))
  		goto error;

-	sysfs_dev = g_new0 (struct sysfs_device, 1);
+	sysfs_dev = g_slice_new0 (struct sysfs_device);
  	if (sysfs_dev == NULL)
  		goto error;

@@ -326,11 +298,11 @@ static int device_list_insert(const char *path, 
const char *subsystem,
  	sysfs_dev->path = g_strdup (path);
  found:
  	sysfs_dev->subsystem = g_strdup (subsystem);
-	device_list = g_slist_prepend (device_list, sysfs_dev);
+	device_list = g_slist_append (device_list, sysfs_dev);
  	return 0;

  error:
-	g_free (sysfs_dev);
+	g_slice_free (struct sysfs_device, sysfs_dev);
  	return -1;
  }

@@ -512,7 +484,7 @@ static void queue_events(void)

  		g_free (sysfs_dev->path);
  		g_free (sysfs_dev->subsystem);
-		g_free (sysfs_dev);
+		g_slice_free (struct sysfs_device, sysfs_dev);
  	}

  	g_slist_free (device_list);
diff --git a/hald/linux/device.c b/hald/linux/device.c
index c41edf3..eda4a3d 100644
--- a/hald/linux/device.c
+++ b/hald/linux/device.c
@@ -753,7 +753,7 @@ missing_scsi_host (const gchar *sysfs_path, 
HotplugEvent *device_event, HotplugA

  	/* fake host event */
  	rc = TRUE;
-	host_event = g_new0 (HotplugEvent, 1);
+	host_event = g_slice_new0 (HotplugEvent);
  	host_event->action = action;
  	host_event->type = HOTPLUG_EVENT_SYSFS_DEVICE;
  	g_strlcpy (host_event->sysfs.subsystem, "scsi_host", sizeof 
(host_event->sysfs.subsystem));
@@ -4446,7 +4446,7 @@ dev_generate_add_hotplug_event (HalDevice *d)
  	sysfs_path = hal_device_property_get_string (d, "linux.sysfs_path");
  	device_file = hal_device_property_get_string (d, "linux.device_file");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_ADD;
  	hotplug_event->type = HOTPLUG_EVENT_SYSFS;
  	g_strlcpy (hotplug_event->sysfs.subsystem, subsystem, sizeof 
(hotplug_event->sysfs.subsystem));
@@ -4470,7 +4470,7 @@ dev_generate_remove_hotplug_event (HalDevice *d)
  	subsystem = hal_device_property_get_string (d, "linux.subsystem");
  	sysfs_path = hal_device_property_get_string (d, "linux.sysfs_path");

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->action = HOTPLUG_ACTION_REMOVE;
  	hotplug_event->type = HOTPLUG_EVENT_SYSFS;
  	g_strlcpy (hotplug_event->sysfs.subsystem, subsystem, sizeof 
(hotplug_event->sysfs.subsystem));
diff --git a/hald/linux/hotplug.c b/hald/linux/hotplug.c
index 239ac31..2ca62ef 100644
--- a/hald/linux/hotplug.c
+++ b/hald/linux/hotplug.c
@@ -62,11 +62,7 @@ hotplug_event_end (void *end_token)

  	hotplug_events_in_progress = g_slist_remove 
(hotplug_events_in_progress, hotplug_event);

-	if (hotplug_event->free_function != NULL) {
-		hotplug_event->free_function (hotplug_event);
-	} else {
-		g_free (hotplug_event);
-	}
+	g_slice_free (HotplugEvent, hotplug_event);
  }

  void
diff --git a/hald/linux/hotplug.h b/hald/linux/hotplug.h
index 27dc9d5..06a32a5 100644
--- a/hald/linux/hotplug.h
+++ b/hald/linux/hotplug.h
@@ -57,9 +57,6 @@ typedef struct
  	HotplugActionType action;				/* Whether the event is add or remove */
  	HotplugEventType type;					/* Type of event */
  	gboolean reposted;					/* Avoid loops */
-
-	void (*free_function) (gpointer data);
-
  	union {
  		struct {
  			char subsystem[HAL_NAME_MAX];		/* Kernel subsystem the device 
belongs to */
diff --git a/hald/linux/osspec.c b/hald/linux/osspec.c
index fb7f436..4900acf 100644
--- a/hald/linux/osspec.c
+++ b/hald/linux/osspec.c
@@ -121,7 +121,7 @@ hald_udev_data (GIOChannel *source, GIOCondition 
condition, gpointer user_data)
  		goto out;
  	}

-	hotplug_event = g_new0 (HotplugEvent, 1);
+	hotplug_event = g_slice_new0 (HotplugEvent);
  	hotplug_event->type = HOTPLUG_EVENT_SYSFS;

  	while (bufpos < sizeof (buf)) {
@@ -273,7 +273,7 @@ hald_udev_data (GIOChannel *source, GIOCondition 
condition, gpointer user_data)
  	}

  invalid:
-	g_free (hotplug_event);
+	g_slice_free (HotplugEvent, hotplug_event);

  out:
  	return TRUE;


More information about the hal mailing list