Dan McGee <dpmcgee@gmail.com>

* Removed some unnecessary headers and library links
* Made things static if possible
* Cleaned up makefiles a bit
* Fixed some old comments in the code
* Fixed some errors the static code checker splint pointed out
* Backwards arguments in a memset call in _alpm_db_read (could have been worse)
* Other various small fixes

Other:
* Default to 80 columns when getcols cannot determine display width
* Removal of ._install as a valid install file in packages

bindings/perl/Makefile.in

@@ -34,7 +34,7 @@ install-pm: Core.pm
 	install -m644 $^ $(DESTDIR)$(LIBDIR)/Alpm/
 
 clean:
-	rm -f Core* alpm{.h,_wrap*}
+	rm -f Core* alpm{.h,.i,_wrap*}
 
 distclean: clean
 	rm -f Makefile

bindings/python/Makefile.in

@@ -41,7 +41,7 @@ install-py: alpm.py alpm.pyc
 	install -m644 $^ $(DESTDIR)$(LIBDIR)
 
 clean:
-	rm -f _alpm* alpm{.h,.py*,_wrap*}
+	rm -f _alpm* alpm{.h,.i,.py*,_wrap*}
 
 distclean: clean
 	rm -f Makefile

lib/libalpm/Makefile.am

@@ -40,7 +40,7 @@ include_HEADERS = alpm.h
 libalpm_la_SOURCES = $(TARGETS)
 
 libalpm_la_LDFLAGS = -no-undefined -version-info $(PM_VERSION_INFO)
-libalpm_la_LIBADD = -ldownload
+libalpm_la_LIBADD = -larchive -ldownload
 
 if HAS_DOXYGEN
 all: doxygen.in

lib/libalpm/alpm.c

