On 09/22/2010 01:55 PM, Denys Vlasenko wrote:
On Wed, 2010-09-22 at 12:48 +0200, Nikola Pajkovsky wrote:
dsc = xstrdup("Crashed executable does not belong to any installed package");
goto reopen_dd_and_save;
... ... ...
- reopen_dd_and_save:
dd = dd_init();
if (!dd_opendir(dd, dump_dir_name))
{
dd_close(dd);
result = 1;
goto ret;
}
I'm not a fan of goto in this context. goto as jump to the end of function is pretty good, but this ;).
Fixed.
/*
Checking the MD5 sum requires to run prelink to "un-prelink" the
binaries - this is considered potential security risk so we don't
use it, until we find some non-intrusive way
Delete?
*/
/*
if (!CheckHash(package_short_name.c_str(), executable))
{
error_msg("Executable '%s' seems to be modified, "
"doesn't match one from package '%s'",
executable, package_short_name.c_str());
result = 1;
goto ret;
}
*/
yes, it's not a problem dig it from git again.
The purpose of this comment is to stop someone from adding this code again. The comment can be shortened, though...
+abrt_action_save_package_data_SOURCES = \
- rpm.h rpm.c \
- Settings.h Settings.cpp \
- abrt-action-save-package-data.cpp
+abrt_action_save_package_data_CPPFLAGS = \
- -I$(srcdir)/../../inc \
- -I$(srcdir)/../../lib/utils \
- -DBIN_DIR="$(bindir)" \
- -DVAR_RUN="$(VAR_RUN)" \
- -DCONF_DIR="$(CONF_DIR)" \
- -DLOCALSTATEDIR='"$(localstatedir)"' \pacakge ^^^ typo
Fixed.
When I see function 200 lines long. I'm the guy at the position 8. (http://www.pakzilla.com/2007/09/05/programmers-life-cartoon/)
The original SavePackageDescriptionToDebugDump() is about the same size. Things aren't getting worse in this regard.
Please review the attached version 2 of the patch.
commit it. Thanks.