On 12/04/2012 12:52 PM, Denys Vlasenko wrote:
On 11/28/2012 06:39 PM, Jakub Filak wrote:
+#include <NetworkManager.h>
...
+static bool is_networking_enabled(void)
+{
+ GError *error = NULL;
+
+ /* Create a D-Bus proxy to get the object properties from the NM Manager
+ * object. NM_DBUS_* defines are from NetworkManager.h.
+ */
+ const int flags = G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES
+ | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS;
+
+ GDBusProxy *props_proxy = g_dbus_proxy_new_sync(g_system_bus,
+ flags,
+ NULL /* GDBusInterfaceInfo */,
+ NM_DBUS_SERVICE,
+ NM_DBUS_PATH,
+ DBUS_INTERFACE_PROPERTIES,
+ NULL /* GCancellable */,
+ &error);
Looks like you need NetworkManager.h and NetworkManager-devel
for these two NM_foo defines.
I prefer to not have NetworkManager.h and NetworkManager-devel
dependency. For one, NetworkManager-devel requires NetworkManager,
and I _definitely_ don't want that package on my machine.
IOW: I can't build abrt package on my machine anymore.
The values are:
NM_DBUS_SERVICE = "org.freedesktop.NetworkManager"
NM_DBUS_PATH = "/org/freedesktop/NetworkManager"
NM_DBUS_INTERFACE = "org.freedesktop.NetworkManager"
Can we use them directly?
+ error_msg ("Failed to get NetworkManager proxy: %s", error->message);
- + need to be translatable ^^^
I think user is not going to understand this error message.
"Can't connect to NetworkManager over DBus" maybe?
+ error_msg ("Failed to Get State property: %s", error->message);
Same. I propose:
"Can't determine network status via NetworkManager: %s".
static void fork_exec_gui(void)
{
- pid_t pid = vfork();
+ pid_t pid = fork();
if (pid < 0)
- perror_msg("vfork");
+ perror_msg("fork");
I don't see why you did vfork -> fork change here.
if (pid == 0)
{
/* child */
- signal(SIGCHLD, SIG_DFL); /* undo SIG_IGN in abrt-applet */
-//TODO: pass s_dirs[] as DIR param(s) to abrt-gui
+ /* double fork to avoid GUI zombies */
+ pid_t grand_child = fork();
+ if (grand_child != 0)
+ {
+ if (grand_child < 0) perror_msg("vfork");
Here you can use vfork too.
(If not for some reason I don't see, then message is not consistent)
+ exit(0);
+ }
+
+ /* grand child */
+ //TODO: pass s_dirs[] as DIR param(s) to abrt-gui
execl(BIN_DIR"/abrt-gui", "abrt-gui", (char*) NULL);
/* Did not find abrt-gui in installation directory. Oh well */
/* Trying to find it in PATH */
execlp("abrt-gui", "abrt-gui", (char*) NULL);
perror_msg_and_die("Can't execute '%s'", "abrt-gui");
}
+
+ safe_waitpid(pid, /* status */ NULL, /* options */ 0);
+
What if pid == -1 here?
+static void run_report_from_applet(const char *dirname)
+{
+ /* prevent zombies; double fork() */
+ pid_t pid = fork();
+ if (pid < 0)
+ perror_msg("fork()");
+ if (pid == 0)
+ {
+ spawn_event_handler_child(dirname, "report-gui", NULL);
+ exit(0);
+ }
+
+ safe_waitpid(pid, /* status */ NULL, /* options */ 0);
+}
Similar in code above.