@@ -137,7 +137,7 @@ int alpm_release()
 
 /** Register a package database
  * @param treename the name of the repository
- * @return 0 on success, -1 on error (pm_errno is set accordingly)
+ * @return a pmdb_t* on success (the value), NULL on error
  */
 pmdb_t *alpm_db_register(char *treename)
 {

lib/libalpm/be_files.c

@@ -180,7 +180,7 @@ int _alpm_db_read(pmdb_t *db, unsigned int inforeq, pmpkg_t *info)
 	FILE *fp = NULL;
 	struct stat buf;
 	char path[PATH_MAX+1];
-	char line[513] = {0};
+	char line[513];
 	pmlist_t *tmplist;
 	char *locale;
 
@@ -205,7 +205,7 @@ int _alpm_db_read(pmdb_t *db, unsigned int inforeq, pmpkg_t *info)
 	_alpm_log(PM_LOG_FUNCTION, _("loading package data for %s : level=%d"), info->name, inforeq);
 
 	/* clear out 'line', to be certain - and to make valgrind happy */
-	memset(line, 513, 0);
+	memset(line, 0, 513);
 
 	snprintf(path, PATH_MAX, "%s/%s-%s", db->path, info->name, info->version);
 	if(stat(path, &buf)) {
@@ -528,20 +528,20 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, unsigned int inforeq)
 			}
 			if(info->size) {
 				fprintf(fp, "%%SIZE%%\n"
-					"%ld\n\n", info->size);
+					"%lu\n\n", info->size);
 			}
 			if(info->reason) {
 				fprintf(fp, "%%REASON%%\n"
-					"%d\n\n", info->reason);
+					"%u\n\n", info->reason);
 			}
 		} else {
 			if(info->size) {
 				fprintf(fp, "%%CSIZE%%\n"
-					"%ld\n\n", info->size);
+					"%lu\n\n", info->size);
 			}
 			if(info->isize) {
 				fprintf(fp, "%%ISIZE%%\n"
-					"%ld\n\n", info->isize);
+					"%lu\n\n", info->isize);
 			}
 			if(info->sha1sum) {
 				fprintf(fp, "%%SHA1SUM%%\n"

lib/libalpm/conflict.c

@@ -206,6 +206,10 @@ pmlist_t *_alpm_checkconflicts(pmdb_t *db, pmlist_t *packages)
 	return(baddeps);
 }
 
+/* Returns a pmlist_t* of file conflicts.
+ *
+ * adds list of files to skip to pmlist_t** skip_list.
+ */
 pmlist_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root, pmlist_t **skip_list)
 {
 	pmlist_t *i, *j, *k;
@@ -230,10 +234,10 @@ pmlist_t *_alpm_db_find_conflicts(pmdb_t *db, pmtrans_t *trans, char *root, pmli
 				for(k = p1->files; k; k = k->next) {
 					filestr = k->data;
 					if(filestr[strlen(filestr)-1] == '/') {
-						/* this filename has a trailing '/', so it's a directory -- skip it. */
+						/* has a trailing '/', so it's a directory -- skip it. */
 						continue;
 					}
-					if(!strcmp(filestr, "._install") || !strcmp(filestr, ".INSTALL")) {
+					if(strcmp(filestr, ".INSTALL") == 0) {
 						continue;
 					}
 					if(_alpm_list_is_strin(filestr, p2->files)) {

lib/libalpm/deps.c

@@ -119,6 +119,10 @@ pmlist_t *_alpm_sortbydeps(pmlist_t *targets, int mode)
 	while(change) {
 		pmlist_t *tmptargs = NULL;
 		change = 0;
+		/* TODO only use of a math.h function in entire libalpm,
+		 *      can we get rid of it? Former code line:
+		 *if(numscans > numtargs) {
+		 */
 		if(numscans > sqrt(numtargs)) {
 			_alpm_log(PM_LOG_DEBUG, _("possible dependency cycle detected"));
 			continue;

lib/libalpm/error.c

@@ -134,7 +134,7 @@ char *alpm_strerror(int err)
 		case PM_ERR_CONF_DIRECTIVE_OUTSIDE_SECTION:
 			return _("all directives must belong to a section");
 		case PM_ERR_INVALID_REGEX:
-			return _("valid regular expression");
+			return _("invalid regular expression");
 		case PM_ERR_CONNECT_FAILED:
 			return _("connection to remote host failed");
 		case PM_ERR_FORK_FAILED:

lib/libalpm/handle.c

@@ -27,7 +27,6 @@
 #include <unistd.h>
 #include <limits.h>
 #include <sys/types.h>
-#include <stdarg.h>
 #include <syslog.h>
 #include <libintl.h>
 #include <time.h>

lib/libalpm/list.c

@@ -23,7 +23,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
-#include <assert.h>
 /* pacman */
 #include "list.h"
 #include "util.h"
@@ -307,8 +306,6 @@ pmlist_t *_alpm_list_last(pmlist_t *list)
 		return(NULL);
 	}
 
-	assert(list->last != NULL);
-
 	return(list->last);
 }
 

lib/libalpm/md5driver.c

@@ -21,7 +21,6 @@ documentation and/or software.
 
 #include <stdlib.h>
 #include <stdio.h>
-#include <time.h>
 #include <string.h>
 #include <libintl.h>
 #include "util.h"

lib/libalpm/package.c

@@ -26,7 +26,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <limits.h>
-#include <fcntl.h>
 #include <string.h>
 #include <libintl.h>
 #include <locale.h>

lib/libalpm/server.c

@@ -27,7 +27,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <time.h>
-#include <sys/time.h>
+#include <download.h>
 
 /* pacman */
 #include "server.h"

lib/libalpm/sha1.c

@@ -33,7 +33,6 @@
 #include <sys/types.h>
 
 #include <stdlib.h>
-#include <time.h>
 #include <string.h>
 
 /*

lib/libalpm/sha1.h

@@ -20,9 +20,14 @@
 #include <limits.h>
 
 #define rol(x,n) ( ((x) << (n)) | ((x) >> (32 -(n))) )
+/* TODO check this comment */
 /* The code below is from md5.h (from coreutils), little modifications */
 #define UINT_MAX_32_BITS 4294967295U
 
+/* This new ifdef allows splint to not fail on its static code check */
+#ifdef S_SPLINT_S
+		typedef unsigned int sha_uint32;
+#else
 #if UINT_MAX == UINT_MAX_32_BITS
     typedef unsigned int sha_uint32;
 #else
@@ -34,9 +39,10 @@
 #else
     /* The following line is intended to evoke an error. Using #error is not portable enough.  */
 #error "Cannot determine unsigned 32-bit data type"
-#endif
-#endif
-#endif
+#endif /* ULONG_MAX */
+#endif /* USHRT_MAX */
+#endif /* UINT_MAX */
+#endif /* S_SPLINT_S */
 /* We have to make a guess about the integer type equivalent in size
    to pointers which should always be correct.  */
 typedef unsigned long int sha_uintptr;

lib/libalpm/sync.c

@@ -121,7 +121,8 @@ static int istoonew(pmpkg_t *pkg)
 /* Find recommended replacements for packages during a sync.
  * (refactored from _alpm_sync_prepare)
  */
-static int find_replacements(pmtrans_t *trans, pmdb_t *db_local, pmlist_t *dbs_sync)
+static int find_replacements(pmtrans_t *trans, pmdb_t *db_local,
+														 pmlist_t *dbs_sync)
 {
 	pmlist_t *i, *j, *k;
 

lib/libalpm/util.c

@@ -36,7 +36,6 @@
 #ifdef __sun__
 #include <alloca.h>
 #endif
-#include <stdarg.h>
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>

src/pacman/Makefile.am

@@ -17,9 +17,9 @@ pacman_SOURCES = util.c log.c list.c package.c downloadprog.c trans.c add.c \
 pacman_static_SOURCES = $(pacman_SOURCES)
 
 pacman_LDADD = -L$(top_srcdir)/lib/libalpm/.libs \
-			   -ldownload -larchive -lm -lalpm -lssl -lcrypto
+			   -ldownload -lm -lalpm
 
 pacman_static_LDADD = -L$(top_srcdir)/lib/libalpm/.libs/ \
-					  -ldownload -larchive -lm -lalpm -lssl -lcrypto
+					  -ldownload -lm -lalpm
 
 pacman_static_LDFLAGS = $(LDFLAGS) -all-static

src/pacman/downloadprog.c

@@ -39,10 +39,10 @@
 #include "conf.h"
 
 /* progress bar */
-float rate_last;
-int xfered_last;
-struct timeval last_time;
-struct timeval initial_time;
+static float rate_last;
+static int xfered_last;
+static struct timeval last_time;
+static struct timeval initial_time;
 
 /* pacman options */
 extern config_t *config;
@@ -52,13 +52,13 @@ extern config_t *config;
 
 void log_progress(const char *filename, int xfered, int total)
 {
-	static int lasthash = 0, mouth = 0;
-	int i, hash;
-	long chomp = 0;
+	static unsigned int lasthash = 0, mouth = 0;
+	unsigned int i, hash;
+	unsigned int chomp = 0;
 	char *fname, *p; 
 	unsigned int maxcols = getcols();
 	unsigned int progresslen = maxcols - 57;
-	int percent = ((float)xfered) / ((float)total) * 100;
+	int percent = (int)((float)xfered) / ((float)total) * 100;
 	struct timeval current_time;
 	float rate = 0.0;
 	unsigned int eta_h = 0, eta_m = 0, eta_s = 0;
@@ -87,17 +87,17 @@ void log_progress(const char *filename, int xfered, int total)
 
 	if(xfered == total) {
 		/* compute final values */
-		rate = total / (total_timediff * 1024);
-		eta_s = (int)total_timediff;
+		rate = (float)total / (total_timediff * 1024);
+		eta_s = (unsigned int)total_timediff;
 		set_output_padding(0); /* shut off padding */
 	} else if(timediff < UPDATE_SPEED_SEC) {
 	/* we avoid computing the ETA on too small periods of time, so that
 		 results are more significant */
 		return;
 	} else {
-		rate = (xfered - xfered_last) / (timediff * 1024);
-		rate = (rate + 2*rate_last) / 3;
-		eta_s = (total - xfered) / (rate * 1024);
+		rate = (float)(xfered - xfered_last) / (timediff * 1024);
+		rate = (float)(rate + 2*rate_last) / 3;
+		eta_s = (unsigned int)(total - xfered) / (rate * 1024);
 	}
 
 	rate_last = rate;
@@ -130,11 +130,12 @@ void log_progress(const char *filename, int xfered, int total)
 		rate = 9999.9;
 	}
 
-	printf(" %-*s %6dK %#6.1fK/s %02d:%02d:%02d [", FILENAME_TRIM_LEN, fname, xfered/1024, rate, eta_h, eta_m, eta_s);
+	printf(" %-*s %6dK %#6.1fK/s %02u:%02u:%02u [", FILENAME_TRIM_LEN, fname, 
+				 xfered/1024, rate, eta_h, eta_m, eta_s);
 
 	free(fname);
 	
-	hash = percent*progresslen/100;
+	hash = (unsigned int)percent*progresslen/100;
 	for(i = progresslen; i > 0; --i) {
 		if(chomp) {
 			if(i > progresslen - hash) {

src/pacman/log.c

@@ -38,8 +38,8 @@
 
 extern config_t *config;
 
-int neednl = 0; /* for cleaner message output */
-int needpad = 0; /* pad blanks to terminal width */
+static int neednl = 0; /* for cleaner message output */
+static int needpad = 0; /* pad blanks to terminal width */
 
 /* simple helper for needpad */
 void set_output_padding(int on)
@@ -131,8 +131,8 @@ void pm_fprintf(FILE *file, unsigned short line, char *fmt, ...)
 
 	fprintf(file, str);
 	if(needpad == 1) {
-		unsigned int cols = getcols();
-		for(int i=len; i < cols; ++i) {
+		unsigned int i, cols = getcols();
+		for(i=len; i < cols; ++i) {
 			fprintf(file, " ");
 		}
 		if(neednl == 1) {

src/pacman/package.c

@@ -218,7 +218,7 @@ void dump_pkg_changelog(char *clfile, const char *pkgname)
 	{
 		while(!feof(fp))
 		{
-			fgets(line, PATH_MAX, fp);
+			fgets(line, (int)PATH_MAX, fp);
 			printf("%s", line);
 			line[0] = '\0';
 		}

src/pacman/pacman.c

@@ -72,15 +72,13 @@ enum {
 	PM_OP_DEPTEST
 };
 
-config_t *config = NULL;
+config_t *config;
 
 pmdb_t *db_local;
 /* list of (sync_t *) structs for sync locations */
-list_t *pmc_syncs = NULL;
+list_t *pmc_syncs;
 /* list of targets specified on command line */
-list_t *pm_targets  = NULL;
-
-unsigned int maxcols = 80;
+static list_t *pm_targets;
 
 extern int neednl;
 
@@ -134,8 +132,7 @@ static void usage(int op, char *myname)
 			printf(_("  -l, --list          list the contents of the queried package\n"));
 			printf(_("  -m, --foreign       list all packages that were not found in the sync db(s)\n"));
 			printf(_("  -o, --owns <file>   query the package that owns <file>\n"));
-			printf(_("  -p, --file          pacman will query the package file [package] instead of\n"));
-			printf(_("                      looking in the database\n"));
+			printf(_("  -p, --file          query the package file [package] instead of the database\n"));
 			printf(_("  -s, --search        search locally-installed packages for matching strings\n"));
 		} else if(op == PM_OP_SYNC) {
 			printf(_("usage:  %s {-S --sync} [options] [package]\n"), myname);
@@ -278,7 +275,7 @@ static int parseargs(int argc, char *argv[])
 		{0, 0, 0, 0}
 	};
 	char root[PATH_MAX];
-	struct stat st = {0};
+	struct stat st;
 
 	while((opt = getopt_long(argc, argv, "ARUFQSTDYr:b:vkhscVfmnoldepiuwyg", opts, &option_index))) {
 		if(opt < 0) {
@@ -325,7 +322,7 @@ static int parseargs(int argc, char *argv[])
 			case 'b':
 			  if(stat(optarg, &st) == -1 || !S_ISDIR(st.st_mode)) {
 					pm_fprintf(stderr, NL, _("error: '%s' is not a valid db path\n"), optarg);
-					exit(1);
+					exit(EXIT_FAILURE);
 				}
 				alpm_option_set_dbpath(optarg);
 				config->dbpath = alpm_option_get_dbpath(optarg);
@@ -481,7 +478,7 @@ int main(int argc, char *argv[])
 			} else {
 				ERR(NL, _("you cannot perform this operation unless you are root.\n"));
 				config_free(config);
-				exit(1);
+				exit(EXIT_FAILURE);
 			}
 		}
 	}

src/pacman/sync.c

@@ -212,6 +212,12 @@ static int sync_synctree(int level, list_t *syncs)
 		if(ret < 0) {
 			if(pm_errno == PM_ERR_DB_SYNC) {
 				/* use libdownload error */
+				/* TODO breaking abstraction barrier here?
+				 *			pacman -> libalpm -> libdownload
+				 *
+				 * Yes.  This will be here until we add a nice pacman "pm_errstr" or
+				 * something, OR add all libdownload error codes into the pm_error enum
+				 */
 				ERR(NL, _("failed to synchronize %s: %s\n"), sync->treename, downloadLastErrString);
 			} else {
 				ERR(NL, _("failed to update %s (%s)\n"), sync->treename, alpm_strerror(pm_errno));

src/pacman/trans.c

@@ -41,7 +41,7 @@
 
 extern config_t *config;
 
-int prevpercent=0; /* for less progressbar output */
+static int prevpercent=0; /* for less progressbar output */
 
 /* Callback to handle transaction events
  */

src/pacman/util.c

@@ -30,7 +30,6 @@
 #include "config.h"
 #include <stdio.h>
 #include <stdlib.h>
-#include <stdarg.h>
 #include <string.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -52,27 +51,28 @@ extern config_t *config;
 extern int neednl;
 
 /* gets the current screen column width */
-int getcols()
+unsigned int getcols()
 {
 	if(!isatty(1)) {
 		/* We will default to 80 columns if we're not a tty
 		 * this seems a fairly standard file width.
 		 */
 		return 80;
-	}
+	} else {
 #ifdef TIOCGSIZE
-	struct ttysize win;
-	if(ioctl(1, TIOCGSIZE, &win) == 0) {
-		return win.ts_cols;
-	}
+		struct ttysize win;
+		if(ioctl(1, TIOCGSIZE, &win) == 0) {
+			return win.ts_cols;
+		}
 #elif defined(TIOCGWINSZ)
-	struct winsize win;
-	if(ioctl(1, TIOCGWINSZ, &win) == 0) {
-		return win.ws_col;
-	}
+		struct winsize win;
+		if(ioctl(1, TIOCGWINSZ, &win) == 0) {
+			return win.ws_col;
+		}
 #endif
-	else {
-		return -1;
+		/* If we can't figure anything out, we'll just assume 80 columns */
+		/* TODO any problems caused by this assumption? */
+		return 80;
 	}
 	/* Original envvar way - prone to display issues
 	const char *cenv = getenv("COLUMNS");
@@ -155,15 +155,14 @@ int rmrf(char *path)
 		}
 		return(errflag);
 	}
-	return(0);
 }
 
 /* output a string, but wrap words properly with a specified indentation
  */
-void indentprint(const char *str, int indent)
+void indentprint(const char *str, unsigned int indent)
 {
 	const char *p = str;
-	int cidx = indent;
+	unsigned int cidx = indent;
 
 	while(*p) {
 		if(*p == ' ') {
@@ -178,7 +177,7 @@ void indentprint(const char *str, int indent)
 			len = next - p;
 			if(len > (getcols()-cidx-1)) {
 				/* newline */
-				int i;
+				unsigned int i;
 				fprintf(stdout, "\n");
 				for(i = 0; i < indent; i++) {
 					fprintf(stdout, " ");
@@ -200,7 +199,7 @@ void indentprint(const char *str, int indent)
 char *buildstring(list_t *strlist)
 {
 	char *str;
-	int size = 1;
+	size_t size = 1;
 	list_t *lp;
 
 	for(lp = strlist; lp; lp = lp->next) {
@@ -208,7 +207,7 @@ char *buildstring(list_t *strlist)
 	}
 	str = (char *)malloc(size);
 	if(str == NULL) {
-		ERR(NL, _("failed to allocated %d bytes\n"), size);
+		ERR(NL, _("failed to allocate %d bytes\n"), size);
 	}
 	str[0] = '\0';
 	for(lp = strlist; lp; lp = lp->next) {

src/pacman/util.h

@@ -28,7 +28,7 @@
 		p = malloc(b); \
 		if (!(p)) { \
 			fprintf(stderr, "malloc failure: could not allocate %d bytes\n", (int)b); \
-			exit(1); \
+			exit(EXIT_FAILURE); \
 		} \
 	} else { \
 		p = NULL; \
@@ -43,10 +43,10 @@
 } while(0)
 
 #define _(str) gettext(str)
-int getcols();
+unsigned int getcols();
 int makepath(char *path);
 int rmrf(char *path);
-void indentprint(const char *str, int indent);
+void indentprint(const char *str, unsigned int indent);
 char *buildstring(list_t *strlist);
 char *strtoupper(char *str);
 char *strtrim(char *str);