hal/hald device_info.c, 1.25, 1.26 hald.c, 1.29, 1.30 hald_dbus.c, 1.28, 1.29 util.c, 1.6, 1.7 util.h, 1.5, 1.6

David Zeuthen david at freedesktop.org
Sun Feb 27 17:16:49 PST 2005


Update of /cvs/hal/hal/hald
In directory gabe:/tmp/cvs-serv8815/hald

Modified Files:
	device_info.c hald.c hald_dbus.c util.c util.h 
Log Message:
2005-02-27  David Zeuthen  <davidz at redhat.com>

	Played around with Valgrind on this slow Sunday :-). Before this
	patch options 'valgrind --show-reachable=yes --leak-check=yes
	--tool=memcheck ./hald --daemon=no --retain-privileges' - remember
	to set up environment variables as in run-hald.sh. Some of the
	output:

	497664 bytes in 486 blocks are still reachable in loss record 33 of 35
	     at 0x1B908984: malloc (vg_replace_malloc.c:131)
             by 0x4DE983: (within /usr/lib/libexpat.so.0.5.0)
             by 0x4E1729: XML_ParserCreate_MM (in /usr/lib/libexpat.so.0.5.0)
             by 0x4E17C1: XML_ParserCreate (in /usr/lib/libexpat.so.0.5.0)

	1003104 bytes in 972 blocks are still reachable in loss record 34 of 35
             at 0x1B908984: malloc (vg_replace_malloc.c:131)
             by 0x4DD4A2: (within /usr/lib/libexpat.so.0.5.0)
             by 0x4DD5F1: (within /usr/lib/libexpat.so.0.5.0)
             by 0x4E0596: (within /usr/lib/libexpat.so.0.5.0)

	2115584 bytes in 486 blocks are still reachable in loss record 35 of 35
             at 0x1B908984: malloc (vg_replace_malloc.c:131)
             by 0x4DC64E: XML_GetBuffer (in /usr/lib/libexpat.so.0.5.0)
             by 0x4DC91E: XML_Parse (in /usr/lib/libexpat.so.0.5.0)
             by 0x805093E: scan_fdi_files (device_info.c:1282)

	LEAK SUMMARY:
	   definitely lost: 20034 bytes in 769 blocks.
	   possibly lost:   1057 bytes in 21 blocks.
	   still reachable: 5289701 bytes in 19813 blocks.
	   suppressed: 0 bytes in 0 blocks.

	plus some illegal memory access errors. After this patch

	130613 bytes in 1 blocks are still reachable in loss record 23 of 24
	   at 0x1B908984: malloc (vg_replace_malloc.c:131)
	   by 0x80610E2: ids_init (ids.c:514)
	   by 0x8056A15: osspec_init (osspec.c:337)
	   by 0x8051DF8: main (hald.c:591)

	322003 bytes in 1 blocks are still reachable in loss record 24 of 24
	   at 0x1B908984: malloc (vg_replace_malloc.c:131)
	   by 0x806101D: ids_init (ids.c:292)
	   by 0x8056A15: osspec_init (osspec.c:337)
	   by 0x8051DF8: main (hald.c:591)

	LEAK SUMMARY:
	   definitely lost: 20884 bytes in 774 blocks.
	   possibly lost:   800 bytes in 20 blocks.
	   still reachable: 643659 bytes in 4499 blocks.

	which gives us a net saving of approx 4.5MB. Sweet. (yes, this
	ChangeLog is somewhat a weblog for me these days)

	* hald/device_info.c (process_fdi_file): Fix up error handling;
	remember to free the XML_Parser context which fixes a 4.5MB memory
	leak on my system.

	* hald/linux2/coldplug.c (coldplug_synthesize_events): Free strings
	to seal a leak

	* hald/util.h: Export hal_util_hexdump prototype

	* hald/util.c (hal_util_grep_string_elem_from_file): Yikes, make
	this a static buffer since we're returning a pointer to this
	variable.
	(hal_util_hexdump): New convenience function

	* hald/hald_dbus.c (device_add_capability): Fixup this function to
	actually work now that info.capabilities is a strlist

	* hald/hald.c (parent_wait_for_child): Fix unneeded char buf[1].



