Jakub Filak <jfilak(a)redhat.com> writes:
Hi Nikola,
On Thursday 26 of April 2012 12:19:42 Nikola Pajkovsky wrote:
> Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com>
> ---
> src/daemon/abrt-action-save-package-data.c | 46 +++++++++--------
> src/daemon/rpm.c | 75
> +++++++++++++++++++++++++--- src/daemon/rpm.h |
> 13 ++++-
> 3 files changed, 105 insertions(+), 29 deletions(-)
>
> diff --git a/src/daemon/abrt-action-save-package-data.c
> b/src/daemon/abrt-action-save-package-data.c index 00b2843..8921b6c 100644
> --- a/src/daemon/abrt-action-save-package-data.c
> +++ b/src/daemon/abrt-action-save-package-data.c
> @@ -181,8 +181,7 @@ static bool is_path_blacklisted(const char *path)
> return false;
> }
>
> -static int get_script_name(const char *cmdline, char **executable,
> - char **package_full_name)
> +static struct pkg_envra *get_script_name(const char *cmdline, char
> **executable) {
> // TODO: we don't verify that python executable is not modified
> // or that python package is properly signed
> @@ -191,7 +190,7 @@ static int get_script_name(const char *cmdline, char
> **executable, * This will work only if the cmdline contains the whole path.
> * Example: python /usr/bin/system-control-network
> */
> - char *script_pkg = NULL;
> + struct pkg_envra *script_pkg = NULL;
> char *script_name = get_argv1_if_full_path(cmdline);
> if (script_name)
> {
> @@ -203,24 +202,22 @@ static int get_script_name(const char *cmdline, char
> **executable, * Replace interpreter's package_full_name and executable *
> with data pertaining to the script.
> */
> - free(*package_full_name);
> - *package_full_name = script_pkg;
> *executable = script_name;
> /* executable has changed, check it again */
> if (is_path_blacklisted(*executable))
> {
> log("Blacklisted executable '%s'", *executable);
> - return 1;
> + return NULL;
> }
> }
> }
> if (!script_pkg && !settings_bProcessUnpackaged)
> {
> log("Interpreter crashed, but no packaged script detected:
'%s'",
> cmdline); - return 1;
> + return NULL;
> }
>
> - return 0;
> + return script_pkg;
> }
>
> static int SavePackageDescriptionToDebugDump(const char *dump_dir_name)
> @@ -243,9 +240,8 @@ static int SavePackageDescriptionToDebugDump(const char
> *dump_dir_name) char *cmdline = NULL;
> char *executable = NULL;
> char *rootdir = NULL;
> - char *script_name = NULL; /* only if "interpreter /path/to/script" */
> char *package_short_name = NULL;
> - char *package_full_name = NULL;
> + struct pkg_envra *pkg_name = NULL;
> char *component = NULL;
> int error = 1;
> /* note: "goto ret" statements below free all the above variables,
> @@ -266,8 +262,8 @@ static int SavePackageDescriptionToDebugDump(const char
> *dump_dir_name) goto ret; /* return 1 (failure) */
> }
>
> - package_full_name = rpm_get_package_nvr(executable, rootdir);
> - if (!package_full_name)
> + pkg_name = rpm_get_package_nvr(executable, rootdir);
> + if (!pkg_name)
> {
> if (settings_bProcessUnpackaged)
> {
> @@ -289,13 +285,16 @@ static int SavePackageDescriptionToDebugDump(const
> char *dump_dir_name) if (!strcmp(basename, "python")
>
> || !strcmp(basename, "perl"))
>
> {
> - int r = get_script_name(cmdline, &executable, &package_full_name);
> - if (r)
> + struct pkg_envra *script_pkg = get_script_name(cmdline,
> &executable); + if (!script_pkg)
> goto ret;
> +
> + free_pkg_envra(pkg_name);
> + pkg_name = script_pkg;
> }
>
> - package_short_name =
> get_package_name_from_NVR_or_NULL(package_full_name); - VERB2
> log("Package:'%s' short:'%s'", package_full_name,
package_short_name); +
> package_short_name = xasprintf("%s", pkg_name->p_name);
> + VERB2 log("Package:'%s' short:'%s'",
pkg_name->p_nvr,
> package_short_name);
>
> GList *li;
>
> @@ -329,14 +328,18 @@ static int SavePackageDescriptionToDebugDump(const
> char *dump_dir_name) if (!dd)
> goto ret; /* return 1 (failure) */
>
> - if (package_full_name)
> + if (pkg_name)
> {
> - dd_save_text(dd, FILENAME_PACKAGE, package_full_name);
> + dd_save_text(dd, FILENAME_PACKAGE, pkg_name->p_nvr);
> + dd_save_text(dd, FILENAME_PKG_EPOCH, pkg_name->p_epoch);
> + dd_save_text(dd, FILENAME_PKG_NAME, pkg_name->p_name);
> + dd_save_text(dd, FILENAME_PKG_VERSION, pkg_name->p_version);
> + dd_save_text(dd, FILENAME_PKG_RELEASE, pkg_name->p_release);
> + dd_save_text(dd, FILENAME_PKG_ARCH, pkg_name->p_arch);
> }
> +
> if (component)
> - {
> dd_save_text(dd, FILENAME_COMPONENT, component);
> - }
>
> dd_close(dd);
>
> @@ -346,9 +349,8 @@ static int SavePackageDescriptionToDebugDump(const char
> *dump_dir_name) free(cmdline);
> free(executable);
> free(rootdir);
> - free(script_name);
> free(package_short_name);
> - free(package_full_name);
> + free_pkg_envra(pkg_name);
> free(component);
>
> return error;
> diff --git a/src/daemon/rpm.c b/src/daemon/rpm.c
> index 2fe57b8..33075a5 100644
> --- a/src/daemon/rpm.c
> +++ b/src/daemon/rpm.c
> @@ -220,13 +220,14 @@ char* rpm_get_component(const char *filename, const
> char *rootdir_or_NULL) }
>
> // caller is responsible to free returned value
> -char* rpm_get_package_nvr(const char *filename, const char
> *rootdir_or_NULL) +struct pkg_envra *rpm_get_package_nvr(const char
> *filename, const char *rootdir_or_NULL) {
> - char *nvr = NULL;
> rpmts ts;
> rpmdbMatchIterator iter;
> Header header;
>
> + struct pkg_envra *p = NULL;
> +
> ts = rpmtsCreate();
> /* This loop executes once (normally) or twice (if we detect chroot) */
> while (1)
> @@ -255,13 +256,75 @@ char* rpm_get_package_nvr(const char *filename, const
> char *rootdir_or_NULL) }
>
> const char *errmsg = NULL;
> - nvr = headerFormat(header, "%{NAME}-%{VERSION}-%{RELEASE}",
&errmsg);
> - if (!nvr && errmsg)
> - error_msg("cannot get nvr. reason: %s", errmsg);
> +
> + p = xzalloc(sizeof(*p));
> + p->p_epoch = headerFormat(header, "%{epoch}", &errmsg);
> + if (!p->p_epoch && errmsg)
> + {
> + error_msg("cannot get epoch. reason: %s", errmsg);
> + goto error;
> + }
> +
> + /*
> + * <npajkovs> hello, what's the difference between epoch '0'
and
> '(none)'? + * <Panu> nothing really, a missing epoch is considered
equal
> to zero epoch + */
> + if (!strncmp(p->p_epoch, "(none)", strlen("(none)")))
> + {
> + free(p->p_epoch);
> + p->p_epoch = xstrdup("0");
> + }
> +
> + p->p_name = headerFormat(header, "%{name}", &errmsg);
> + if (!p->p_name && errmsg)
> + {
> + error_msg("cannot get name. reason: %s", errmsg);
> + goto error;
> + }
> +
> + p->p_version = headerFormat(header, "%{version}", &errmsg);
> + if (!p->p_version && errmsg)
> + {
> + error_msg("cannot get version. reason: %s", errmsg);
> + goto error;
> + }
> +
> + p->p_release = headerFormat(header, "%{release}", &errmsg);
> + if (!p->p_version && errmsg)
[1] - copy paste issue :) (consider a function or a macro)
yea, I've already fix another bug before I sent patch. so be it as func
or macro
> + {
> + error_msg("cannot get release. reason: %s", errmsg);
> + goto error;
> + }
> +
> + p->p_arch = headerFormat(header, "%{arch}", &errmsg);
> + if (!p->p_arch && errmsg)
> + {
> + error_msg("cannot get arch. reason: %s", errmsg);
> + goto error;
> + }
> +
> + p->p_nvr = xasprintf("%s-%s-%s", p->p_name, p->p_version,
> p->p_release); +
> + return p;
>
> error:
> + free_pkg_envra(p);
> +
> rpmdbFreeIterator(iter);
> error1:
> rpmtsFree(ts);
> - return nvr;
> + return NULL;
> +}
> +
> +void free_pkg_envra(struct pkg_envra *p)
> +{
> + if (!p)
> + return;
> +
[2 ] - is it intentional to not free p->n_nvr here?
good catch. I'm going to fix it.
> + free(p->p_epoch);
> + free(p->p_name);
> + free(p->p_version);
> + free(p->p_release);
> + free(p->p_arch);
> + free(p);
> }
> diff --git a/src/daemon/rpm.h b/src/daemon/rpm.h
> index bd17bec..1b90368 100644
> --- a/src/daemon/rpm.h
> +++ b/src/daemon/rpm.h
> @@ -31,6 +31,17 @@
> extern "C" {
> #endif
>
> +struct pkg_envra {
> + char *p_nvr;
> + char *p_epoch;
> + char *p_name;
> + char *p_version;
> + char *p_release;
> + char *p_arch;
> +};
> +
> +void free_pkg_envra(struct pkg_envra *p);
> +
> /**
> * Checks if an application is modified by third party.
> * @param pPackage A package name. The package contains the application.
> @@ -64,7 +75,7 @@ int rpm_chk_fingerprint(const char* pkg);
> * @param filename A file name.
> * @return A package name (malloc'ed string)
> */
> -char* rpm_get_package_nvr(const char *filename, const char
> *rootdir_or_NULL); +struct pkg_envra *rpm_get_package_nvr(const char
> *filename, const char *rootdir_or_NULL); /**
> * Finds a main package for given file. This package contains particular
> * file. If the file doesn't belong to any package, empty string is
Regards,
Jakub
--
Nikola