Skip to content

possibility to configure kill_pgid also via environment variable - #21

Open
f0rki wants to merge 1 commit into
openSUSE:mainfrom
f0rki:killpg-env
Open

f0rki wants to merge 1 commit into
openSUSE:mainfrom
f0rki:killpg-env

Conversation

@f0rki

@f0rki f0rki commented Mar 18, 2022

Copy link
Copy Markdown

Sometimes it is inconvenient to pass a command line flag to catatonit, e.g., the -g flag. So this PR adds environment variable to configure catatonit to forward signals to the whole process group.

@cyphar

cyphar commented Oct 3, 2023

Copy link
Copy Markdown
Member

Can you please rebase this? We have changed the license from GPL-3.0-or-later to GPL-2.0-or-later and we need you to rebase it in order to confirm that you agree with the new license terms.

Signed-off-by: Michael Rodler <m@mrodler.eu>
@f0rki

f0rki commented Oct 5, 2023

Copy link
Copy Markdown
Author

@cyphar done.

Comment thread catatonit.c
if (argc < 1 && !run_as_pause)
bail_usage("missing program name");

char *kill_pgid_env = secure_getenv("CATATONIT_KILLPG");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

secure_getenv doesn't make sense in this context -- secure_getenv is only relevant for setuid binaries (and even then, it only really makes sense for general purpose libraries that might be used by a setuid binary).

Comment thread catatonit.c
Comment on lines +486 to +488
if (kill_pgid_env != NULL) {
kill_pgid = true;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the Linux kernel style:

Suggested change
if (kill_pgid_env != NULL) {
kill_pgid = true;
}
if (kill_pgid_env)
kill_pgid = true;

or even just do:

Suggested change
if (kill_pgid_env != NULL) {
kill_pgid = true;
}
kill_pgid |= getenv("CATATONIT_KILLPG") != NULL;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants