[lxc-devel] [PATCH RFC] lxc_*.c: don't exit with -1
Stéphane Graber
stgraber at ubuntu.com
Mon Apr 7 21:11:01 UTC 2014
On Mon, Apr 07, 2014 at 11:56:20AM -0500, Serge Hallyn wrote:
> In this patch I tried to stick with each file's coding style, however I
> think we should probably change that. Every main() should always not
> return and only exit; they should always return EXIT_SUCCESS or EXIT_FAILURE
> with the only exceptions being cases where we are returning a child's
> exit status (lxc_execute, lxc_attach, lxc_init).
Thanks for that, those return values have been a bit until now...
I'll cherry-pick into stable too, hoping nobody actually relied on those
high return values until now and just used good old != 0 till now.
Acked-by: Stéphane Graber <stgraber at ubuntu.com>
>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> ---
> doc/lxc-stop.sgml.in | 35 +++++++++++++++++++++++++++++++++++
> src/lxc/lxc_attach.c | 12 ++++++------
> src/lxc/lxc_cgroup.c | 14 +++++++-------
> src/lxc/lxc_clone.c | 2 +-
> src/lxc/lxc_config.c | 2 +-
> src/lxc/lxc_console.c | 2 +-
> src/lxc/lxc_destroy.c | 4 ++--
> src/lxc/lxc_execute.c | 20 ++++++++++++--------
> src/lxc/lxc_freeze.c | 4 ++--
> src/lxc/lxc_init.c | 4 +++-
> src/lxc/lxc_monitor.c | 14 +++++++-------
> src/lxc/lxc_monitord.c | 6 ++++--
> src/lxc/lxc_snapshot.c | 8 +++++---
> src/lxc/lxc_stop.c | 2 ++
> src/lxc/lxc_unfreeze.c | 4 ++--
> src/lxc/lxc_unshare.c | 6 +++---
> src/lxc/lxc_usernsexec.c | 2 +-
> src/lxc/lxc_wait.c | 10 +++++-----
> 18 files changed, 99 insertions(+), 52 deletions(-)
>
> diff --git a/doc/lxc-stop.sgml.in b/doc/lxc-stop.sgml.in
> index dc002c5..bc5e6a8 100644
> --- a/doc/lxc-stop.sgml.in
> +++ b/doc/lxc-stop.sgml.in
> @@ -163,6 +163,41 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> </refsect1>
>
> <refsect1>
> + <title>Exit value</title>
> +
> + <variablelist>
> +
> + <varlistentry>
> + <term>0</term>
> + <listitem>
> + <para>
> + The container was successfully stopped.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> + <term>1</term>
> + <listitem>
> + <para>
> + An error occurred while stopping the container.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> + <term>2</term>
> + <listitem>
> + <para>
> + The specified container exists but was not running.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + </variablelist>
> +
> + </refsect1>
> + <refsect1>
> <title>Diagnostic</title>
>
> <variablelist>
> diff --git a/src/lxc/lxc_attach.c b/src/lxc/lxc_attach.c
> index 3fbcb82..3757ff9 100644
> --- a/src/lxc/lxc_attach.c
> +++ b/src/lxc/lxc_attach.c
> @@ -195,11 +195,11 @@ int main(int argc, char *argv[])
>
> ret = lxc_caps_init();
> if (ret)
> - return ret;
> + return 1;
>
> ret = lxc_arguments_parse(&my_args, argc, argv);
> if (ret)
> - return ret;
> + return 1;
>
> if (!my_args.log_file)
> my_args.log_file = "none";
> @@ -207,7 +207,7 @@ int main(int argc, char *argv[])
> ret = lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
> my_args.progname, my_args.quiet, my_args.lxcpath[0]);
> if (ret)
> - return ret;
> + return 1;
> lxc_log_options_no_override();
>
> if (remount_sys_proc)
> @@ -229,14 +229,14 @@ int main(int argc, char *argv[])
> }
>
> if (ret < 0)
> - return -1;
> + return 1;
>
> ret = lxc_wait_for_pid_status(pid);
> if (ret < 0)
> - return -1;
> + return 1;
>
> if (WIFEXITED(ret))
> return WEXITSTATUS(ret);
>
> - return -1;
> + return 1;
> }
> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
> index a3cd2b7..f150c43 100644
> --- a/src/lxc/lxc_cgroup.c
> +++ b/src/lxc/lxc_cgroup.c
> @@ -68,32 +68,32 @@ int main(int argc, char *argv[])
> struct lxc_container *c;
>
> if (lxc_arguments_parse(&my_args, argc, argv))
> - return -1;
> + return 1;
>
> if (!my_args.log_file)
> my_args.log_file = "none";
>
> if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
> my_args.progname, my_args.quiet, my_args.lxcpath[0]))
> - return -1;
> + return 1;
> lxc_log_options_no_override();
>
> state_object = my_args.argv[0];
>
> c = lxc_container_new(my_args.name, my_args.lxcpath[0]);
> if (!c)
> - return -1;
> + return 1;
>
> if (!c->may_control(c)) {
> ERROR("Insufficent privileges to control %s:%s", my_args.lxcpath[0], my_args.name);
> lxc_container_put(c);
> - return -1;
> + return 1;
> }
>
> if (!c->is_running(c)) {
> ERROR("'%s:%s' is not running", my_args.lxcpath[0], my_args.name);
> lxc_container_put(c);
> - return -1;
> + return 1;
> }
>
> if ((my_args.argc) > 1) {
> @@ -102,7 +102,7 @@ int main(int argc, char *argv[])
> ERROR("failed to assign '%s' value to '%s' for '%s'",
> value, state_object, my_args.name);
> lxc_container_put(c);
> - return -1;
> + return 1;
> }
> } else {
> int len = 4096;
> @@ -112,7 +112,7 @@ int main(int argc, char *argv[])
> ERROR("failed to retrieve value of '%s' for '%s:%s'",
> state_object, my_args.lxcpath[0], my_args.name);
> lxc_container_put(c);
> - return -1;
> + return 1;
> }
> printf("%*s", ret, buffer);
> }
> diff --git a/src/lxc/lxc_clone.c b/src/lxc/lxc_clone.c
> index 05579b8..df1b5ef 100644
> --- a/src/lxc/lxc_clone.c
> +++ b/src/lxc/lxc_clone.c
> @@ -176,7 +176,7 @@ int main(int argc, char *argv[])
> if (!c1->may_control(c1)) {
> fprintf(stderr, "Insufficent privileges to control %s\n", orig);
> lxc_container_put(c1);
> - return -1;
> + return 1;
> }
>
> if (!c1->is_defined(c1)) {
> diff --git a/src/lxc/lxc_config.c b/src/lxc/lxc_config.c
> index dc8827c..0658beb 100644
> --- a/src/lxc/lxc_config.c
> +++ b/src/lxc/lxc_config.c
> @@ -70,5 +70,5 @@ int main(int argc, char *argv[])
> }
> }
> printf("Unknown configuration item: %s\n", argv[1]);
> - exit(-1);
> + exit(1);
> }
> diff --git a/src/lxc/lxc_console.c b/src/lxc/lxc_console.c
> index 0fd08e8..b22322b 100644
> --- a/src/lxc/lxc_console.c
> +++ b/src/lxc/lxc_console.c
> @@ -115,7 +115,7 @@ int main(int argc, char *argv[])
> if (!c->may_control(c)) {
> fprintf(stderr, "Insufficent privileges to control %s\n", my_args.name);
> lxc_container_put(c);
> - return -1;
> + exit(EXIT_FAILURE);
> }
>
> if (!c->is_running(c)) {
> diff --git a/src/lxc/lxc_destroy.c b/src/lxc/lxc_destroy.c
> index d8bca34..955fc23 100644
> --- a/src/lxc/lxc_destroy.c
> +++ b/src/lxc/lxc_destroy.c
> @@ -83,7 +83,7 @@ int main(int argc, char *argv[])
> if (!c->may_control(c)) {
> fprintf(stderr, "Insufficent privileges to control %s\n", my_args.name);
> lxc_container_put(c);
> - return -1;
> + exit(1);
> }
>
> if (!c->is_defined(c)) {
> @@ -108,5 +108,5 @@ int main(int argc, char *argv[])
> }
>
> lxc_container_put(c);
> - return 0;
> + exit(0);
> }
> diff --git a/src/lxc/lxc_execute.c b/src/lxc/lxc_execute.c
> index 2ba4aeb..df25df3 100644
> --- a/src/lxc/lxc_execute.c
> +++ b/src/lxc/lxc_execute.c
> @@ -91,18 +91,19 @@ int main(int argc, char *argv[])
> {
> char *rcfile;
> struct lxc_conf *conf;
> + int ret;
>
> lxc_list_init(&defines);
>
> if (lxc_caps_init())
> - return -1;
> + return 1;
>
> if (lxc_arguments_parse(&my_args, argc, argv))
> - return -1;
> + return 1;
>
> if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
> my_args.progname, my_args.quiet, my_args.lxcpath[0]))
> - return -1;
> + return 1;
> lxc_log_options_no_override();
>
> /* rcfile is specified in the cli option */
> @@ -114,7 +115,7 @@ int main(int argc, char *argv[])
> rc = asprintf(&rcfile, "%s/%s/config", my_args.lxcpath[0], my_args.name);
> if (rc == -1) {
> SYSERROR("failed to allocate memory");
> - return -1;
> + return 1;
> }
>
> /* container configuration does not exist */
> @@ -127,16 +128,19 @@ int main(int argc, char *argv[])
> conf = lxc_conf_init();
> if (!conf) {
> ERROR("failed to initialize configuration");
> - return -1;
> + return 1;
> }
>
> if (rcfile && lxc_config_read(rcfile, conf)) {
> ERROR("failed to read configuration file");
> - return -1;
> + return 1;
> }
>
> if (lxc_config_define_load(&defines, conf))
> - return -1;
> + return 1;
>
> - return lxc_execute(my_args.name, my_args.argv, my_args.quiet, conf, my_args.lxcpath[0]);
> + ret = lxc_execute(my_args.name, my_args.argv, my_args.quiet, conf, my_args.lxcpath[0]);
> + if (ret < 0)
> + return 1;
> + return ret;
> }
> diff --git a/src/lxc/lxc_freeze.c b/src/lxc/lxc_freeze.c
> index fa64963..0744c82 100644
> --- a/src/lxc/lxc_freeze.c
> +++ b/src/lxc/lxc_freeze.c
> @@ -77,7 +77,7 @@ int main(int argc, char *argv[])
> if (!c->may_control(c)) {
> ERROR("Insufficent privileges to control %s:%s", my_args.lxcpath[0], my_args.name);
> lxc_container_put(c);
> - return -1;
> + exit(1);
> }
>
> if (!c->freeze(c)) {
> @@ -88,5 +88,5 @@ int main(int argc, char *argv[])
>
> lxc_container_put(c);
>
> - return 0;
> + exit(0);
> }
> diff --git a/src/lxc/lxc_init.c b/src/lxc/lxc_init.c
> index 91ed08d..b5596a0 100644
> --- a/src/lxc/lxc_init.c
> +++ b/src/lxc/lxc_init.c
> @@ -260,5 +260,7 @@ int main(int argc, char *argv[])
> }
> }
> out:
> - return err;
> + if (err < 0)
> + exit(EXIT_FAILURE);
> + exit(err);
> }
> diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
> index 7e2299a..85993fc 100644
> --- a/src/lxc/lxc_monitor.c
> +++ b/src/lxc/lxc_monitor.c
> @@ -78,14 +78,14 @@ int main(int argc, char *argv[])
> int len, rc, i, nfds = -1;
>
> if (lxc_arguments_parse(&my_args, argc, argv))
> - return -1;
> + return 1;
>
> if (!my_args.log_file)
> my_args.log_file = "none";
>
> if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
> my_args.progname, my_args.quiet, my_args.lxcpath[0]))
> - return -1;
> + return 1;
> lxc_log_options_no_override();
>
> if (quit_monitord) {
> @@ -114,19 +114,19 @@ int main(int argc, char *argv[])
> regexp = malloc(len + 3);
> if (!regexp) {
> ERROR("failed to allocate memory");
> - return -1;
> + return 1;
> }
> rc = snprintf(regexp, len, "^%s$", my_args.name);
> if (rc < 0 || rc >= len) {
> ERROR("Name too long");
> free(regexp);
> - return -1;
> + return 1;
> }
>
> if (regcomp(&preg, regexp, REG_NOSUB|REG_EXTENDED)) {
> ERROR("failed to compile the regex '%s'", my_args.name);
> free(regexp);
> - return -1;
> + return 1;
> }
> free(regexp);
>
> @@ -144,7 +144,7 @@ int main(int argc, char *argv[])
> fd = lxc_monitor_open(my_args.lxcpath[i]);
> if (fd < 0) {
> regfree(&preg);
> - return -1;
> + return 1;
> }
> FD_SET(fd, &rfds);
> if (fd > nfds)
> @@ -160,7 +160,7 @@ int main(int argc, char *argv[])
>
> if (lxc_monitor_read_fdset(&rfds, nfds, &msg, -1) < 0) {
> regfree(&preg);
> - return -1;
> + return 1;
> }
>
> msg.name[sizeof(msg.name)-1] = '\0';
> diff --git a/src/lxc/lxc_monitord.c b/src/lxc/lxc_monitord.c
> index 6328bf8..c317cbe 100644
> --- a/src/lxc/lxc_monitord.c
> +++ b/src/lxc/lxc_monitord.c
> @@ -378,7 +378,7 @@ int main(int argc, char *argv[])
> sigdelset(&mask, SIGTERM) ||
> sigprocmask(SIG_BLOCK, &mask, NULL)) {
> SYSERROR("failed to set signal mask");
> - return -1;
> + return 1;
> }
>
> signal(SIGILL, lxc_monitord_sig_handler);
> @@ -428,5 +428,7 @@ int main(int argc, char *argv[])
> ret = EXIT_SUCCESS;
> NOTICE("monitor exiting");
> out:
> - return ret;
> + if (ret == 0)
> + return 0;
> + return 1;
> }
> diff --git a/src/lxc/lxc_snapshot.c b/src/lxc/lxc_snapshot.c
> index eba9c30..1f8fdaf 100644
> --- a/src/lxc/lxc_snapshot.c
> +++ b/src/lxc/lxc_snapshot.c
> @@ -179,7 +179,7 @@ int main(int argc, char *argv[])
>
> if (my_args.argc > 1) {
> ERROR("Too many arguments");
> - return -1;
> + exit(1);
> }
> if (my_args.argc == 1)
> newname = my_args.argv[0];
> @@ -205,7 +205,7 @@ int main(int argc, char *argv[])
> if (!c->may_control(c)) {
> fprintf(stderr, "Insufficent privileges to control %s\n", my_args.name);
> lxc_container_put(c);
> - return -1;
> + exit(1);
> }
>
> switch(action) {
> @@ -225,5 +225,7 @@ int main(int argc, char *argv[])
>
> lxc_container_put(c);
>
> - exit(ret);
> + if (ret == 0)
> + exit(EXIT_SUCCESS);
> + exit(EXIT_FAILURE);
> }
> diff --git a/src/lxc/lxc_stop.c b/src/lxc/lxc_stop.c
> index 78a1da2..cb2fee0 100644
> --- a/src/lxc/lxc_stop.c
> +++ b/src/lxc/lxc_stop.c
> @@ -236,5 +236,7 @@ int main(int argc, char *argv[])
>
> out:
> lxc_container_put(c);
> + if (ret < 0)
> + return 1;
> return ret;
> }
> diff --git a/src/lxc/lxc_unfreeze.c b/src/lxc/lxc_unfreeze.c
> index 7371980..6a40ed7 100644
> --- a/src/lxc/lxc_unfreeze.c
> +++ b/src/lxc/lxc_unfreeze.c
> @@ -75,7 +75,7 @@ int main(int argc, char *argv[])
> if (!c->may_control(c)) {
> ERROR("Insufficent privileges to control %s:%s", my_args.lxcpath[0], my_args.name);
> lxc_container_put(c);
> - return -1;
> + exit(1);
> }
>
> if (!c->unfreeze(c)) {
> @@ -86,5 +86,5 @@ int main(int argc, char *argv[])
>
> lxc_container_put(c);
>
> - return 0;
> + exit(0);
> }
> diff --git a/src/lxc/lxc_unshare.c b/src/lxc/lxc_unshare.c
> index a591d0a..fc6f9db 100644
> --- a/src/lxc/lxc_unshare.c
> +++ b/src/lxc/lxc_unshare.c
> @@ -211,7 +211,7 @@ int main(int argc, char *argv[])
>
> ret = lxc_caps_init();
> if (ret)
> - return ret;
> + return 1;
>
> ret = lxc_fill_namespace_flags(namespaces, &flags);
> if (ret)
> @@ -235,7 +235,7 @@ int main(int argc, char *argv[])
> pid = lxc_clone(do_start, &start_arg, flags);
> if (pid < 0) {
> ERROR("failed to clone");
> - return -1;
> + return 1;
> }
>
> if (my_iflist) {
> @@ -250,7 +250,7 @@ int main(int argc, char *argv[])
>
> if (waitpid(pid, &status, 0) < 0) {
> ERROR("failed to wait for '%d'", pid);
> - return -1;
> + return 1;
> }
>
> return lxc_error_set_and_log(pid, status);
> diff --git a/src/lxc/lxc_usernsexec.c b/src/lxc/lxc_usernsexec.c
> index f5b7fe0..732a74a 100644
> --- a/src/lxc/lxc_usernsexec.c
> +++ b/src/lxc/lxc_usernsexec.c
> @@ -364,7 +364,7 @@ int main(int argc, char *argv[])
>
> if ((ret = waitpid(pid, &status, __WALL)) < 0) {
> printf("waitpid() returns %d, errno %d\n", ret, errno);
> - exit(ret);
> + exit(1);
> }
>
> exit(WEXITSTATUS(status));
> diff --git a/src/lxc/lxc_wait.c b/src/lxc/lxc_wait.c
> index e1010e2..897d0ea 100644
> --- a/src/lxc/lxc_wait.c
> +++ b/src/lxc/lxc_wait.c
> @@ -84,29 +84,29 @@ int main(int argc, char *argv[])
> struct lxc_container *c;
>
> if (lxc_arguments_parse(&my_args, argc, argv))
> - return -1;
> + return 1;
>
> if (!my_args.log_file)
> my_args.log_file = "none";
>
> if (lxc_log_init(my_args.name, my_args.log_file, my_args.log_priority,
> my_args.progname, my_args.quiet, my_args.lxcpath[0]))
> - return -1;
> + return 1;
> lxc_log_options_no_override();
>
> c = lxc_container_new(my_args.name, my_args.lxcpath[0]);
> if (!c)
> - return -1;
> + return 1;
>
> if (!c->may_control(c)) {
> fprintf(stderr, "Insufficent privileges to control %s\n", c->name);
> lxc_container_put(c);
> - return -1;
> + return 1;
> }
>
> if (!c->wait(c, my_args.states, my_args.timeout)) {
> lxc_container_put(c);
> - return -1;
> + return 1;
> }
> return 0;
> }
> --
> 1.9.1
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140407/5c17ba12/attachment-0001.sig>
More information about the lxc-devel
mailing list