Index: device_info.c
===================================================================
RCS file: /cvs/hal/hal/hald/device_info.c,v
retrieving revision 1.25
retrieving revision 1.26
diff -u -d -r1.25 -r1.26
--- device_info.c	25 Feb 2005 01:55:08 -0000	1.25
+++ device_info.c	28 Feb 2005 01:16:47 -0000	1.26
@@ -1218,15 +1218,22 @@
 	XML_Parser parser;
 	ParsingContext *parsing_context;
 
-	snprintf (buf, 511, "%s/%s", dir, filename);
+	file = NULL;
+	filebuf = NULL;
+	parser = NULL;
+	parsing_context = NULL;
+
+	device_matched = FALSE;
+
+	snprintf (buf, sizeof (buf), "%s/%s", dir, filename);
 
 	/*HAL_INFO(("analysing file %s", buf)); */
 
 	/* open file and read it into a buffer; it's a small file... */
 	file = fopen (buf, "r");
 	if (file == NULL) {
-		perror ("fopen");
-		return FALSE;
+		HAL_ERROR (("Could not open file %s", buf));
+		goto out;
 	}
 
 	fseek (file, 0L, SEEK_END);
@@ -1234,25 +1241,27 @@
 	rewind (file);
 	filebuf = (char *) malloc (filesize);
 	if (filebuf == NULL) {
-		perror ("malloc");
-		fclose (file);
-		return FALSE;
+		HAL_ERROR (("Could not allocate %d bytes for file %s", filesize, buf));
+		goto out;
 	}
 	fread (filebuf, sizeof (char), filesize, file);
 
-
-	/* ok, now parse the file (should probably reuse parser and say we are
-	 * not thread safe 
-	 */
-	parser = XML_ParserCreate (NULL);
-
 	/* initialize parsing context */
 	parsing_context =
 	    (ParsingContext *) malloc (sizeof (ParsingContext));
 	if (parsing_context == NULL) {
-		perror ("malloc");
-		return FALSE;
+		HAL_ERROR (("Could not allocate parsing context"));
+		goto out;
+	}
+
+	/* TODO: reuse parser
+	 */
+	parser = XML_ParserCreate (NULL);
+	if (parser == NULL) {
+		HAL_ERROR (("Could not allocate XML parser"));
+		goto out;
 	}
+
 	parsing_context->depth = 0;
 	parsing_context->device_matched = FALSE;
 	parsing_context->match_ok = TRUE;
@@ -1287,9 +1296,15 @@
 		device_matched = parsing_context->device_matched;
 	}
 
-	free (filebuf);
-	fclose (file);
-	free (parsing_context);
+out:
+	if (filebuf != NULL)
+		free (filebuf);
+	if (file != NULL)
+		fclose (file);
+	if (parser != NULL)
+		XML_ParserFree (parser);
+	if (parsing_context != NULL)
+		free (parsing_context);
 
 	return device_matched;
 }

Index: hald.c
===================================================================
RCS file: /cvs/hal/hal/hald/hald.c,v
retrieving revision 1.29
retrieving revision 1.30
diff -u -d -r1.29 -r1.30
--- hald.c	25 Feb 2005 03:49:06 -0000	1.29
+++ hald.c	28 Feb 2005 01:16:47 -0000	1.30
@@ -335,7 +335,6 @@
 static int 
 parent_wait_for_child (int child_fd, pid_t child_pid)
 {
-	char buf[1];
 	fd_set rfds;
 	fd_set efds;
 	struct timeval tv;
@@ -406,6 +405,7 @@
 
 	retain_privs = FALSE;
   
+
 	openlog ("hald", LOG_PID, LOG_DAEMON);
 
 	g_type_init ();
@@ -619,6 +619,7 @@
 	close (startup_daemonize_pipe[1]);
 
 	hald_is_initialising = FALSE;
+
 }
 
 

