On Thu, 2010-03-11 at 13:44 +0100, Jiri Moskovcak wrote:
Tested. Seems to work. (The bubble is still placed in a strange place though)
-void CApplet::CrashNotify(const char *format, ...) +void CApplet::action_report(NotifyNotification *notification, gchar *action, gpointer user_data) {
- va_list args;
- CApplet *applet = (CApplet *)user_data;
- if (applet->m_bDaemonRunning)
- {
pid_t pid = vfork();
if (pid< 0)
perror_msg("vfork");
if (pid == 0)
{ /* child */
char *buf = NULL;
int n = 0;
n = asprintf(&buf, "--report=%s", applet->m_pLastCrashID);
xasprintf() will be simpler (no need to have "int n" variable)...
Changed.
- * - Crash(progname,uid) - a new crash occurred (new /var/cache/abrt/DIR is found) + * - Crash(progname, uid, crash_id) - a new crash occurred (new /var/cache/abrt/DIR is found)
the order is wrong, should be: crash_id, uid
+ char *buf = NULL; + buf = xasprintf("--report=%s", applet->m_pLastCrashID);
This is technically correct, and gcc will optimize out redundant store, but you still may want to combine it into one line or at least remove "= NULL".
+ execlp("abrt-gui", "abrt-gui", buf, (char*) NULL); + free(buf); + perror_msg_and_die("Can't exec abrt-gui");
free() is not needed, exiting frees all. Consider removing if it doesn't go against your coding habits.
Please apply after you deal with these, I think no further review iterations are needed.