Skip to content
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

implement the signal command #39 #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions main/gridinit.c
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,104 @@ command_stop(GString *out, int argc, char **argv)
return service_run_groupv(argc, argv, out, stop_process);
}

static void
command_signal(GString *out, int argc, char **argv)
{
char *str_signal;
int signal;

static struct signal_mapping_s {
char *name;
int value;
} SIGNAL [] = {
#ifdef __unix__ // the base signals
Copy link

Choose a reason for hiding this comment

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

First, lot of copy and paste, you should use a macro

#define ITEM(x) {x, #x}

and have

] SIGNAL [] = {
ITEM(SIGABRT),
ITEM(SIGHUP),
...
}

{ "SIGABRT", SIGABRT},
{ "SIGALRM", SIGALRM },
{ "SIGBUS", SIGBUS },
{ "SIGCHLD", SIGCHLD },
{ "SIGCONT", SIGCONT },
{ "SIGEMT", SIGEMT },
{ "SIGFPE", SIGFPE },
{ "SIGHUP", SIGHUP },
{ "SIGILL", SIGILL },
{ "SIGINFO", SIGINFO },
{ "SIGINT", SIGINT },
{ "SIGIO", SIGIO },
{ "SIGKILL", SIGKILL },
{ "SIGPIPE", SIGPIPE },
{ "SIGPROF", SIGPROF },
{ "SIGQUIT", SIGQUIT },
{ "SIGSEGV", SIGSEGV },
{ "SIGSTOP", SIGSTOP },
{ "SIGSYS", SIGSYS },
{ "SIGTERM", SIGTERM },
{ "SIGTRAP", SIGTRAP },
{ "SIGTSTP", SIGTSTP },
{ "SIGTTIN", SIGTTIN },
{ "SIGTTOU", SIGTTOU },
{ "SIGURG", SIGURG },
{ "SIGUSR1", SIGUSR1 },
{ "SIGUSR2", SIGUSR2 },
{ "SIGVTALRM", SIGVTALRM },
{ "SIGWINCH", SIGWINCH },
{ "SIGXCPU", SIGXCPU },
{ "SIGXFSZ", SIGXFSZ },
#else
#error Your plateform is not supported
Copy link

Choose a reason for hiding this comment

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

I prefer to have #error at top

#endif

#ifdef __FreeBSD__ // freebsd specific signals
{ "SIGTHR", SIGTHR },
{ "SIGLIBRT", SIGLIBRT },
#endif
#ifdef __linux__ // linux specific signals
{ "SIGCLD", SIGCLD },
{ "SIGIOT", SIGIOT },
{ "SIGLOST", SIGLOST },
{ "SIGPOLL", SIGPOLL },
{ "SIGPWR", SIGPWR },
{ "SIGSTKFLT", SIGSTKFLT },
{ "SIGUNUSED", SIGUNUSED },
{ NULL, 0 },
#endif
Copy link
Author

Choose a reason for hiding this comment

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

I don’t think we should put this thing here. It’s not good for code readability.
It would probably be best to create a plateform file or a signal file for this.

Copy link

Choose a reason for hiding this comment

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

👍 to have a dedicated file

};

errno = 0; // this should not be needed

if( argc <= 0) {
g_string_append_printf(out, "No signal were specified\n");
Copy link
Author

Choose a reason for hiding this comment

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

We should return an error here, but I have no idea on how to do this.

return; // nothing to do
}

str_signal = argv[0];
argv++;
argc--;

// first we try to convert the string to an integer
signal = strtol(str_signal, NULL, 10); // TODO should we autodetect the base?
Copy link
Author

@irevoire irevoire Jan 21, 2020

Choose a reason for hiding this comment

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

Do we want to allow hexadecimal signals and things like that?

if (errno != 0) { // conversion failed
errno = 0;
// we look into the of known signals
for (struct signal_mapping_s* s = SIGNAL;; s++) {
if (s->name == NULL) { // we did not find any signals
g_string_append_printf(out, "Unable to decode the signal %s\n", str_signal);
Copy link
Author

Choose a reason for hiding this comment

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

send an error

return;
}
if (0 == strcasecmp(str_signal, s->name)) {
signal = s->value;
break;
}
}
}

void signal_process(void *u UNUSED, struct child_info_s *ci) {
kill(ci->pid, signal);
Copy link

Choose a reason for hiding this comment

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

IMO we should emit a message with kill return an error

}

g_assert_nonnull(out);
return service_run_groupv(argc, argv, out, signal_process);
}

static void
command_restart(GString *out, int argc, char **argv)
{
Expand Down Expand Up @@ -427,6 +525,7 @@ __resolve_command(const gchar *n)
{"repair", command_repair },
{"start", command_start },
{"stop", command_stop },
{"signal", command_signal },
{"restart", command_restart },
{"reload", command_reload },
{NULL, NULL}
Expand Down
13 changes: 13 additions & 0 deletions main/gridinit_cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ static const gchar description[] =
" kill : Stops the given processes or groups, they won't be automatically\n"
" restarted even after a configuration reload\n"
" stop : Calls 'kill' until the children exit\n"
" signal : Send a choosen signal to the given processes or groups\n"
" restart : Restarts the given processes or groups\n"
" reload : Reloads the configuration, stopping obsolete processes, starting\n"
" the newly discovered. Broken or stopped processes are not restarted\n"
Expand Down Expand Up @@ -630,6 +631,17 @@ command_kill(int argc, char **args)
|| dump_args.count_success == 0;
}

static int
command_signal(int argc, char **args)
{
struct dump_as_is_arg_s dump_args = {};

int rc = send_commandv(dump_as_is, &dump_args, "signal", argc, args);
return rc
|| dump_args.count_errors != 0
|| dump_args.count_success == 0;
}

static gboolean
_all_down(char **args, gboolean *down)
{
Expand Down Expand Up @@ -713,6 +725,7 @@ struct command_s {
{ "status3", command_status2 },
{ "start", command_start },
{ "stop", command_stop },
{ "signal", command_signal },
{ "kill", command_kill },
{ "restart", command_restart },
{ "reload", command_reload },
Expand Down