[PATCH] Scaffold -- some gnome-build leaks fix

[Rui Lopes <rui ruilopes com>]

Hi all,

Here are some gnome-build leaks fixes for am backend.


I need you to look at my XXX/TODO comments and tell me what you think!


Regards,
Rui Lopes

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/gnome-build/ChangeLog,v
retrieving revision 1.150
diff -u -r1.150 ChangeLog
--- ChangeLog	11 Aug 2003 17:12:55 -0000	1.150
+++ ChangeLog	5 Sep 2003 01:44:01 -0000
@@ -1,3 +1,13 @@
+2003-09-05  Rui Lopes  <rui ruilopes com>
+
+	* po/POTFILES.in: Added src/backends/libgbf_am/gbf-am-build.c.
+	* src/backends/libgbf_am/gbf-am-build.c: (build_info_free),
+	(parse_output), (build_output_cb), (gbf_build_run): Fixed
+	file descriptor leakage.  Fixed some memory leaks.  Added charset
+	conversion from locale to UTF-8 into program capture io channel.
+	* src/gbf/gbf-build-info.c: (gbf_build_info_instance_init):
+	Added a TODO comment.
+
 2003-08-11  Alex Duggan  <aldug astrolinux com>
 
 	* configure.in: Fix chmod +x warnings
Index: po/POTFILES.in
===================================================================
RCS file: /cvs/gnome/gnome-build/po/POTFILES.in,v
retrieving revision 1.10
--- po/POTFILES.in	9 Apr 2003 04:56:45 -0000	1.10
+++ po/POTFILES.in	5 Sep 2003 01:28:12 -0000
@@ -1,5 +1,6 @@
 # List of source files containing translatable strings.
 # Please keep this file sorted alphabetically.
+src/backends/libgbf_am/gbf-am-build.c
 src/backends/libgbf_am/gbf-am-config.c
 src/backends/libgbf_am/gbf-am-project.c
 src/backends/libgbf_am/gbf-am.server.in
Index: src/backends/libgbf_am/gbf-am-build.c
===================================================================
RCS file: /cvs/gnome/gnome-build/src/backends/libgbf_am/gbf-am-build.c,v
retrieving revision 1.4
--- src/backends/libgbf_am/gbf-am-build.c	9 Apr 2003 04:56:48 -0000	1.4
+++ src/backends/libgbf_am/gbf-am-build.c	5 Sep 2003 01:28:13 -0000
@@ -27,12 +27,10 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
 #include <regex.h>
 #include "gbf-i18n.h"
 #include "gbf-am-build.h"
 
-/* FIXME: mark strings for translation and this file to POTFILES.in */
-
 typedef struct {
 	GbfAmProject *project;
 	GbfBuildType type;
@@ -50,6 +48,23 @@
 	char *build_dir;
 } BuildInfo;
 
