Hi, I tried to clean up lib/libalpm/util.[ch]. It's not yet complete, I would like to have some comments on it first. Some notes: There has to be a proper way handling out-of-memory. The lib can't do anything anymore without memory - am I right? _alpm_runscriptlet is really long and I'm going to split it. So, here goes the patch: Signed-off-by: Johannes Weiner <hannes@saeurebad.de> diff -Naur pacman-lib.old/lib/libalpm/util.c pacman-lib/lib/libalpm/util.c --- pacman-lib.old/lib/libalpm/util.c 2007-01-24 16:09:16.000000000 +0100 +++ pacman-lib/lib/libalpm/util.c 2007-01-24 16:10:13.000000000 +0100 @@ -64,9 +64,49 @@ #include "package.h" #include "alpm.h" +/* Memory allocation frontends with return value check. + * TODO: Think about an elegant way to die on failure */ +static void *safe_mem(void *r) +{ + if (r) { + return (r); + } + + _alpm_log(PM_LOG_ERROR, "Memory exhausted.\n"); + + /* TODO: This is probably the wrong way */ + alpm_release(); + exit(2); +} + +void *pmalloc(size_t s) +{ + return (safe_mem(malloc(s))); +} + +void *pcalloc(size_t e, size_t s) +{ + return (safe_mem(calloc(e, s))); +} + +void *prealloc(void *p, size_t s) +{ + return (safe_mem(realloc(p, s))); +} + +char *pstrdup(const char *s) +{ + return (safe_mem((char *) strdup(s))); +} + +void *palloca(size_t s) +{ + return (safe_mem(alloca(s))); +} + #ifdef __sun__ /* This is a replacement for strsep which is not portable (missing on Solaris). - * Copyright (c) 2001 by François Gouget <fgouget_at_codeweavers.com> */ + * Copyright (c) 2001 by Fran�ois Gouget <fgouget_at_codeweavers.com> */ char* strsep(char** str, const char* delims) { char* token; @@ -92,13 +132,12 @@ /* Backported from Solaris Express 4/06 * Copyright (c) 2006 Sun Microsystems, Inc. */ -char * mkdtemp(char *template) +char * mkdtemp(const char *template) { - char *t = alloca(strlen(template) + 1); - char *r; + char *r, *t = palloca(strlen(template) + 1); /* Save template */ - (void) strcpy(t, template); + strcpy(t, template); for (; ; ) { r = mktemp(template); @@ -113,7 +152,7 @@ return (NULL); /* Reset template */ - (void) strcpy(template, t); + strcpy(template, t); } } #endif @@ -121,14 +160,16 @@ /* does the same thing as 'mkdir -p' */ int _alpm_makepath(char *path) { - char *orig, *str, *ptr; - char full[PATH_MAX] = ""; mode_t oldmask; + char *orig, *str, *ptr, full[PATH_MAX]; oldmask = umask(0000); - orig = strdup(path); + memset(full, 0, sizeof(full)); + + orig = pstrdup(path); str = orig; + while((ptr = strsep(&str, "/"))) { if(strlen(ptr)) { struct stat buf; @@ -149,32 +190,46 @@ return(0); } -int _alpm_copyfile(char *src, char *dest) +int _alpm_copyfile(const char *src, const char *dest) { - FILE *in, *out; - size_t len; - char buf[4097]; + FILE *fp; + char *buf; + struct stat sbuf; - in = fopen(src, "r"); - if(in == NULL) { + if (stat(src, &sbuf)) { return(1); } - out = fopen(dest, "w"); - if(out == NULL) { - fclose(in); + + if (!(fp = fopen(src, "r"))) { return(1); } - while((len = fread(buf, 1, 4096, in))) { - fwrite(buf, 1, len, out); + buf = pmalloc(sbuf.st_size); + + if (fread(buf, sbuf.st_size, 1, fp) != 1) { + FREE(buf); + return(1); + } + + fclose(fp); + + if (!(fp = fopen(dest, "w"))) { + FREE(buf); + return(1); + } + + if (fwrite(buf, sbuf.st_size, 1, fp) != 1) { + FREE(buf); + fclose(fp); + return(1); } - fclose(in); - fclose(out); + FREE(buf); + fclose(fp); return(0); } -/* Convert a string to uppercase +/* Convert a string to uppercase destructively */ char *_alpm_strtoupper(char *str) { @@ -187,7 +242,7 @@ return str; } -/* Trim whitespace and newlines from a string +/* Trim whitespace and newlines from a string destructively */ char *_alpm_strtrim(char *str) { @@ -210,8 +265,8 @@ return(str); } - pch = (char *)(str + (strlen(str) - 1)); - while(isspace((int)*pch)) { + pch = (str + (strlen(str) - 1)); + while(isspace(*pch)) { pch--; } *++pch = '\0'; @@ -221,35 +276,36 @@ /* Create a lock file */ -int _alpm_lckmk(char *file) +int _alpm_lckmk(const char *file) { int fd, count = 0; char *dir, *ptr; /* create the dir of the lockfile first */ - dir = strdup(file); + dir = pstrdup(file); ptr = strrchr(dir, '/'); if(ptr) { *ptr = '\0'; } _alpm_makepath(dir); - while((fd = open(file, O_WRONLY | O_CREAT | O_EXCL, 0000)) == -1 && errno == EACCES) { + while((fd = open(file, O_WRONLY | O_CREAT | O_EXCL, 0000) == -1) && + (errno == EACCES)) { if(++count < 1) { sleep(1); - } else { + } else { return(-1); } } - free(dir); + FREE(dir); return(fd > 0 ? fd : -1); } /* Remove a lock file */ -int _alpm_lckrm(char *file) +int _alpm_lckrm(const char *file) { if(unlink(file) == -1 && errno != ENOENT) { return(-1); @@ -272,8 +328,12 @@ archive_read_support_compression_all(_archive); archive_read_support_format_all(_archive); - if(archive_read_open_file(_archive, archive, ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { - _alpm_log(PM_LOG_ERROR, _("could not open %s: %s\n"), archive, archive_error_string(_archive)); + if(archive_read_open_file(_archive, + archive, + ARCHIVE_DEFAULT_BYTES_PER_BLOCK) + != ARCHIVE_OK) { + _alpm_log(PM_LOG_ERROR, _("could not open %s: %s\n"), archive, + archive_error_string(_archive)); RET_ERR(PM_ERR_PKG_OPEN, -1); } @@ -283,10 +343,18 @@ return(1); continue; } - snprintf(expath, PATH_MAX, "%s/%s", prefix, archive_entry_pathname(entry)); + + snprintf(expath, PATH_MAX, "%s/%s", prefix, + archive_entry_pathname(entry)); archive_entry_set_pathname(entry, expath); - if(archive_read_extract(_archive, entry, ARCHIVE_EXTRACT_FLAGS) != ARCHIVE_OK) { - _alpm_log(PM_LOG_ERROR, _("could not extract %s: %s\n"), archive_entry_pathname(entry), archive_error_string(_archive)); + + if(archive_read_extract(_archive, + entry, + ARCHIVE_EXTRACT_FLAGS) + != ARCHIVE_OK) { + _alpm_log(PM_LOG_ERROR, _("could not extract %s: %s\n"), + archive_entry_pathname(entry), + archive_error_string(_archive)); return(1); } @@ -300,49 +368,53 @@ } /* does the same thing as 'rm -rf' */ -int _alpm_rmrf(char *path) +int _alpm_rmrf(const char *path) { int errflag = 0; struct dirent *dp; DIR *dirp; char name[PATH_MAX]; - struct stat st; + struct stat st; - if(stat(path, &st) == 0) { - if(S_ISREG(st.st_mode)) { - if(!unlink(path)) { - return(0); - } else { - if(errno == ENOENT) { - return(0); - } else { - /* not a directory */ - return(1); - } - } - } else if(S_ISDIR(st.st_mode)) { - if((dirp = opendir(path)) == (DIR *)-1) { - return(1); - } - for(dp = readdir(dirp); dp != NULL; dp = readdir(dirp)) { - if(dp->d_ino) { - sprintf(name, "%s/%s", path, dp->d_name); - if(strcmp(dp->d_name, "..") && strcmp(dp->d_name, ".")) { - errflag += _alpm_rmrf(name); - } + if(stat(path, &st)) { + return(0); + } + + if(S_ISREG(st.st_mode)) { + if(!unlink(path)) { + return(0); + } + + if(errno == ENOENT) { + return(0); + } else { + /* not a directory */ + return(1); + } + } else if(S_ISDIR(st.st_mode)) { + if(!(dirp = opendir(path))) { + return(1); + } + for(dp = readdir(dirp); dp != NULL; dp = readdir(dirp)) { + if(dp->d_ino) { + sprintf(name, "%s/%s", path, dp->d_name); + if(strcmp(dp->d_name, "..") && + strcmp(dp->d_name, ".")) { + errflag += _alpm_rmrf(name); } } - closedir(dirp); - if(rmdir(path)) { - errflag++; - } } - return(errflag); + closedir(dirp); + + if(rmdir(path)) { + errflag++; + } } - return(0); + + return(errflag); } -int _alpm_logaction(unsigned short usesyslog, FILE *f, const char *str) +int _alpm_logaction(unsigned char usesyslog, FILE *f, const char *str) { _alpm_log(PM_LOG_DEBUG, _("logaction called: %s"), str); @@ -387,37 +459,39 @@ /* A cheap grep for text files, returns 1 if a substring * was found in the text file fn, 0 if it wasn't + * + * TODO: Remove hardcoded buffersize */ static int grep(const char *fn, const char *needle) { FILE *fp; + char line[1024]; if((fp = fopen(fn, "r")) == NULL) { return(0); } - while(!feof(fp)) { - char line[1024]; - fgets(line, 1024, fp); - if(feof(fp)) { - continue; - } + + while(fgets(line, 1024, fp)) { if(strstr(line, needle)) { fclose(fp); return(1); } } + fclose(fp); return(0); } -int _alpm_runscriptlet(char *root, char *installfn, char *script, char *ver, char *oldver, pmtrans_t *trans) +/* + * TODO: This function is WAY too complex and long!!! + */ +int _alpm_runscriptlet(const char *root, const char *installfn, + const char *script, const char *ver, + const char *oldver, pmtrans_t *trans) { - char scriptfn[PATH_MAX]; - char cmdline[PATH_MAX]; - char tmpdir[PATH_MAX] = ""; - char *scriptpath; + char scriptfn[PATH_MAX], cmdline[PATH_MAX], tmpdir[PATH_MAX]; + char *scriptpath, cwd[PATH_MAX] = ""; struct stat buf; - char cwd[PATH_MAX] = ""; pid_t pid; int retval = 0; @@ -433,7 +507,8 @@ } snprintf(tmpdir, PATH_MAX, "%stmp/alpm_XXXXXX", root); if(mkdtemp(tmpdir) == NULL) { - _alpm_log(PM_LOG_ERROR, _("could not create temp directory")); + _alpm_log(PM_LOG_ERROR, + _("could not create temp directory")); return(1); } _alpm_unpack(installfn, tmpdir, ".INSTALL"); @@ -453,14 +528,18 @@ /* save the cwd so we can restore it later */ if(getcwd(cwd, PATH_MAX) == NULL) { - _alpm_log(PM_LOG_ERROR, _("could not get current working directory")); - /* in case of error, cwd content is undefined: so we set it to something */ + _alpm_log(PM_LOG_ERROR, + _("could not get current working directory")); + /* in case of error, cwd content is undefined: + * so we set it to something */ cwd[0] = 0; } /* just in case our cwd was removed in the upgrade operation */ if(chdir(root) != 0) { - _alpm_log(PM_LOG_ERROR, _("could not change directory to %s (%s)"), root, strerror(errno)); + _alpm_log(PM_LOG_ERROR, + _("could not change directory to %s (%s)"), + root, strerror(errno)); } _alpm_log(PM_LOG_FLOW2, _("executing %s script..."), script); @@ -476,52 +555,70 @@ pid = fork(); if(pid == -1) { - _alpm_log(PM_LOG_ERROR, _("could not fork a new process (%s)"), strerror(errno)); + _alpm_log(PM_LOG_ERROR, + _("could not fork a new process (%s)"), + strerror(errno)); retval = 1; goto cleanup; } if(pid == 0) { FILE *pp; + char line[1024]; _alpm_log(PM_LOG_DEBUG, _("chrooting in %s"), root); if(chroot(root) != 0) { - _alpm_log(PM_LOG_ERROR, _("could not change the root directory (%s)"), strerror(errno)); + _alpm_log(PM_LOG_ERROR, + _("could not change the root directory (%s)"), + strerror(errno)); return(1); } if(chdir("/") != 0) { - _alpm_log(PM_LOG_ERROR, _("could not change directory to / (%s)"), strerror(errno)); + _alpm_log(PM_LOG_ERROR, + _("could not change directory to / (%s)"), + strerror(errno)); return(1); } umask(0022); _alpm_log(PM_LOG_DEBUG, _("executing \"%s\""), cmdline); pp = popen(cmdline, "r"); if(!pp) { - _alpm_log(PM_LOG_ERROR, _("call to popen failed (%s)"), strerror(errno)); + _alpm_log(PM_LOG_ERROR, + _("call to popen failed (%s)"), + strerror(errno)); retval = 1; goto cleanup; } - while(!feof(pp)) { - char line[1024]; - if(fgets(line, 1024, pp) == NULL) - break; + while(fgets(line, 1024, pp)) { + size_t slen, dlen; + slen = strlen(SCRIPTLET_START); + dlen = strlen(SCRIPTLET_DONE); + /* "START <event desc>" */ - if((strlen(line) > strlen(SCRIPTLET_START)) && !strncmp(line, SCRIPTLET_START, strlen(SCRIPTLET_START))) { - EVENT(trans, PM_TRANS_EVT_SCRIPTLET_START, _alpm_strtrim(line + strlen(SCRIPTLET_START)), NULL); + if((strlen(line) > slen) && + !strncmp(line, SCRIPTLET_START, slen)) { + EVENT(trans, PM_TRANS_EVT_SCRIPTLET_START, + _alpm_strtrim(line + slen), NULL); /* "DONE <ret code>" */ - } else if((strlen(line) > strlen(SCRIPTLET_DONE)) && !strncmp(line, SCRIPTLET_DONE, strlen(SCRIPTLET_DONE))) { - EVENT(trans, PM_TRANS_EVT_SCRIPTLET_DONE, (void*)atol(_alpm_strtrim(line + strlen(SCRIPTLET_DONE))), NULL); + } else if((strlen(line) > dlen) && + !strncmp(line, SCRIPTLET_DONE, dlen)) { + EVENT(trans, PM_TRANS_EVT_SCRIPTLET_DONE, + (void *)atol(_alpm_strtrim(line + dlen)), + NULL); } else { _alpm_strtrim(line); /* log our script output */ alpm_logaction(line); - EVENT(trans, PM_TRANS_EVT_SCRIPTLET_INFO, line, NULL); + EVENT(trans, PM_TRANS_EVT_SCRIPTLET_INFO, + line, NULL); } } pclose(pp); exit(0); } else { if(waitpid(pid, 0, 0) == -1) { - _alpm_log(PM_LOG_ERROR, _("call to waitpid failed (%s)"), strerror(errno)); + _alpm_log(PM_LOG_ERROR, + _("call to waitpid failed (%s)"), + strerror(errno)); retval = 1; goto cleanup; } @@ -529,14 +626,16 @@ cleanup: if(strlen(tmpdir) && _alpm_rmrf(tmpdir)) { - _alpm_log(PM_LOG_WARNING, _("could not remove tmpdir %s"), tmpdir); + _alpm_log(PM_LOG_WARNING, + _("could not remove tmpdir %s"), + tmpdir); } if(strlen(cwd)) { chdir(cwd); } return(retval); -} +} /* Thank goddess, I'm through. */ #ifndef __sun__ static long long get_freespace() @@ -584,23 +683,16 @@ } } freespace = get_freespace(); - _alpm_log(PM_LOG_DEBUG, _("check_freespace: total pkg size: %lld, disk space: %lld"), pkgsize, freespace); + _alpm_log(PM_LOG_DEBUG, + _("check_freespace: total pkg size: %lld, disk space: %lld"), + pkgsize, freespace); if(pkgsize > freespace) { if(data) { - long long *ptr; - if((ptr = (long long*)malloc(sizeof(long long)))==NULL) { - _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes"), sizeof(long long)); - pm_errno = PM_ERR_MEMORY; - return(-1); - } + long long *ptr = pmalloc(sizeof(long long)); *ptr = pkgsize; *data = alpm_list_add(*data, ptr); - if((ptr = (long long*)malloc(sizeof(long long)))==NULL) { - _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes"), sizeof(long long)); - FREELIST(*data); - pm_errno = PM_ERR_MEMORY; - return(-1); - } + + ptr = pmalloc(sizeof(long long)); *ptr = freespace; *data = alpm_list_add(*data, ptr); } @@ -621,8 +713,8 @@ struct tm *lt; lt = localtime(&t); sprintf(buffer, "%4d%02d%02d%02d%02d%02d", - lt->tm_year+1900, lt->tm_mon+1, lt->tm_mday, - lt->tm_hour, lt->tm_min, lt->tm_sec); + lt->tm_year+1900, lt->tm_mon+1, lt->tm_mday, + lt->tm_hour, lt->tm_min, lt->tm_sec); buffer[14] = '\0'; } } diff -Naur pacman-lib.old/lib/libalpm/util.h pacman-lib/lib/libalpm/util.h --- pacman-lib.old/lib/libalpm/util.h 2007-01-24 16:09:16.000000000 +0100 +++ pacman-lib/lib/libalpm/util.h 2007-01-24 16:10:13.000000000 +0100 @@ -43,7 +43,9 @@ s1[(len)-1] = 0; \ } while(0) -#define ARCHIVE_EXTRACT_FLAGS ARCHIVE_EXTRACT_OWNER | ARCHIVE_EXTRACT_PERM | ARCHIVE_EXTRACT_TIME +#define ARCHIVE_EXTRACT_FLAGS (ARCHIVE_EXTRACT_OWNER \ + | ARCHIVE_EXTRACT_PERM \ + | ARCHIVE_EXTRACT_TIME) #ifdef ENABLE_NLS #define _(str) dgettext ("libalpm", str) @@ -54,18 +56,25 @@ #define SCRIPTLET_START "START " #define SCRIPTLET_DONE "DONE " +void *pmalloc(size_t s); +void *pcalloc(size_t e, size_t s); +void *prealloc(void *p, size_t s); +char *pstrdup(const char *s); +void *palloca(size_t s); int _alpm_makepath(char *path); -int _alpm_copyfile(char *src, char *dest); +int _alpm_copyfile(const char *src, const char *dest); char *_alpm_strtoupper(char *str); char *_alpm_strtrim(char *str); -int _alpm_lckmk(char *file); -int _alpm_lckrm(char *file); +int _alpm_lckmk(const char *file); +int _alpm_lckrm(const char *file); int _alpm_unpack(const char *archive, const char *prefix, const char *fn); -int _alpm_rmrf(char *path); -int _alpm_logaction(unsigned short usesyslog, FILE *f, const char *str); +int _alpm_rmrf(const char *path); +int _alpm_logaction(unsigned char usesyslog, FILE *f, const char *str); int _alpm_ldconfig(char *root); #ifdef _ALPM_TRANS_H -int _alpm_runscriptlet(char *util, char *installfn, char *script, char *ver, char *oldver, pmtrans_t *trans); +int _alpm_runscriptlet(const char *util, const char *installfn, + const char *script, const char *ver, + const char *oldver, pmtrans_t *trans); #ifndef __sun__ int _alpm_check_freespace(pmtrans_t *trans, alpm_list_t **data); #endif