This is an automated email from the git hooks/post-receive script. guillem pushed a commit to branch master in repository dpkg.
View the commit online: https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=80b9ae537f0d1d1c1f4dbb7f046c434618ee6363 commit 80b9ae537f0d1d1c1f4dbb7f046c434618ee6363 Author: Guillem Jover <[email protected]> AuthorDate: Sat Feb 23 04:55:59 2019 +0100 libdpkg: Optimize error handling Move the error reporting outside the involved functions so that we do not need to call gettext if there is no error, which has a significant performance cost. --- debian/changelog | 3 +++ lib/dpkg/fields.c | 63 +++++++++++++++++++++++++++++++--------------------- lib/dpkg/parse.c | 1 + lib/dpkg/parsedump.h | 14 +++++++++--- lib/dpkg/parsehelp.c | 47 +++++++++++++++++++++++++-------------- 5 files changed, 83 insertions(+), 45 deletions(-) diff --git a/debian/changelog b/debian/changelog index a0f88a3df..670266e9a 100644 --- a/debian/changelog +++ b/debian/changelog @@ -26,6 +26,9 @@ dpkg (1.19.5) UNRELEASED; urgency=medium * dpkg-genbuildinfo: Add support for a new Build-Tainted-By field in .buildinfo files. Suggested by Alexander E. Patrakov <[email protected]>. * libdpkg: Clarify field names in error and warning messages. + * libdpkg: Optimize error handling. Move the error reporting outside the + involved functions so that we do not need to call gettext if there is no + error, which has a significant performance cost. * Perl modules: - Dpkg::Vendor::Debian: Add support for merged-usr-via-symlinks tainted tag. Suggested by Alexander E. Patrakov <[email protected]>. diff --git a/lib/dpkg/fields.c b/lib/dpkg/fields.c index 63f9441bd..a2e5ef81c 100644 --- a/lib/dpkg/fields.c +++ b/lib/dpkg/fields.c @@ -62,20 +62,22 @@ enum parse_nv_flags { */ static int parse_nv(struct parsedb_state *ps, enum parse_nv_flags flags, - const char **strp, const struct namevalue *nv_head, const char *what) + const char **strp, const struct namevalue *nv_head) { const char *str_start = *strp, *str_end; const struct namevalue *nv; int value; + dpkg_error_destroy(&ps->err); + if (str_start[0] == '\0') - parse_error(ps, _("%s is missing"), what); + return dpkg_put_error(&ps->err, _("is missing a value")); nv = namevalue_find_by_name(nv_head, str_start); if (nv == NULL) { /* We got no match, skip further string validation. */ if (!(flags & PARSE_NV_FALLBACK)) - parse_error(ps, _("'%.50s' is not allowed for %s"), str_start, what); + return dpkg_put_error(&ps->err, _("has invalid value '%.50s'"), str_start); str_end = NULL; value = -1; @@ -87,7 +89,7 @@ parse_nv(struct parsedb_state *ps, enum parse_nv_flags flags, } if (!(flags & PARSE_NV_NEXT) && str_is_set(str_end)) - parse_error(ps, _("junk after %s"), what); + return dpkg_put_error(&ps->err, _("has trailing junk")); *strp = str_end; @@ -174,8 +176,11 @@ f_boolean(struct pkginfo *pkg, struct pkgbin *pkgbin, if (!*value) return; - boolean = parse_nv(ps, PARSE_NV_LAST, &value, booleaninfos, - _("yes/no in boolean field")); + boolean = parse_nv(ps, PARSE_NV_LAST, &value, booleaninfos); + if (dpkg_has_error(&ps->err)) + parse_error(ps, _("boolean (yes/no) '%s' field: %s"), + fip->name, ps->err.str); + STRUCTFIELD(pkgbin, fip->integer, bool) = boolean; } @@ -189,8 +194,10 @@ f_multiarch(struct pkginfo *pkg, struct pkgbin *pkgbin, if (!*value) return; - multiarch = parse_nv(ps, PARSE_NV_LAST, &value, multiarchinfos, - _("foreign/allowed/same/no in quadstate field")); + multiarch = parse_nv(ps, PARSE_NV_LAST, &value, multiarchinfos); + if (dpkg_has_error(&ps->err)) + parse_error(ps, _("quadstate (foreign/allowed/same/no) '%s' field: %s"), + fip->name, ps->err.str); STRUCTFIELD(pkgbin, fip->integer, int) = multiarch; } @@ -225,7 +232,9 @@ f_priority(struct pkginfo *pkg, struct pkgbin *pkgbin, if (!*value) return; priority = parse_nv(ps, PARSE_NV_LAST | PARSE_NV_FALLBACK, &str, - priorityinfos, _("word in 'Priority' field")); + priorityinfos); + if (dpkg_has_error(&ps->err)) + parse_error(ps, _("word in '%s' field: %s"), fip->name, ps->err.str); if (str == NULL) { pkg->priority = PKG_PRIO_OTHER; @@ -247,12 +256,18 @@ f_status(struct pkginfo *pkg, struct pkgbin *pkgbin, if (ps->flags & pdb_recordavailable) return; - pkg->want = parse_nv(ps, PARSE_NV_NEXT, &value, wantinfos, - _("first (want) word in 'Status' field")); - pkg->eflag = parse_nv(ps, PARSE_NV_NEXT, &value, eflaginfos, - _("second (error) word in 'Status' field")); - pkg->status = parse_nv(ps, PARSE_NV_LAST, &value, statusinfos, - _("third (status) word in 'Status' field")); + pkg->want = parse_nv(ps, PARSE_NV_NEXT, &value, wantinfos); + if (dpkg_has_error(&ps->err)) + parse_error(ps, _("first (want) word in '%s' field: %s"), + fip->name, ps->err.str); + pkg->eflag = parse_nv(ps, PARSE_NV_NEXT, &value, eflaginfos); + if (dpkg_has_error(&ps->err)) + parse_error(ps, _("second (error) word in '%s' field: %s"), + fip->name, ps->err.str); + pkg->status = parse_nv(ps, PARSE_NV_LAST, &value, statusinfos); + if (dpkg_has_error(&ps->err)) + parse_error(ps, _("third (status) word in '%s' field: %s"), + fip->name, ps->err.str); } void @@ -260,9 +275,8 @@ f_version(struct pkginfo *pkg, struct pkgbin *pkgbin, struct parsedb_state *ps, const char *value, const struct fieldinfo *fip) { - parse_db_version(ps, &pkgbin->version, value, - _("'%s' field value '%.250s'"), - fip->name, value); + if (parse_db_version(ps, &pkgbin->version, value) < 0) + parse_problem(ps, _("'%s' field value '%.250s'"), fip->name, value); } void @@ -298,10 +312,8 @@ f_configversion(struct pkginfo *pkg, struct pkgbin *pkgbin, if (ps->flags & pdb_recordavailable) return; - parse_db_version(ps, &pkg->configversion, value, - _("'%s' field value '%.250s'"), - fip->name, value); - + if (parse_db_version(ps, &pkg->configversion, value) < 0) + parse_problem(ps, _("'%s' field value '%.250s'"), fip->name, value); } /* @@ -578,9 +590,10 @@ f_dependency(struct pkginfo *pkg, struct pkgbin *pkgbin, varbuf_reset(&version); varbuf_add_buf(&version, versionstart, versionlength); varbuf_end_str(&version); - parse_db_version(ps, &dop->version, version.buf, - _("'%s' field, reference to '%.255s': " - "error in version"), fip->name, depname.buf); + if (parse_db_version(ps, &dop->version, version.buf) < 0) + parse_problem(ps, + _("'%s' field, reference to '%.255s': version '%s'"), + fip->name, depname.buf, version.buf); p++; while (c_isspace(*p)) p++; diff --git a/lib/dpkg/parse.c b/lib/dpkg/parse.c index a2663cf5f..2a96e7e46 100644 --- a/lib/dpkg/parse.c +++ b/lib/dpkg/parse.c @@ -529,6 +529,7 @@ parsedb_new(const char *filename, int fd, enum parsedbflags flags) struct parsedb_state *ps; ps = m_malloc(sizeof(*ps)); + ps->err = DPKG_ERROR_OBJECT; ps->filename = filename; ps->type = parse_get_type(ps, flags); ps->flags = flags; diff --git a/lib/dpkg/parsedump.h b/lib/dpkg/parsedump.h index 2a857c47d..8e9a71056 100644 --- a/lib/dpkg/parsedump.h +++ b/lib/dpkg/parsedump.h @@ -25,6 +25,8 @@ #include <stdint.h> +#include <dpkg/error.h> + /** * @defgroup parsedump In-core package database parsing and reading * @ingroup dpkg-public @@ -46,6 +48,7 @@ enum parsedbtype { struct parsedb_state { enum parsedbtype type; enum parsedbflags flags; + struct dpkg_error err; struct pkginfo *pkg; struct pkgbin *pkgbin; char *data; @@ -135,14 +138,19 @@ struct fieldinfo { size_t integer; }; -void parse_db_version(struct parsedb_state *ps, - struct dpkg_version *version, const char *value, - const char *fmt, ...) DPKG_ATTR_PRINTF(4); +int +parse_db_version(struct parsedb_state *ps, + struct dpkg_version *version, const char *value) + DPKG_ATTR_REQRET; void parse_error(struct parsedb_state *ps, const char *fmt, ...) DPKG_ATTR_NORET DPKG_ATTR_PRINTF(2); void parse_warn(struct parsedb_state *ps, const char *fmt, ...) DPKG_ATTR_PRINTF(2); +void +parse_problem(struct parsedb_state *ps, const char *fmt, ...) + DPKG_ATTR_PRINTF(2); + void parse_must_have_field(struct parsedb_state *ps, const char *value, const char *what); void parse_ensure_have_field(struct parsedb_state *ps, diff --git a/lib/dpkg/parsehelp.c b/lib/dpkg/parsehelp.c index caba83afb..a9095f1f4 100644 --- a/lib/dpkg/parsehelp.c +++ b/lib/dpkg/parsehelp.c @@ -77,6 +77,24 @@ parse_warn(struct parsedb_state *ps, const char *fmt, ...) va_end(args); } +void +parse_problem(struct parsedb_state *ps, const char *fmt, ...) +{ + va_list args; + char *str; + + va_start(args, fmt); + m_vasprintf(&str, parse_error_msg(ps, fmt), args); + va_end(args); + + if (ps->err.type == DPKG_MSG_WARN) + warning("%s: %s", str, ps->err.str); + else + ohshit("%s: %s", str, ps->err.str); + + free(str); +} + const struct fieldinfo * find_field_info(const struct fieldinfo *fields, const char *fieldname) { @@ -266,29 +284,24 @@ parseversion(struct dpkg_version *rversion, const char *string, * @param ps The parsedb state. * @param version The version to parse into. * @param value The version string to parse from. - * @param fmt The error format string. + * + * @retval 0 On success, and err is reset. + * @retval -1 On failure, and err is set accordingly. */ -void +int parse_db_version(struct parsedb_state *ps, struct dpkg_version *version, - const char *value, const char *fmt, ...) + const char *value) { - struct dpkg_error err; - va_list args; - char buf[1000]; + dpkg_error_destroy(&ps->err); - if (parseversion(version, value, &err) == 0) - return; + if (parseversion(version, value, &ps->err) == 0) + return 0; - va_start(args, fmt); - vsnprintf(buf, sizeof(buf), fmt, args); - va_end(args); - - if (err.type == DPKG_MSG_WARN && (ps->flags & pdb_lax_version_parser)) - parse_warn(ps, "%s: %.250s", buf, err.str); - else - parse_error(ps, "%s: %.250s", buf, err.str); + /* If not in lax mode, turn everything into an error. */ + if (!(ps->flags & pdb_lax_version_parser)) + ps->err.type = DPKG_MSG_ERROR; - dpkg_error_destroy(&err); + return -1; } void -- Dpkg.Org's dpkg