+
+static void
+build_info_free (BuildInfo *info)
+{
+	g_assert (info);
+	
+	if (info->build_dir)
+		g_free (info->build_dir);
+	if (info->dir_buf.fastmap)
+		g_free (info->dir_buf.fastmap);
+	if (info->warn_buf.fastmap)
+		g_free (info->warn_buf.fastmap);
+	if (info->err_buf.fastmap)
+		g_free (info->err_buf.fastmap);
+	g_free (info);
+}
+
 static void
 build_msg (BuildInfo      *info,
 	   GbfBuildMessage type,
@@ -69,13 +84,15 @@
 parse_output (BuildInfo  *info,
 	      const char *line)
 {
+	int line_length = strlen (line);
+
 	/* Check for directory changes. */
-	if (re_search (&info->dir_buf, line, strlen (line), 0,
-		       strlen (line), &info->reg) != -1) {
+	if (re_search (&info->dir_buf, line, line_length, 0,
+		       line_length, &info->reg) != -1) {
 		if (info->reg.num_regs >= 2) {
 			if (info->build_dir) {
 				g_free (info->build_dir);
-				info->build_dir = NULL;
+				info->build_dir = NULL; /* XXX: Why? */
 			}
 			info->build_dir = g_strndup (line + info->reg.start[1],
 						     info->reg.end[1] - info->reg.start[1]);
@@ -83,8 +100,8 @@
 	}
 
 	/* Check for warnings & errors. */
-	if (re_search (&info->warn_buf, line, strlen (line), 0,
-		       strlen (line), &info->reg) != -1) {
+	if (re_search (&info->warn_buf, line, line_length, 0,
+		       line_length, &info->reg) != -1) {
 		GbfBuildWarning *warn;
 		char *text;
 
@@ -103,8 +120,9 @@
 		warn->output = g_strdup (line);
 
 		build_msg (info, GBF_BUILD_WARNING, warn);
-	} else if (re_search (&info->err_buf, line, strlen (line), 0,
-			      strlen (line), &info->reg) != -1) {
+		/* XXX: don't we need to free warn? */
+	} else if (re_search (&info->err_buf, line, line_length, 0,
+			      line_length, &info->reg) != -1) {
 		GbfBuildError *err;
 		char *text;
 
@@ -123,6 +141,7 @@
 		err->output = g_strdup (line);
 
 		build_msg (info, GBF_BUILD_ERROR, err);
+		/* XXX: don't we need to free err? */
 	} else {
 		build_msg (info, GBF_BUILD_OUTPUT, (gpointer)line);
 	}
@@ -142,23 +161,16 @@
 
 	status = g_io_channel_read_line (chan, &line, &len, &term, &err);
 	if (status != G_IO_STATUS_NORMAL || line == NULL || err != NULL) {
+		if (err)
+			g_error_free (err); /* TODO: also report err->message using build_msg() */
 		info->num_channels--;
 		if (info->num_channels == 0) {
-			build_msg (info, GBF_BUILD_END, "Build ended");
+			build_msg (info, GBF_BUILD_END, _("Build ended"));
 
 			g_signal_emit_by_name (G_OBJECT (info->project),
 					       "build_stop", TRUE);
 
-			/* Cleanup. */
-			if (info->build_dir)
-				g_free (info->build_dir);
-			if (info->dir_buf.fastmap)
-				g_free (info->dir_buf.fastmap);
-			if (info->warn_buf.fastmap)
-				g_free (info->warn_buf.fastmap);
-			if (info->err_buf.fastmap)
-				g_free (info->err_buf.fastmap);
-			g_free (info);
+			build_info_free (info);
 		}
 
 		return FALSE;
@@ -205,11 +217,12 @@
 	static const char *dir_regex = "Entering directory `([^']+)'";
 	static const char *warn_regex = "^([^:]+):([0-9]+): warning: (.+)$";
 	static const char *err_regex = "^([^:]+):([0-9]+): (.+)$";
+	/* TODO: These paths should come from a configuration interface .. */
 	static const char *prepare_argv[] = { "./autogen.sh", "--prefix=/gnome", NULL };
 	static const char *configure_argv[] = { "./configure", "--prefix=/gnome", NULL };
 	static const char *clean_argv[] = {"/usr/bin/make", "clean", NULL };
 	static const char *all_argv[] = { "/usr/bin/make", "all", NULL };
 	static const char *install_argv[] = {"/usr/bin/make", "install", NULL };
 	static int buildid = 0;
 	const char **argv = NULL;
 	BuildInfo *info;
@@ -232,11 +245,14 @@
 		argv = all_argv;
 		break;
 	case GBF_BUILD_CURRENT:
-		g_warning ("no build for current");
+		g_warning ("no build for current\n");
 		break;
 	case GBF_BUILD_INSTALL:
 		argv = install_argv;
 		break;
+	default:
+		g_warning ("invalid build type\n");
+		return -1;
 	}
 
 	if (!g_spawn_async_with_pipes (project_dir,
@@ -246,12 +262,32 @@
 				       &pid, 
 				       NULL, &output, &err,
 				       NULL)) {
+		/* XXX: This error shouldnt go to the interface using build_msg()? */
 		g_warning ("Couldn't spawn %s\n", argv[0]);
 		return -1;
 	}
 
 	out_channel = g_io_channel_unix_new (output);
+	g_io_channel_set_close_on_unref (out_channel, TRUE);
 	err_channel = g_io_channel_unix_new (err);
+	g_io_channel_set_close_on_unref (err_channel, TRUE);
+	/* set io channel encoding if current locale is not utf-8 */
+	{
+		GError *error = NULL;
+		const char *charset;
+		if (!g_get_charset (&charset)) {
+			if (G_IO_STATUS_NORMAL != g_io_channel_set_encoding (out_channel, charset, &error) ||
+			    G_IO_STATUS_NORMAL != g_io_channel_set_encoding (err_channel, charset, &error)) {
+				g_io_channel_unref (out_channel);
+				g_io_channel_unref (err_channel);
+
+				/* XXX: report this to interface using build_msg? */
+				g_warning ("Failed to set encodings: %s\n", error->message);
+				g_error_free (error);
+				return -1;
+			}
+		}
+	}
 
 	info = g_new0 (BuildInfo, 1);
 	info->project = project;
@@ -262,12 +298,16 @@
 	info->build_dir = NULL;
 
 	/* Intialize regexs. */
+	/* XXX: gnu regex do not support utf8, we should use PCRE */
 	old_options = re_syntax_options;
 	re_syntax_options = RE_SYNTAX_EGREP;
 
 	if (!compile_pattern (&info->dir_buf, dir_regex) ||
 	    !compile_pattern (&info->warn_buf, warn_regex) ||
 	    !compile_pattern (&info->err_buf, err_regex)) {
+		g_io_channel_unref (out_channel);
+		g_io_channel_unref (err_channel);
+		build_info_free (info);
 		g_warning ("failed to compile regexs necessary for build output parsing");
 		return -1;
 	}
Index: src/gbf/gbf-build-info.c
===================================================================
RCS file: /cvs/gnome/gnome-build/src/gbf/gbf-build-info.c,v
retrieving revision 1.22
--- src/gbf/gbf-build-info.c	9 Apr 2003 04:56:49 -0000	1.22
+++ src/gbf/gbf-build-info.c	5 Sep 2003 01:28:13 -0000
@@ -153,7 +153,7 @@
 	g_signal_connect (priv->text_view, "motion-notify-event",
 			  G_CALLBACK (motion_notify_cb), info);
 
-	font_desc = pango_font_description_from_string ("mono 9");
+	font_desc = pango_font_description_from_string ("mono 9"); /* TODO: Get font description from config */
 	gtk_widget_modify_font (priv->text_view, font_desc);
 	pango_font_description_free (font_desc);