From ccc1c731529de16f6fa4064fd992a8f63d7cfc26 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Fri, 11 Jan 2008 00:01:58 -0600 Subject: Use dynamic string allocation in package structures This also affects all structures with static strings, such as depmiss, conflict, etc. This should help a lot with memory usage, and hopefully make things a bit more "idiot proof". Currently our pactest pass/fail rate is identical before and after this patch. This is not to say it is a perfect patch- I have yet to pull valgrind out. However, this should be quite safe to use in all situations from here on out, and we can start plugging the memleaks. Original-work-by: Aaron Griffin Signed-off-by: Dan McGee --- lib/libalpm/be_files.c | 194 ++++++++++++++++++++++++++----------------------- 1 file changed, 103 insertions(+), 91 deletions(-) (limited to 'lib/libalpm/be_files.c') diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index 724e3c8f..acd102ad 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -106,7 +106,7 @@ void _alpm_db_rewind(pmdb_t *db) rewinddir(db->handle); } -static int _alpm_db_splitname(const char *target, char *name, char *version) +static int splitname(const char *target, pmpkg_t *pkg) { /* the format of a db entry is as follows: * package-version-rel/ @@ -115,10 +115,10 @@ static int _alpm_db_splitname(const char *target, char *name, char *version) */ char *tmp, *p, *q; - if(target == NULL) { + if(target == NULL || pkg == NULL) { return(-1); } - tmp = strdup(target); + STRDUP(tmp, target, RET_ERR(PM_ERR_MEMORY, -1)); p = tmp + strlen(tmp); /* do the magic parsing- find the beginning of the version string @@ -130,14 +130,16 @@ static int _alpm_db_splitname(const char *target, char *name, char *version) } /* copy into fields and return */ - if(version) { - strncpy(version, p+1, PKG_VERSION_LEN); + if(pkg->version) { + FREE(pkg->version); } + STRDUP(pkg->version, p+1, RET_ERR(PM_ERR_MEMORY, -1)); /* insert a terminator at the end of the name (on hyphen)- then copy it */ *p = '\0'; - if(name) { - strncpy(name, tmp, PKG_NAME_LEN); + if(pkg->name) { + FREE(pkg->name); } + STRDUP(pkg->name, tmp, RET_ERR(PM_ERR_MEMORY, -1)); free(tmp); return(0); @@ -148,7 +150,6 @@ pmpkg_t *_alpm_db_scan(pmdb_t *db, const char *target) struct dirent *ent = NULL; struct stat sbuf; char path[PATH_MAX]; - char name[PKG_FULLNAME_LEN]; char *ptr = NULL; int found = 0; pmpkg_t *pkg = NULL; @@ -168,7 +169,9 @@ pmpkg_t *_alpm_db_scan(pmdb_t *db, const char *target) /* search for a specific package (by name only) */ rewinddir(db->handle); while(!found && (ent = readdir(db->handle)) != NULL) { - if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { + char *name; + + if(strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0) { continue; } /* stat the entry, make sure it's a directory */ @@ -176,7 +179,9 @@ pmpkg_t *_alpm_db_scan(pmdb_t *db, const char *target) if(stat(path, &sbuf) || !S_ISDIR(sbuf.st_mode)) { continue; } - strncpy(name, ent->d_name, PKG_FULLNAME_LEN); + + STRDUP(name, ent->d_name, return(NULL)); + /* truncate the string at the second-to-last hyphen, */ /* which will give us the package name */ if((ptr = rindex(name, '-'))) { @@ -185,10 +190,12 @@ pmpkg_t *_alpm_db_scan(pmdb_t *db, const char *target) if((ptr = rindex(name, '-'))) { *ptr = '\0'; } - if(!strcmp(name, target)) { + if(strcmp(name, target) == 0) { found = 1; } + FREE(name); } + if(!found) { return(NULL); } @@ -199,7 +206,7 @@ pmpkg_t *_alpm_db_scan(pmdb_t *db, const char *target) if(ent == NULL) { return(NULL); } - if(!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { + if(strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0) { isdir = 0; continue; } @@ -217,7 +224,7 @@ pmpkg_t *_alpm_db_scan(pmdb_t *db, const char *target) return(NULL); } /* split the db entry name */ - if(_alpm_db_splitname(ent->d_name, pkg->name, pkg->version) != 0) { + if(splitname(ent->d_name, pkg) != 0) { _alpm_log(PM_LOG_ERROR, _("invalid name for database entry '%s'\n"), ent->d_name); alpm_pkg_free(pkg); @@ -251,7 +258,7 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) RET_ERR(PM_ERR_DB_NULL, -1); } - if(info == NULL || info->name[0] == 0 || info->version[0] == 0) { + if(info == NULL || info->name == NULL || info->version == NULL) { _alpm_log(PM_LOG_DEBUG, "invalid package entry provided to _alpm_db_read, skipping\n"); return(-1); } @@ -296,122 +303,117 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) break; } _alpm_strtrim(line); - if(!strcmp(line, "%FILENAME%")) { - /* filename is _new_ - it provides the real name of the package, on the - * server, to allow for us to not tie the name of the actual file to the - * data of the package - */ - if(fgets(info->filename, sizeof(info->filename), fp) == NULL) { + if(strcmp(line, "%FILENAME%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(info->filename); - } else if(!strcmp(line, "%DESC%")) { - if(fgets(info->desc, sizeof(info->desc), fp) == NULL) { + STRDUP(info->filename, _alpm_strtrim(line), goto error); + } else if(strcmp(line, "%DESC%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(info->desc); - } else if(!strcmp(line, "%GROUPS%")) { + STRDUP(info->desc, _alpm_strtrim(line), goto error); + } else if(strcmp(line, "%GROUPS%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->groups = alpm_list_add(info->groups, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->groups = alpm_list_add(info->groups, linedup); } - } else if(!strcmp(line, "%URL%")) { - if(fgets(info->url, sizeof(info->url), fp) == NULL) { + } else if(strcmp(line, "%URL%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(info->url); - } else if(!strcmp(line, "%LICENSE%")) { + STRDUP(info->url, _alpm_strtrim(line), goto error); + } else if(strcmp(line, "%LICENSE%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->licenses = alpm_list_add(info->licenses, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->licenses = alpm_list_add(info->licenses, linedup); } - } else if(!strcmp(line, "%ARCH%")) { - if(fgets(info->arch, sizeof(info->arch), fp) == NULL) { + } else if(strcmp(line, "%ARCH%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(info->arch); - } else if(!strcmp(line, "%BUILDDATE%")) { - char tmp[32]; - if(fgets(tmp, sizeof(tmp), fp) == NULL) { + STRDUP(info->arch, _alpm_strtrim(line), goto error); + } else if(strcmp(line, "%BUILDDATE%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(tmp); + _alpm_strtrim(line); - char first = tolower(tmp[0]); + char first = tolower(line[0]); if(first > 'a' && first < 'z') { struct tm tmp_tm = {0}; //initialize to null incase of failure setlocale(LC_TIME, "C"); - strptime(tmp, "%a %b %e %H:%M:%S %Y", &tmp_tm); + strptime(line, "%a %b %e %H:%M:%S %Y", &tmp_tm); info->builddate = mktime(&tmp_tm); setlocale(LC_TIME, ""); } else { - info->builddate = atol(tmp); + info->builddate = atol(line); } - } else if(!strcmp(line, "%INSTALLDATE%")) { - char tmp[32]; - if(fgets(tmp, sizeof(tmp), fp) == NULL) { + } else if(strcmp(line, "%INSTALLDATE%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(tmp); + _alpm_strtrim(line); - char first = tolower(tmp[0]); + char first = tolower(line[0]); if(first > 'a' && first < 'z') { struct tm tmp_tm = {0}; //initialize to null incase of failure setlocale(LC_TIME, "C"); - strptime(tmp, "%a %b %e %H:%M:%S %Y", &tmp_tm); + strptime(line, "%a %b %e %H:%M:%S %Y", &tmp_tm); info->installdate = mktime(&tmp_tm); setlocale(LC_TIME, ""); } else { - info->installdate = atol(tmp); + info->installdate = atol(line); } - } else if(!strcmp(line, "%PACKAGER%")) { - if(fgets(info->packager, sizeof(info->packager), fp) == NULL) { + } else if(strcmp(line, "%PACKAGER%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(info->packager); - } else if(!strcmp(line, "%REASON%")) { - char tmp[32]; - if(fgets(tmp, sizeof(tmp), fp) == NULL) { + STRDUP(info->packager, _alpm_strtrim(line), goto error); + } else if(strcmp(line, "%REASON%") == 0) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(tmp); - info->reason = atol(tmp); - } else if(!strcmp(line, "%SIZE%") || !strcmp(line, "%CSIZE%")) { + info->reason = atol(_alpm_strtrim(line)); + } else if(strcmp(line, "%SIZE%") == 0 || strcmp(line, "%CSIZE%") == 0) { /* NOTE: the CSIZE and SIZE fields both share the "size" field * in the pkginfo_t struct. This can be done b/c CSIZE * is currently only used in sync databases, and SIZE is * only used in local databases. */ - char tmp[32]; - if(fgets(tmp, sizeof(tmp), fp) == NULL) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(tmp); - info->size = atol(tmp); + info->size = atol(_alpm_strtrim(line)); /* also store this value to isize if isize is unset */ if(info->isize == 0) { - info->isize = atol(tmp); + info->isize = info->size; } - } else if(!strcmp(line, "%ISIZE%")) { + } else if(strcmp(line, "%ISIZE%") == 0) { /* ISIZE (installed size) tag only appears in sync repositories, * not the local one. */ - char tmp[32]; - if(fgets(tmp, sizeof(tmp), fp) == NULL) { + if(fgets(line, 512, fp) == NULL) { goto error; } - _alpm_strtrim(tmp); - info->isize = atol(tmp); - } else if(!strcmp(line, "%MD5SUM%")) { + info->isize = atol(_alpm_strtrim(line)); + } else if(strcmp(line, "%MD5SUM%") == 0) { /* MD5SUM tag only appears in sync repositories, * not the local one. */ - if(fgets(info->md5sum, sizeof(info->md5sum), fp) == NULL) { + if(fgets(line, 512, fp) == NULL) { goto error; } - } else if(!strcmp(line, "%REPLACES%")) { + STRDUP(info->md5sum, _alpm_strtrim(line), goto error); + } else if(strcmp(line, "%REPLACES%") == 0) { /* the REPLACES tag is special -- it only appears in sync repositories, * not the local one. */ while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->replaces = alpm_list_add(info->replaces, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->replaces = alpm_list_add(info->replaces, linedup); } - } else if(!strcmp(line, "%FORCE%")) { + } else if(strcmp(line, "%FORCE%") == 0) { /* FORCE tag only appears in sync repositories, * not the local one. */ info->force = 1; @@ -430,13 +432,17 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } while(fgets(line, 256, fp)) { _alpm_strtrim(line); - if(!strcmp(line, "%FILES%")) { + if(strcmp(line, "%FILES%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->files = alpm_list_add(info->files, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->files = alpm_list_add(info->files, linedup); } - } else if(!strcmp(line, "%BACKUP%")) { + } else if(strcmp(line, "%BACKUP%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->backup = alpm_list_add(info->backup, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->backup = alpm_list_add(info->backup, linedup); } } } @@ -454,33 +460,39 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) while(!feof(fp)) { fgets(line, 255, fp); _alpm_strtrim(line); - if(!strcmp(line, "%DEPENDS%")) { + if(strcmp(line, "%DEPENDS%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - pmdepend_t *dep = alpm_splitdep(line); + pmdepend_t *dep = alpm_splitdep(_alpm_strtrim(line)); info->depends = alpm_list_add(info->depends, dep); } - } else if(!strcmp(line, "%OPTDEPENDS%")) { + } else if(strcmp(line, "%OPTDEPENDS%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->optdepends = alpm_list_add(info->optdepends, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->optdepends = alpm_list_add(info->optdepends, linedup); } - } else if(!strcmp(line, "%CONFLICTS%")) { + } else if(strcmp(line, "%CONFLICTS%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->conflicts = alpm_list_add(info->conflicts, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->conflicts = alpm_list_add(info->conflicts, linedup); } - } else if(!strcmp(line, "%PROVIDES%")) { + } else if(strcmp(line, "%PROVIDES%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - info->provides = alpm_list_add(info->provides, strdup(line)); + char *linedup; + STRDUP(linedup, _alpm_strtrim(line), goto error); + info->provides = alpm_list_add(info->provides, linedup); } } /* TODO: we were going to move these things here, but it should wait. * A better change would be to figure out how to restructure the DB. */ - /* else if(!strcmp(line, "%REPLACES%")) { + /* else if(strcmp(line, "%REPLACES%") == 0) { * the REPLACES tag is special -- it only appears in sync repositories, * not the local one. * while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { info->replaces = alpm_list_add(info->replaces, strdup(line)); } - } else if(!strcmp(line, "%FORCE%")) { + } else if(strcmp(line, "%FORCE%") == 0) { * FORCE tag only appears in sync repositories, * not the local one. * info->force = 1; @@ -497,7 +509,7 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) while(!feof(fp)) { fgets(line, 255, fp); _alpm_strtrim(line); - if(!strcmp(line, "%DELTAS%")) { + if(strcmp(line, "%DELTAS%") == 0) { while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { info->deltas = alpm_list_add(info->deltas, _alpm_delta_parse(line)); } @@ -565,7 +577,7 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } fprintf(fp, "%%NAME%%\n%s\n\n" "%%VERSION%%\n%s\n\n", info->name, info->version); - if(info->desc[0]) { + if(info->desc) { fprintf(fp, "%%DESC%%\n" "%s\n\n", info->desc); } @@ -577,7 +589,7 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) fprintf(fp, "\n"); } if(local) { - if(info->url[0]) { + if(info->url) { fprintf(fp, "%%URL%%\n" "%s\n\n", info->url); } @@ -588,7 +600,7 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } fprintf(fp, "\n"); } - if(info->arch[0]) { + if(info->arch) { fprintf(fp, "%%ARCH%%\n" "%s\n\n", info->arch); } @@ -600,7 +612,7 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) fprintf(fp, "%%INSTALLDATE%%\n" "%ju\n\n", (uintmax_t)info->installdate); } - if(info->packager[0]) { + if(info->packager) { fprintf(fp, "%%PACKAGER%%\n" "%s\n\n", info->packager); } -- cgit v1.2.3-54-g00ecf