On Thu, 2010-05-06 at 13:22 +0200, Karel Klic wrote:
Abrtd dies because it reads a file /var/cache/abrt/*/uid and this
file
did not contain a number.
I propose to rewrite the UID loading to be more defensive, and report
MW_ERROR for the affected debug dump instead of killing whole daemon.
diff --git a/src/Daemon/MiddleWare.cpp b/src/Daemon/MiddleWare.cpp
index 667a36f..e6fba66 100644
--- a/src/Daemon/MiddleWare.cpp
+++ b/src/Daemon/MiddleWare.cpp
@@ -941,7 +941,22 @@ mw_result_t SaveDebugDump(const char *pDebugDumpDir,
return MW_ERROR;
}
- if (IsDebugDumpSaved(xatou(UID.c_str()), pDebugDumpDir))
+ /* Convert UID string to number uid_num. The UID string can be
modified by user or
+ wrongly saved (empty or non-numeric), so xatou() cannot be used
here,
+ because it would kill the daemon. */
+ char *endptr;
+ int old_errno = errno;
+ errno = 0;
+ unsigned long uid_num = strtoul(UID.c_str(), &endptr, 10);
+ if (errno || UID.c_str() == endptr || *endptr != '\0' || uid_num >
UINT_MAX)
+ {
+ error_msg("Invalid UID '%s' loaded from %s", UID.c_str(),
pDebugDumpDir);
+ errno = old_errno;
+ return MW_ERROR;
+ }
+ errno = old_errno;
You do not need to save/restore errno.
Usual idiom is: whoever needs errno, must check it immediately
after it is generated (or must save it, if can't check immediately).
IOW: you are protecting against SaveDebugDump() breaking code like this:
errno_generating_call(...);
SaveDebugDump(...);
if (errno == ESOMETHING) ...
but the above code is wrong - it should be
errno_generating_call(...);
sv_errno = errno;
SaveDebugDump(...);
if (sv_errno == ESOMETHING) ...
Thus, SaveDebugDump() may overwrite errno.
--
vda