Skip to content

Commit

Permalink
Fix execv() failure handling and argv0 usage. (#105)
Browse files Browse the repository at this point in the history
In some cases:

  - we would use argv[0] as the binary to invoke again in sub-programs
  - execv() would fail with "No such file or directory"

Then our handling of execv() failure was leaving a process and a pipe
behind, leading to a strange process tree and a bad situation.

This patch fixes the code so that we get the realpath for the currently
executed program (using proper API in macOS but a heuristic for other OSes),
and fixes the error handling at the execv() call site too.
  • Loading branch information
DimCitus committed Nov 19, 2019
1 parent 7bb38fd commit b367074
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 16 deletions.
4 changes: 4 additions & 0 deletions src/bin/lib/subcommands.c/runprogram.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ execute_program(Program *prog)
{
prog->returnCode = -1;
prog->error = errno;

fprintf(stdout, "%s\n", strerror(errno));
fprintf(stderr, "%s\n", strerror(errno));
exit(EXIT_CODE_INTERNAL_ERROR);
}
return;
}
Expand Down
2 changes: 2 additions & 0 deletions src/bin/pg_autoctl/cli_root.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "commandline.h"

extern char pg_autoctl_argv0[];
extern char pg_autoctl_program[];
extern int logLevel;

extern CommandLine help;
extern CommandLine version;
Expand Down
5 changes: 4 additions & 1 deletion src/bin/pg_autoctl/file_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ unlink_file(const char *filename)
* then argv[0] (here pg_autoctl_argv0) is just "pg_autoctl".
*/
bool
get_program_absolute_path(char *program, int size)
set_program_absolute_path(char *program, int size)
{
#if defined(__APPLE__)
int actualSize = _NSGetExecutablePath(program, (uint32_t *) &size);
Expand All @@ -438,6 +438,9 @@ get_program_absolute_path(char *program, int size)
"to %d bytes only", actualSize, size);
return false;
}

log_debug("Found absolute program: \"%s\"", program);

#else
/*
* On Linux and FreeBSD and Solaris, we can find a symbolic link to our
Expand Down
2 changes: 1 addition & 1 deletion src/bin/pg_autoctl/file_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ void path_in_same_directory(const char *basePath,
int search_pathlist(const char *pathlist, const char *filename, char ***result);
void search_pathlist_destroy_result(char **result);
bool unlink_file(const char *filename);
bool get_program_absolute_path(char *program, int size);
bool set_program_absolute_path(char *program, int size);

#endif /* FILE_UTILS_H */
26 changes: 20 additions & 6 deletions src/bin/pg_autoctl/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "keeper_config.h"

char pg_autoctl_argv0[MAXPGPATH];
char pg_autoctl_program[MAXPGPATH];

/*
* Main entry point for the binary.
Expand All @@ -26,12 +27,6 @@ main(int argc, char **argv)
{
CommandLine command = root;

/*
* Stash away the ARGV[0] used to run this program, we might need it to
* fill in our systemd service unit configuration file later.
*/
strlcpy(pg_autoctl_argv0, argv[0], MAXPGPATH);

/*
* When PG_AUTOCTL_DEBUG is set in the environement, provide the user
* commands available to debug a pg_autoctl instance.
Expand Down Expand Up @@ -64,6 +59,25 @@ main(int argc, char **argv)
*/
log_use_colors(isatty(fileno(stderr)));

/*
* Stash away the argv[0] used to run this program and compute the realpath
* of the program invoked, which we need at several places including when
* preparing the systemd unit files.
*
* Note that we're using log_debug() in get_program_absolute_path and we
* have not set the log level from the command line option parsing yet. We
* hard-coded LOG_INFO as our log level. For now we won't see the log_debug
* output, but as a developer you could always change the LOG_INFO to
* LOG_DEBUG above and then see the message.
*/
strlcpy(pg_autoctl_argv0, argv[0], MAXPGPATH);

if (!set_program_absolute_path(pg_autoctl_program, MAXPGPATH))
{
/* errors have already been logged */
exit(EXIT_CODE_INTERNAL_ERROR);
}

(void) commandline_run(&command, argc, argv);

return 0;
Expand Down
9 changes: 1 addition & 8 deletions src/bin/pg_autoctl/systemd_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ void
systemd_config_init(SystemdServiceConfig *config, const char *pgdata)
{
IniOption systemdOptions[] = SET_INI_OPTIONS_ARRAY(config);
char program[MAXPGPATH] = { 0 };

/* time to setup config->pathnames.systemd */
snprintf(config->pathnames.systemd, MAXPGPATH,
Expand All @@ -91,13 +90,7 @@ systemd_config_init(SystemdServiceConfig *config, const char *pgdata)

strlcpy(config->User, config->pgSetup.username, NAMEDATALEN);

if (!get_program_absolute_path(program, MAXPGPATH))
{
/* errors have already been logged */
exit(EXIT_CODE_INTERNAL_ERROR);
}

snprintf(config->ExecStart, BUFSIZE, "%s run", program);
snprintf(config->ExecStart, BUFSIZE, "%s run", pg_autoctl_program);

if (!ini_validate_options(systemdOptions))
{
Expand Down

0 comments on commit b367074

Please sign in to comment.