PERFORCE change 129858 for review

[Garrett Cooper]

http://perforce.freebsd.org/chv.cgi?CH=129858

Change 129858 by gcooper at shiina-ibook on 2007/12/01 01:21:39

	- Separate out freebsd_package free'ing functionality from generic pkg free'ing functionality so we can call freebsd_free whenever we need to free up freebsd_package structures, and pkg_freebsd_free whenever we need to free up freebsd_package structures attached to pkg structures...
	- Some style changes.
	- Get ready to implement more NOTYET / todo code..

Affected files ...

.. //depot/projects/soc2007/revised_fbsd_pkgtools/pkg_revised/v2/contrib/libpkg/pkg_freebsd.c#4 edit

Differences ...

==== //depot/projects/soc2007/revised_fbsd_pkgtools/pkg_revised/v2/contrib/libpkg/pkg_freebsd.c#4 (text+ko) ====

@@ -69,7 +69,10 @@
 static struct pkg	**freebsd_get_rdeps(struct pkg *);
 static int		  freebsd_run_script(struct pkg *,const char *,
 				pkg_script);
-static int		  freebsd_free(struct pkg *);
+static int		  pkg_freebsd_free(struct pkg *);
+
+static int		  pkg_freebsd_free(struct pkg *);
+static int		  freebsd_free(struct freebsd_package *);
 
 /* Internal functions */
 static struct freebsd_package	 *freebsd_package_new(void);
@@ -153,16 +156,17 @@
 	assert(fpkg->contents != NULL);
 	if (fpkg->contents->lines[1].line_type != PKG_LINE_NAME ||
 	    fpkg->contents->lines[3].line_type != PKG_LINE_CWD) {
-		/** @todo cleanup */
+		freebsd_free(fpkg->contents);
+		warnx("Invalid package format..");
 		return NULL;
 	}
 
 	pkg_name = fpkg->contents->lines[1].data;
 	pkg = pkg_new(pkg_name, freebsd_get_control_files,
 	    freebsd_get_control_file, freebsd_get_manifest, freebsd_get_deps,
-	    NULL, freebsd_free);
+	    NULL, pkg_freebsd_free);
 	if (pkg == NULL) {
-		/** @todo cleanup */
+		warnx("Package returned was null..");
 		return NULL;
 	}
 	pkg_add_callbacks_data(pkg, freebsd_get_version, freebsd_get_origin,
@@ -203,14 +207,16 @@
 	/* check the directory exists and is a directory */
 	if (lstat(pkg_db_dir, &sb) == -1)
 		return NULL;
-	if (!S_ISDIR(sb.st_mode))
+	if (0 == S_ISDIR(sb.st_mode))
 		return NULL;
 
 	pkg = pkg_new(pkg_name, freebsd_get_control_files,
 	    freebsd_get_control_file, freebsd_get_manifest, freebsd_get_deps,
-	    freebsd_get_rdeps, freebsd_free);
+	    freebsd_get_rdeps, pkg_freebsd_free);
+
 	if (pkg == NULL)
 		return NULL;
+
 	pkg_add_callbacks_data(pkg, freebsd_get_version, freebsd_get_origin,
 	    freebsd_set_origin);
 	pkg_add_callbacks_install(pkg, NULL, freebsd_deinstall,
@@ -249,7 +255,7 @@
 	struct freebsd_package *fpkg;
 
 	/* Create the package */
-	pkg = pkg_new(pkg_name, NULL, NULL, NULL, NULL, NULL, freebsd_free);
+	pkg = pkg_new(pkg_name, NULL, NULL, NULL, NULL, NULL, pkg_freebsd_free);
 	if (pkg == NULL)
 		return NULL;
 
@@ -403,7 +409,7 @@
  * @return -1
  */
 static int
-freebsd_add_depend(struct pkg *pkg __unused, struct pkg *depend __unused)
+freebsd_add_depend(struct pkg *pkg, struct pkg *depend)
 {
 	assert(0);
 	return -1;
@@ -543,9 +549,9 @@
 	file_data = pkgfile_get_data(contents_file);
 	contents = pkg_freebsd_contents_new(file_data,
 	    pkgfile_get_size(contents_file));
-	if (contents == NULL) {
+
+	if (contents == NULL)
 		return -1;
-	}
 
 	pkg_get_manifest(pkg);
 	assert(pkg->pkg_manifest != NULL);
@@ -1161,19 +1167,24 @@
  * @return 0
  */
 static int
-freebsd_free(struct pkg *pkg)
+pkg_freebsd_free(struct pkg *pkg)
 {
-	struct freebsd_package *fpkg;
 	assert(pkg != NULL);
 
-	fpkg = pkg->data;
-	if (fpkg) {
+	return freebsd_free( (struct freebsd_package *) pkg->data);
+}
+
+static int
+freebsd_free(struct freebsd_package *fpkg)
+{
+
+	if (fpkg != NULL) {
+
 		if (fpkg->db_dir != NULL)
 			free(fpkg->db_dir);
 
-		/** @todo Fix this to only call free when required */
-		/* if (fpkg->origin != NULL)
-			free(fpkg->origin); */
+		if (fpkg->origin != NULL)
+			free(fpkg->origin);
 
 		if (fpkg->next_file != NULL)
 			pkgfile_free(fpkg->next_file);
@@ -1227,7 +1238,7 @@
 
 /**
  * @brief Creates an empty struct freebsd_package
- * @return A new creebsd_package object or NULL
+ * @return A new freebsd_package object or NULL
  */
 static struct freebsd_package *
 freebsd_package_new()
@@ -1258,14 +1269,17 @@
 	unsigned int control_size, control_count;
 	struct pkgfile *pkgfile;
 
-/** @todo Check the return of realloc */
 #define addFile(pkgfile) \
 	control_size += sizeof(struct pkgfile **); \
+	size_t pkg_control_size = sizeof(fpkg->control); \
 	fpkg->control = realloc(fpkg->control, control_size); \
+	if (fpkg->control == NULL || \
+	    pkg_control_size == sizeof(fpkg->control)) \
+		return -1; \
 	fpkg->control[control_count] = pkgfile; \
 	control_count++; \
 	fpkg->control[control_count] = NULL;
-	
+
 	assert(fpkg != NULL);
 
 	/* Don't attempt to get the control files again */
@@ -1281,6 +1295,13 @@
 	/* Setup the store to hold all the files */
 	control_size = sizeof(struct pkgfile **);
 	fpkg->control = malloc(control_size);
+
+	/*
+	 * Don't forget -- this can be NULL..
+	 */ 
+	if (fpkg->control == NULL)
+		return -1;
+
 	fpkg->control[0] = NULL;
 	control_count = 0;
 	
@@ -1294,7 +1315,7 @@
 			return -1;
 
 		/* Load all the + files into control */
-		while ((de = readdir(d)) != NULL) {
+		while ( (de = readdir(d)) != NULL ) {
 			char *file;
 	
 			if (de->d_name[0] == '.') {
@@ -1327,14 +1348,17 @@
 
 		return 0;
 	} else if (fpkg->pkg_type == fpkg_from_file) {
+
 		assert(fpkg->archive != NULL);
 		pkgfile = freebsd_get_next_entry(fpkg->archive);
+
 		while (pkgfile_get_name(pkgfile)[0] == '+') {
 			addFile(pkgfile);
 			pkgfile = freebsd_get_next_entry(fpkg->archive);
 		}
 		fpkg->next_file = pkgfile;
 		return 0;
+
 	}
 	assert(0);
 	return -1;