Index: hald_dbus.c
===================================================================
RCS file: /cvs/hal/hal/hald/hald_dbus.c,v
retrieving revision 1.28
retrieving revision 1.29
diff -u -d -r1.28 -r1.29
--- hald_dbus.c	25 Feb 2005 16:55:23 -0000	1.28
+++ hald_dbus.c	28 Feb 2005 01:16:47 -0000	1.29
@@ -1271,10 +1271,6 @@
 	return DBUS_HANDLER_RESULT_HANDLED;
 }
 
-
-/** Maximum string length for capabilities; quite a hack :-/ */
-#define MAX_CAP_SIZE 2048
-
 /** This function is used to modify the Capabilities property. The reason
  *  for having a dedicated function is that the HAL daemon will broadcast
  *  a signal on the Manager interface to tell applications that the device
@@ -1307,11 +1303,9 @@
 {
 	const char *udi;
 	const char *capability;
-	const char *caps;
 	HalDevice *d;
 	DBusMessage *reply;
 	DBusError error;
-	char buf[MAX_CAP_SIZE];
 
 	HAL_TRACE (("entering"));
 
@@ -1340,20 +1334,7 @@
 	}
 
 
-	caps = hal_device_property_get_string (d, "info.capabilities");
-	if (caps == NULL) {
-		hal_device_property_set_string (d, "info.capabilities",
-					capability);
-	} else {
-		if (strstr (caps, capability) == NULL) {
-			snprintf (buf, MAX_CAP_SIZE, "%s %s", caps,
-				  capability);
-			hal_device_property_set_string (d, "info.capabilities",
-						buf);
-		}
-	}
-
-	manager_send_signal_new_capability (d, capability);
+	hal_device_add_capability (d, capability);
 
 	reply = dbus_message_new_method_return (message);
 	if (reply == NULL)

Index: util.c
===================================================================
RCS file: /cvs/hal/hal/hald/util.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -d -r1.6 -r1.7
--- util.c	25 Feb 2005 01:55:08 -0000	1.6
+++ util.c	28 Feb 2005 01:16:47 -0000	1.7
@@ -681,7 +681,7 @@
 {
 	gchar *line;
 	gchar *res;
-	gchar buf[256];
+	static gchar buf[256];
 	gchar **tokens;
 	guint i, j;
 
@@ -1056,3 +1056,46 @@
 
 	return n;
 }
+
+void
+hal_util_hexdump (const void *mem, unsigned int size)
+{
+	unsigned int i;
+	unsigned int j;
+	unsigned int n;
+	const char *buf = (const char *) mem;
+
+	n = 0;
+	printf ("Dumping %d=0x%x bytes\n", size, size);
+	while (n < size) {
+
+		printf ("0x%04x: ", n);
+
+		j = n;
+		for (i = 0; i < 16; i++) {
+			if (j >= size)
+				break;
+			printf ("%02x ", buf[j]);
+			j++;
+		}
+		
+		for ( ; i < 16; i++) {
+			printf ("   ");
+		}
+		
+		printf ("   ");
+		
+		j = n;
+		for (i = 0; i < 16; i++) {
+			if (j >= size)
+				break;
+			printf ("%c", isprint(buf[j]) ? buf[j] : '.');
+			j++;
+		}
+
+		printf ("\n");
+		
+		n += 16;
+	}
+}
+

Index: util.h
===================================================================
RCS file: /cvs/hal/hal/hald/util.h,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -d -r1.5 -r1.6
--- util.h	25 Feb 2005 01:55:08 -0000	1.5
+++ util.h	28 Feb 2005 01:16:47 -0000	1.6
@@ -110,6 +110,8 @@
 void hal_util_callout_device_remove (HalDevice *d, HalCalloutsDone callback, gpointer userdata1, gpointer userdata2);
 void hal_util_callout_device_preprobe (HalDevice *d, HalCalloutsDone callback, gpointer userdata1, gpointer userdata2);
 
+void hal_util_hexdump (const void *buf, unsigned int size);
+
 #define HAL_HELPER_TIMEOUT 10000
 
 #define HAL_PATH_MAX 256




More information about the hal-commit mailing list