[pacman-dev] [PATCH] Separate file parsing from backend
Split the huge list of else ifs which was used for parsing 'desc', 'depends', 'files' and 'deltas' files, into separate functions and move them to a new file parse.c . This makes it possible for other backends to share the same parse code. Also change the parsing from the giant else if list to a single loop calling different functions as specified in an array of parser structs (see parse_lines() for details). Move _alpm_delta_parse to parse.c and make it static. Signed-off-by: Henning Garus <henning.garus@gmail.com> --- I actually managed to make it longer, instead of shorter (well if you take out comments and license headers this version is roughly as long as the old one), however I think it is more readable. Feel free to call it overkill though. I don't really like the ugets stuff I used to get the next line, but I wanted this to work on something other than FILE*, even though this is not used (yet), and this was the best thing I could come up with. I toyed around with using fopencookie, but that sucks when it comes to portability (there is funopen on bsd, beyond that you are simply screwed). I removed the memset for the line array, it should be fairly safe and I ran valgrind with several pactests and it did not shout at me (at least after I supressed the numerous leaks reported for bash). Btw, is there a reason why pacman uses two different file formats (line based in db files vs. keyword = value in .PKGINFO ? lib/libalpm/Makefile.am | 1 + lib/libalpm/be_files.c | 222 +++++----------------------------- lib/libalpm/delta.c | 56 --------- lib/libalpm/delta.h | 1 - lib/libalpm/parse.c | 305 +++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/parse.h | 40 ++++++ 6 files changed, 377 insertions(+), 248 deletions(-) create mode 100644 lib/libalpm/parse.c create mode 100644 lib/libalpm/parse.h diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am index 871855e..5ed8866 100644 --- a/lib/libalpm/Makefile.am +++ b/lib/libalpm/Makefile.am @@ -27,6 +27,7 @@ libalpm_la_SOURCES = \ backup.h backup.c \ be_files.c \ be_package.c \ + parse.h parse.c \ cache.h cache.c \ conflict.h conflict.c \ db.h db.c \ diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index 21533ef..1066b60 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -31,7 +31,6 @@ #include <ctype.h> #include <time.h> #include <limits.h> /* PATH_MAX */ -#include <locale.h> /* setlocale */ /* libalpm */ #include "db.h" @@ -42,9 +41,9 @@ #include "alpm.h" #include "handle.h" #include "package.h" -#include "delta.h" #include "deps.h" #include "dload.h" +#include "parse.h" /* @@ -386,7 +385,8 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) { FILE *fp = NULL; char path[PATH_MAX]; - char line[513]; + char *pkgname = NULL; + char *pkgver = NULL; char *pkgpath = NULL; ALPM_LOG_FUNC; @@ -417,9 +417,6 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) _alpm_log(PM_LOG_FUNCTION, "loading package data for %s : level=0x%x\n", info->name, inforeq); - /* clear out 'line', to be certain - and to make valgrind happy */ - memset(line, 0, 513); - pkgpath = get_pkgpath(db, info); if(access(pkgpath, F_OK)) { @@ -429,6 +426,7 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) goto error; } + /* DESC */ if(inforeq & INFRQ_DESC) { snprintf(path, PATH_MAX, "%sdesc", pkgpath); @@ -436,139 +434,26 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) _alpm_log(PM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); goto error; } - while(!feof(fp)) { - if(fgets(line, 256, fp) == NULL) { - break; - } - _alpm_strtrim(line); - if(strcmp(line, "%NAME%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - if(strcmp(_alpm_strtrim(line), info->name) != 0) { - _alpm_log(PM_LOG_ERROR, _("%s database is inconsistent: name " - "mismatch on package %s\n"), db->treename, info->name); - } - } else if(strcmp(line, "%VERSION%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - if(strcmp(_alpm_strtrim(line), info->version) != 0) { - _alpm_log(PM_LOG_ERROR, _("%s database is inconsistent: version " - "mismatch on package %s\n"), db->treename, info->name); - } - } else if(strcmp(line, "%FILENAME%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - STRDUP(info->filename, _alpm_strtrim(line), goto error); - } else if(strcmp(line, "%DESC%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - STRDUP(info->desc, _alpm_strtrim(line), goto error); - } else if(strcmp(line, "%GROUPS%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->groups = alpm_list_add(info->groups, linedup); - } - } else if(strcmp(line, "%URL%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - STRDUP(info->url, _alpm_strtrim(line), goto error); - } else if(strcmp(line, "%LICENSE%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->licenses = alpm_list_add(info->licenses, linedup); - } - } else if(strcmp(line, "%ARCH%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - STRDUP(info->arch, _alpm_strtrim(line), goto error); - } else if(strcmp(line, "%BUILDDATE%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - _alpm_strtrim(line); - - char first = tolower(line[0]); - if(first > 'a' && first < 'z') { - struct tm tmp_tm = {0}; /* initialize to null in case of failure */ - setlocale(LC_TIME, "C"); - strptime(line, "%a %b %e %H:%M:%S %Y", &tmp_tm); - info->builddate = mktime(&tmp_tm); - setlocale(LC_TIME, ""); - } else { - info->builddate = atol(line); - } - } else if(strcmp(line, "%INSTALLDATE%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - _alpm_strtrim(line); - - char first = tolower(line[0]); - if(first > 'a' && first < 'z') { - struct tm tmp_tm = {0}; /* initialize to null in case of failure */ - setlocale(LC_TIME, "C"); - strptime(line, "%a %b %e %H:%M:%S %Y", &tmp_tm); - info->installdate = mktime(&tmp_tm); - setlocale(LC_TIME, ""); - } else { - info->installdate = atol(line); - } - } else if(strcmp(line, "%PACKAGER%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - STRDUP(info->packager, _alpm_strtrim(line), goto error); - } else if(strcmp(line, "%REASON%") == 0) { - if(fgets(line, 512, fp) == NULL) { - goto error; - } - info->reason = (pmpkgreason_t)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. - */ - if(fgets(line, 512, fp) == NULL) { - goto error; - } - info->size = atol(_alpm_strtrim(line)); - /* also store this value to isize if isize is unset */ - if(info->isize == 0) { - info->isize = info->size; - } - } else if(strcmp(line, "%ISIZE%") == 0) { - /* ISIZE (installed size) tag only appears in sync repositories, - * not the local one. */ - if(fgets(line, 512, fp) == NULL) { - goto error; - } - 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(line, 512, fp) == NULL) { - goto error; - } - STRDUP(info->md5sum, _alpm_strtrim(line), goto error); - } else if(strcmp(line, "%REPLACES%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->replaces = alpm_list_add(info->replaces, linedup); - } - } else if(strcmp(line, "%FORCE%") == 0) { - info->force = 1; - } + STRDUP(pkgname, info->name, goto error); + STRDUP(pkgver, info->version, goto error); + if(_alpm_parse_desc(&_alpm_parse_from_file, fp, info) != 0) { + goto error; } + if(strcmp(pkgname,info->name) != 0) { + _alpm_log(PM_LOG_ERROR, _("%s database is inconsistent: name mismatch " + "on package %s\n"), db->treename, pkgname); + } + if(strcmp(pkgver,info->version) != 0) { + _alpm_log(PM_LOG_ERROR, _("%s database is inconsistent: version mismatch " + "on package %s\n"), db->treename, pkgver); + } + FREE(pkgname); + FREE(pkgver); + /* store size in isize if isize is unset */ + if(info->isize == 0) { + info->isize = info->size; + } + fclose(fp); fp = NULL; } @@ -580,21 +465,8 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) _alpm_log(PM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); goto error; } - while(fgets(line, 256, fp)) { - _alpm_strtrim(line); - if(strcmp(line, "%FILES%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->files = alpm_list_add(info->files, linedup); - } - } else if(strcmp(line, "%BACKUP%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->backup = alpm_list_add(info->backup, linedup); - } - } + if(_alpm_parse_files(&_alpm_parse_from_file, fp, info) != 0) { + goto error; } fclose(fp); fp = NULL; @@ -607,33 +479,8 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) _alpm_log(PM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); goto error; } - while(!feof(fp)) { - fgets(line, 255, fp); - _alpm_strtrim(line); - if(strcmp(line, "%DEPENDS%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - pmdepend_t *dep = _alpm_splitdep(_alpm_strtrim(line)); - info->depends = alpm_list_add(info->depends, dep); - } - } else if(strcmp(line, "%OPTDEPENDS%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->optdepends = alpm_list_add(info->optdepends, linedup); - } - } else if(strcmp(line, "%CONFLICTS%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->conflicts = alpm_list_add(info->conflicts, linedup); - } - } else if(strcmp(line, "%PROVIDES%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - char *linedup; - STRDUP(linedup, _alpm_strtrim(line), goto error); - info->provides = alpm_list_add(info->provides, linedup); - } - } + if(_alpm_parse_depends(&_alpm_parse_from_file, fp, info) != 0) { + goto error; } fclose(fp); fp = NULL; @@ -643,17 +490,8 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) if(inforeq & INFRQ_DELTAS) { snprintf(path, PATH_MAX, "%sdeltas", pkgpath); if((fp = fopen(path, "r"))) { - while(!feof(fp)) { - fgets(line, 255, fp); - _alpm_strtrim(line); - if(strcmp(line, "%DELTAS%") == 0) { - while(fgets(line, 512, fp) && strlen(_alpm_strtrim(line))) { - pmdelta_t *delta = _alpm_delta_parse(line); - if(delta) { - info->deltas = alpm_list_add(info->deltas, delta); - } - } - } + if(_alpm_parse_deltas(&_alpm_parse_from_file, fp, info) != 0) { + goto error; } fclose(fp); fp = NULL; @@ -675,6 +513,8 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) return(0); error: + free(pkgname); + free(pkgver); free(pkgpath); if(fp) { fclose(fp); diff --git a/lib/libalpm/delta.c b/lib/libalpm/delta.c index 523968e..5202e4b 100644 --- a/lib/libalpm/delta.c +++ b/lib/libalpm/delta.c @@ -25,7 +25,6 @@ #include <stdint.h> /* intmax_t */ #include <limits.h> #include <sys/types.h> -#include <regex.h> /* libalpm */ #include "delta.h" @@ -247,61 +246,6 @@ off_t _alpm_shortest_delta_path(alpm_list_t *deltas, return(bestsize); } -/** Parses the string representation of a pmdelta_t object. - * This function assumes that the string is in the correct format. - * This format is as follows: - * $deltafile $deltamd5 $deltasize $oldfile $newfile - * @param line the string to parse - * @return A pointer to the new pmdelta_t object - */ -/* TODO this does not really belong here, but in a parsing lib */ -pmdelta_t *_alpm_delta_parse(char *line) -{ - pmdelta_t *delta; - char *tmp = line, *tmp2; - regex_t reg; - - regcomp(®, - "^[^[:space:]]* [[:xdigit:]]{32} [[:digit:]]*" - " [^[:space:]]* [^[:space:]]*$", - REG_EXTENDED | REG_NOSUB | REG_NEWLINE); - if(regexec(®, line, 0, 0, 0) != 0) { - /* delta line is invalid, return NULL */ - regfree(®); - return(NULL); - } - regfree(®); - - CALLOC(delta, 1, sizeof(pmdelta_t), RET_ERR(PM_ERR_MEMORY, NULL)); - - tmp2 = tmp; - tmp = strchr(tmp, ' '); - *(tmp++) = '\0'; - STRDUP(delta->delta, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); - - tmp2 = tmp; - tmp = strchr(tmp, ' '); - *(tmp++) = '\0'; - STRDUP(delta->delta_md5, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); - - tmp2 = tmp; - tmp = strchr(tmp, ' '); - *(tmp++) = '\0'; - delta->delta_size = atol(tmp2); - - tmp2 = tmp; - tmp = strchr(tmp, ' '); - *(tmp++) = '\0'; - STRDUP(delta->from, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); - - tmp2 = tmp; - STRDUP(delta->to, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); - - _alpm_log(PM_LOG_DEBUG, "delta : %s %s '%lld'\n", delta->from, delta->to, (long long)delta->delta_size); - - return(delta); -} - void _alpm_delta_free(pmdelta_t *delta) { FREE(delta->from); diff --git a/lib/libalpm/delta.h b/lib/libalpm/delta.h index 4f426cb..3c4e4cf 100644 --- a/lib/libalpm/delta.h +++ b/lib/libalpm/delta.h @@ -39,7 +39,6 @@ struct __pmdelta_t { off_t download_size; }; -pmdelta_t *_alpm_delta_parse(char *line); void _alpm_delta_free(pmdelta_t *delta); off_t _alpm_shortest_delta_path(alpm_list_t *deltas, const char *to, alpm_list_t **path); diff --git a/lib/libalpm/parse.c b/lib/libalpm/parse.c new file mode 100644 index 0000000..31038fc --- /dev/null +++ b/lib/libalpm/parse.c @@ -0,0 +1,305 @@ +/* + * parse.c + * + * Copyright (c) 2006-2009 Pacman Development Team <pacman-dev@archlinux.org> + * Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include "config.h" + +#include <stdio.h> +#include <string.h> +#include <stdlib.h> +#include <ctype.h> +#include <locale.h> /* setlocale */ +#include <regex.h> +#include <time.h> + +/* libalpm */ +#include "parse.h" +#include "util.h" +#include "alpm_list.h" +#include "log.h" +#include "package.h" +#include "delta.h" +#include "deps.h" + +enum { + PARSE_FINISHED = 0, + PARSE_UNFINISHED = 1, + PARSE_ERROR = 2 +}; + +typedef struct parser { + char *keyword; + int (*func)(char*, void*); + void *saveto; +} parser_t; + +/** Parses the string representation of a pmdelta_t object. + * This function assumes that the string is in the correct format. + * This format is as follows: + * $deltafile $deltamd5 $deltasize $oldfile $newfile + * @param line the string to parse + * @return A pointer to the new pmdelta_t object + */ +static pmdelta_t *delta_parse(char *line) +{ + pmdelta_t *delta; + char *tmp = line, *tmp2; + regex_t reg; + + regcomp(®, + "^[^[:space:]]* [[:xdigit:]]{32} [[:digit:]]*" + " [^[:space:]]* [^[:space:]]*$", + REG_EXTENDED | REG_NOSUB | REG_NEWLINE); + if(regexec(®, line, 0, 0, 0) != 0) { + /* delta line is invalid, return NULL */ + regfree(®); + return(NULL); + } + regfree(®); + + CALLOC(delta, 1, sizeof(pmdelta_t), RET_ERR(PM_ERR_MEMORY, NULL)); + + tmp2 = tmp; + tmp = strchr(tmp, ' '); + *(tmp++) = '\0'; + STRDUP(delta->delta, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + + tmp2 = tmp; + tmp = strchr(tmp, ' '); + *(tmp++) = '\0'; + STRDUP(delta->delta_md5, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + + tmp2 = tmp; + tmp = strchr(tmp, ' '); + *(tmp++) = '\0'; + delta->delta_size = atol(tmp2); + + tmp2 = tmp; + tmp = strchr(tmp, ' '); + *(tmp++) = '\0'; + STRDUP(delta->from, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + + tmp2 = tmp; + STRDUP(delta->to, tmp2, RET_ERR(PM_ERR_MEMORY, NULL)); + + _alpm_log(PM_LOG_DEBUG, "delta : %s %s '%lld'\n", delta->from, delta->to, (long long)delta->delta_size); + + return(delta); +} + +/** Parse line based data, as found in desc, depends, files and deltas files + * The data is expected to be in the following format: + * keyword + * [entry] + * [entry] + * ... + * <newline> + * [keyword] + * ... + * + * The handling of the data for each keyword is described by the matching + * struct of type parser_t. This struct holds a function pointer and a void + * pointer for each keyword. The function is called for the lines following a + * keyword until it returns PARSE_FINISHED. It is expected to handle the actual + * parsing and store the data at the passed void pointer. + * @param parsers describes how to parse data for each keyword + * @param nparsers size of parsers + * @param ugets pointer to an fgets like function, which gets called to get + * the next line + * @param from describes from where to read the next line, this is passed to + * ugets + */ +static int parse_lines(parser_t parsers[], int nparsers, ugets_t ugets, + void *from) +{ + int i, ret; + char line[513]; + parser_t *p = NULL; + + while((*ugets)(line, sizeof(line) - 1, from) != NULL) { + _alpm_strtrim(line); + if(p == NULL && strlen(line) == 0) { + continue; + } + if(p == NULL) { + for(i = 0; i < nparsers; i++) { + if(strcmp(line,parsers[i].keyword) == 0) { + p = &parsers[i]; + } + } + } + else { + ret = (*(p->func))(line, p->saveto); + if(ret == PARSE_FINISHED) { + p = NULL; + } + else if(ret == PARSE_ERROR) { + return(ret); + } + } + } + return(0); +} + +static int copy_line(char *line, void *target) { + STRDUP(*(char**)target, line, return(PARSE_ERROR)); + return(PARSE_FINISHED); +} + +static int parse_off_t(char *line, void *target) { + *(off_t*)target = (off_t)atol(line); + return(PARSE_FINISHED); +} + +static int parse_reason(char *line, void *target) { + *(pmpkgreason_t*)target = (pmpkgreason_t)atol(line); + return(PARSE_FINISHED); +} + +static int parse_date(char *line, void *target) { + char first = tolower(line[0]); + if(first > 'a' && first < 'z') { + struct tm tmp_tm = {0}; /* initialize to null in case of failure */ + setlocale(LC_TIME, "C"); + strptime(line, "%a %b %e %H:%M:%S %Y", &tmp_tm); + *(time_t*)target = mktime(&tmp_tm); + setlocale(LC_TIME, ""); + } else { + *(time_t*)target = atol(line); + } + return(PARSE_FINISHED); +} + +static int set_flag(char *line, void *target) { + *(unsigned short*)target = 1; + return(PARSE_FINISHED); +} + +static int copy_to_list(char *line, void *list) { + if(strlen(line) == 0) { + return(PARSE_FINISHED); + } + char *linedup; + STRDUP(linedup, line, RET_ERR(PM_ERR_MEMORY, PARSE_ERROR)); + *(alpm_list_t**)list = alpm_list_add(*(alpm_list_t**)list, linedup); + return(PARSE_UNFINISHED); +} + +static int deltas_to_list(char *line, void *list) { + if(strlen(line) == 0) { + return(PARSE_FINISHED); + } + pmdelta_t *delta = delta_parse(line); + if(delta != NULL) { + *(alpm_list_t**)list = alpm_list_add(*(alpm_list_t**)list, delta); + } + return(PARSE_UNFINISHED); +} + +static int deps_to_list(char *line, void *list) { + if(strlen(line) == 0) { + return(PARSE_FINISHED); + } + pmdepend_t *dep = _alpm_splitdep(line); + *(alpm_list_t**)list = alpm_list_add(*(alpm_list_t**)list, dep); + return(PARSE_UNFINISHED); +} + +char *_alpm_parse_from_file(char *buf,int size, void* fp) +{ + return(fgets(buf, size, (FILE*)fp)); +} + +char *_alpm_parse_from_archive(char *buf,int size, void* a) +{ + return(_alpm_archive_fgets(buf, size, (struct archive*)a)); +} + + +int _alpm_parse_desc(ugets_t ugets, void *from, pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + parser_t parsers[] = { + {"%NAME%", ©_line, &pkg->name}, + {"%VERSION%", ©_line, &pkg->version}, + {"%FILENAME%", ©_line, &pkg->filename}, + {"%DESC%", ©_line, &pkg->desc}, + {"%GROUPS%", ©_to_list, &pkg->groups}, + {"%URL%", ©_line, &pkg->url}, + {"%LICENSE%", ©_to_list, &pkg->licenses}, + {"%ARCH%", ©_line, &pkg->arch}, + {"%BUILDDATE%", &parse_date, &pkg->builddate}, + {"%INSTALLDATE%", &parse_date, &pkg->installdate}, + {"%PACKAGER%", ©_line, &pkg->packager}, + {"%REASON%", &parse_reason, &pkg->reason}, + {"%SIZE%", &parse_off_t, &pkg->size}, + {"%CSIZE%", &parse_off_t, &pkg->size}, + {"%ISIZE%", &parse_off_t, &pkg->isize}, + {"%MD5SUM%", ©_line, &pkg->md5sum}, + {"%REPLACES%", ©_to_list, &pkg->replaces}, + {"%FORCE%", &set_flag, &pkg->force} + }; + int nparsers = sizeof(parsers) / sizeof(parser_t); + + return(parse_lines(parsers, nparsers, ugets, from)); +} + +int _alpm_parse_files(ugets_t ugets, void* from, + pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + parser_t parsers[] = { + {"%FILES%", ©_to_list, &pkg->files}, + {"%BACKUP%", ©_to_list, &pkg->backup} + }; + int nparsers = sizeof(parsers) / sizeof(parser_t); + + return(parse_lines(parsers, nparsers, ugets, from)); +} + +int _alpm_parse_depends(char* (*ugets)(char*, int, void*), void *from, + pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + parser_t parsers[] = { + {"%DEPENDS%", &deps_to_list, &pkg->depends}, + {"%OPTDEPENDS%", ©_to_list, &pkg->optdepends}, + {"%CONFLICTS%", ©_to_list, &pkg->conflicts}, + {"%PROVIDES%", ©_to_list, &pkg->provides} + }; + int nparsers = sizeof(parsers) / sizeof(parser_t); + + return(parse_lines(parsers, nparsers, ugets, from)); +} + +int _alpm_parse_deltas(ugets_t ugets, void *from, pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + parser_t parsers[] = { + {"%DELTAS%", &deltas_to_list, &pkg->deltas} + }; + int nparsers = sizeof(parsers) / sizeof(parser_t); + + return(parse_lines(parsers, nparsers, ugets, from)); +} + +/* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/parse.h b/lib/libalpm/parse.h new file mode 100644 index 0000000..d3ef77e --- /dev/null +++ b/lib/libalpm/parse.h @@ -0,0 +1,40 @@ +/* + * parse.h + * + * Copyright (c) 2006-2009 Pacman Development Team <pacman-dev@archlinux.org> + * Copyright (c) 2002-2006 by Judd Vinet <jvinet@zeroflux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#ifndef _ALPM_PARSE_H +#define _ALPM_PARSE_H + +#include "package.h" + +/* An fgets like function, argument for the parse functions */ +typedef char *(*ugets_t)(char*, int, void*); + +/* arguments for the parse functions (ugets_t) */ +char *_alpm_parse_from_file(char*, int, void*); +char *_alpm_parse_from_archive(char*, int, void*); + +/* parse functions */ +int _alpm_parse_desc(ugets_t, void*, pmpkg_t*); +int _alpm_parse_files(ugets_t, void*, pmpkg_t*); +int _alpm_parse_depends(ugets_t, void*, pmpkg_t*); +int _alpm_parse_deltas(ugets_t, void*, pmpkg_t*); + +#endif /* _ALPM_PARSE_H */ + +/* vim: set ts=2 sw=2 noet: */ -- 1.6.4.4
First replying to these two points before looking at the rest.. On Wed, Sep 23, 2009 at 2:46 PM, Henning Garus <henning.garus@googlemail.com> wrote:
I removed the memset for the line array, it should be fairly safe and I ran valgrind with several pactests and it did not shout at me (at least after I supressed the numerous leaks reported for bash).
AFAIK, you should use the following binary when using valgrind : src/pacman/.libs/lt-pacman But I never tried to understand what all that libtool mess is about.
Btw, is there a reason why pacman uses two different file formats (line based in db files vs. keyword = value in .PKGINFO ?
This one has bugged me since I joined, like 2 years ago :) See http://mailman.archlinux.org/pipermail/pacman-dev/2007-June/003179.html Unfortunately, all mailing list links have been broken :( Here is the correct one for the previous post : http://mailman.archlinux.org/pipermail/pacman-dev/2006-March/000280.html The problem is that changing the package and database is not an easy task, we have to consider migration and conversion, backward compatibility, etc. So we should only change the format if it provides significant benefits.
On Wed, Sep 23, 2009 at 03:25:28PM +0200, Xavier wrote:
First replying to these two points before looking at the rest..
On Wed, Sep 23, 2009 at 2:46 PM, Henning Garus <henning.garus@googlemail.com> wrote:
I removed the memset for the line array, it should be fairly safe and I ran valgrind with several pactests and it did not shout at me (at least after I supressed the numerous leaks reported for bash).
AFAIK, you should use the following binary when using valgrind : src/pacman/.libs/lt-pacman But I never tried to understand what all that libtool mess is about.
Thanks, I am actually finding something now, though nothing of it seems to be related to line not being nulled. But I found another leak I introduced. I will sent a new patch and go do some reading on libtool.
Btw, is there a reason why pacman uses two different file formats (line based in db files vs. keyword = value in .PKGINFO ?
This one has bugged me since I joined, like 2 years ago :) See http://mailman.archlinux.org/pipermail/pacman-dev/2007-June/003179.html Unfortunately, all mailing list links have been broken :( Here is the correct one for the previous post : http://mailman.archlinux.org/pipermail/pacman-dev/2006-March/000280.html
So it is historic and there is no real reason apart from "we didn't change it with 3.0". I thought that much.
The problem is that changing the package and database is not an easy task, we have to consider migration and conversion, backward compatibility, etc. So we should only change the format if it provides significant benefits.
Sure, I do not want to go ahead and change anything, I was just being curious.
On Wed, Sep 23, 2009 at 2:46 PM, Henning Garus <henning.garus@googlemail.com> wrote:
Split the huge list of else ifs which was used for parsing 'desc', 'depends', 'files' and 'deltas' files, into separate functions and move them to a new file parse.c . This makes it possible for other backends to share the same parse code. Also change the parsing from the giant else if list to a single loop calling different functions as specified in an array of parser structs (see parse_lines() for details).
Move _alpm_delta_parse to parse.c and make it static.
Signed-off-by: Henning Garus <henning.garus@gmail.com> ---
I actually managed to make it longer, instead of shorter (well if you take out comments and license headers this version is roughly as long as the old one), however I think it is more readable. Feel free to call it overkill though.
I don't really like the ugets stuff I used to get the next line, but I wanted this to work on something other than FILE*, even though this is not used (yet), and this was the best thing I could come up with. I toyed around with using fopencookie, but that sucks when it comes to portability (there is funopen on bsd, beyond that you are simply screwed).
Well, after a quick look, I would say it looks cool. This work was still motivated by tar database backend, right ? (http://bugs.archlinux.org/task/8586) How far do you think you are from it ? :) Anyway, I would like to hear what Dan thinks, with all the backend work he did last year which was never finished : http://code.toofishes.net/cgit/dan/pacman.git/log/?h=backend Anyway, your work is on a lower level, and dan's work was on a higher level, so the two don't seem incompatible at first look.
On Wed, Sep 23, 2009 at 05:03:22PM +0200, Xavier wrote:
On Wed, Sep 23, 2009 at 2:46 PM, Henning Garus <henning.garus@googlemail.com> wrote:
Split the huge list of else ifs which was used for parsing 'desc', 'depends', 'files' and 'deltas' files, into separate functions and move them to a new file parse.c . This makes it possible for other backends to share the same parse code. Also change the parsing from the giant else if list to a single loop calling different functions as specified in an array of parser structs (see parse_lines() for details).
Move _alpm_delta_parse to parse.c and make it static.
Signed-off-by: Henning Garus <henning.garus@gmail.com> ---
I actually managed to make it longer, instead of shorter (well if you take out comments and license headers this version is roughly as long as the old one), however I think it is more readable. Feel free to call it overkill though.
I don't really like the ugets stuff I used to get the next line, but I wanted this to work on something other than FILE*, even though this is not used (yet), and this was the best thing I could come up with. I toyed around with using fopencookie, but that sucks when it comes to portability (there is funopen on bsd, beyond that you are simply screwed).
Well, after a quick look, I would say it looks cool. This work was still motivated by tar database backend, right ? (http://bugs.archlinux.org/task/8586) How far do you think you are from it ? :)
Not much further since we last talked on IRC, I have a working version, but I am still debating if basing it on Dan's backend branch might be better, some input from Dan could be helpful in that regard.
Anyway, I would like to hear what Dan thinks, with all the backend work he did last year which was never finished : http://code.toofishes.net/cgit/dan/pacman.git/log/?h=backend
Dito, especially about the introduction of a dboperations struct or something similar.
Anyway, your work is on a lower level, and dan's work was on a higher level, so the two don't seem incompatible at first look.
It should be, Dan did not touch the parsing stuff on that branch at all iirc. However the branch is outdated in some places by now.
participants (2)
-
Henning Garus
-
Xavier