-
-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Add process_name key to support broken/missing _NET_WM_PID #28
base: master
Are you sure you want to change the base?
Changes from all commits
2067264
72abbe0
fa95e9e
0437d2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,11 @@ | |
# # Also suspend descendant processes that match this regex. | ||
# suspend_subtree_pattern = . | ||
# | ||
# # Assume that all windows matching this rule belong to the | ||
# # process with this name. This is a workaround for | ||
# # missing or incorrect _NET_WM_PID values, e.g. with Flatpak. | ||
# process_name = ... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should in fact mention this matches cmdlines, so I wonder if we couldn't have a better expressed name, such as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And then need to explain this is being matched as literal strings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I missed that. Wouldn't it be more versatile to match full cmdlines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there's a good way to match on the full cmdline. Contains matchers would be too broad. Prefix and exact matchers would be too cumbersome. I don't think being able to match on the path to the executable is worth it. Regex could work, but I think that would be error-prone especially when paths contain spaces. Matching on the process name + args would however be an improvement, I think. |
||
# | ||
# # Whether to apply the rule only when on battery power. | ||
# only_on_battery = true | ||
# | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
#include "proc.h" | ||
#include <limits.h> | ||
#include <proc/readproc.h> | ||
#include "rule.h" | ||
|
||
|
||
static | ||
gboolean | ||
cmdline_matches (char **cmdline, char *s) | ||
{ | ||
if (cmdline == NULL) | ||
return FALSE; | ||
|
||
char *basename = g_path_get_basename (cmdline[0]); | ||
gboolean result = g_strcmp0 (basename, s) == 0; | ||
g_free (basename); | ||
return result; | ||
} | ||
|
||
|
||
static | ||
proc_t* | ||
proc_by_pid (pid_t pid) | ||
{ | ||
pid_t arr[2] = {pid, 0}; | ||
PROCTAB *pt = openproc(PROC_FILLARG | PROC_FILLSTAT | PROC_PID, arr); | ||
proc_t *result = readproc(pt, NULL); | ||
closeproc(pt); | ||
return result; | ||
} | ||
|
||
|
||
static | ||
pid_t | ||
process_name_get_pid (char *process_name) | ||
{ | ||
// Types match readproc.c | ||
pid_t cand_pid = 0; | ||
unsigned long long cand_start_time = ULLONG_MAX; // NOLINT(runtime/int) | ||
|
||
PROCTAB *pt = openproc(PROC_FILLARG | PROC_FILLSTAT); | ||
proc_t *proc; | ||
|
||
while ((proc = readproc(pt, NULL))) { | ||
pid_t tree_cand_pid = 0; | ||
unsigned long long tree_cand_start_time = ULLONG_MAX; // NOLINT(runtime/int) | ||
|
||
// Some applications use multiple processes with the same name -- find the root one | ||
pid_t ppid; | ||
do { | ||
if (cmdline_matches(proc->cmdline, process_name)) { | ||
tree_cand_pid = proc->tid; | ||
tree_cand_start_time = proc->start_time; | ||
} | ||
ppid = proc->ppid; | ||
freeproc(proc); | ||
} while ((proc = proc_by_pid(ppid))); | ||
|
||
if (tree_cand_pid != 0 && tree_cand_pid != cand_pid) { | ||
if (cand_pid != 0) | ||
g_warning("Multiple processes named '%s': %u and %u", | ||
process_name, cand_pid, tree_cand_pid); | ||
|
||
if (tree_cand_start_time < cand_start_time || | ||
(tree_cand_start_time == cand_start_time && | ||
tree_cand_pid < cand_pid)) { | ||
// Update | ||
cand_pid = tree_cand_pid; | ||
cand_start_time = tree_cand_start_time; | ||
} | ||
} | ||
} | ||
|
||
closeproc(pt); | ||
return cand_pid; | ||
} | ||
|
||
|
||
pid_t | ||
xsus_window_get_pid (WnckWindow *window) | ||
{ | ||
Rule *rule = xsus_window_get_rule (window); | ||
char *process_name = rule ? rule->process_name : NULL; | ||
pid_t pid_by_process_name = process_name ? process_name_get_pid(process_name) : 0; | ||
// Prefer getting the PID by process name so we can override _NET_WM_PID if it's wrong | ||
// (e.g. when using flatpak) | ||
return pid_by_process_name ? pid_by_process_name : wnck_window_get_pid(window); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
#ifndef XSUSPENDER_PROC_H | ||
#define XSUSPENDER_PROC_H | ||
#include <libwnck/libwnck.h> | ||
|
||
int xsus_window_get_pid (WnckWindow *window); | ||
|
||
#endif // XSUSPENDER_PROC_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know the package name(s) for RPM distros