Hi,
On Tue, 2015-10-27 at 09:58 +0200, Mikko Tuumanen wrote:
> ---
> client/agent.c| 2 +-
> client/commands.c | 27 +--
> client/dbus_helpers.c | 5 +++--
> client/input.c| 6 --
> client/input.h| 2 +-
> 5 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/client/agent.c b/client/agent.c
> index d020889..b87d5bb 100644
> --- a/client/agent.c
> +++ b/client/agent.c
> @@ -235,7 +235,7 @@ static DBusMessage *agent_release(DBusConnection
> *connection,
> "VPNd\n");
>
> if (__connmanctl_is_interactive() == false)
> - __connmanctl_quit();
> + __connmanctl_quit(0);
>
> return dbus_message_new_method_return(message);
> }
> diff --git a/client/commands.c b/client/commands.c
> index 3bfdfd7..ee56d46 100644
> --- a/client/commands.c
> +++ b/client/commands.c
> @@ -153,14 +153,21 @@ static int enable_return(DBusMessageIter *iter, const
> char *error,
> else
> str = tech;
>
> - if (!error)
> + int ret;
int ret = 0; should be declared together with the other variables. As a
nitpick the convention has been that variables with values were declared
first, then uninitialized variables.
> + if (!error) {
> + ret = 0;
> fprintf(stdout, "Enabled %s\n", str);
> - else
> + } else {
> + if (!strncmp("Already ",error,8))
> + ret = -2;
> + else
> + ret = -1;
I don't think one can rely on the string here. It's a semi-descriptive
one that may be used for logging or other purposes, but not much else.
The code is looking for -EINPROGRESS in dbus_method_reply(), so "proper"
values from errno*.h shall be used. What if this just sets the return
value to -EOPNOTSUPP?
> fprintf(stderr, "Error %s: %s\n", str, error);
> + }
>
> g_free(user_data);
>
> - return 0;
> + return ret;
> }
>
> static int cmd_enable(char *args[], int num, struct connman_option *options)
> @@ -202,14 +209,22 @@ static int disable_return(DBusMessageIter *iter, const
> char *error,
> else
> str = tech;
>
> - if (!error)
> + int ret;
> + if (!error) {
> + ret = 0;
> fprintf(stdout, "Disabled %s\n", str);
> - else
> + } else {
> + if (!strncmp("Already ", error, 8))
> + ret = -2;
> + else
> + ret = -1;
> +
> fprintf(stderr, "Error %s: %s\n", str, error);
> + }
Same here.
> g_free(user_data);
>
> - return 0;
> + return ret;
> }
>
> static int cmd_disable(char *args[], int num, struct connman_option *options)
> diff --git a/client/dbus_helpers.c b/client/dbus_helpers.c
> index 9c4cfee..fe55abf 100644
> --- a/client/dbus_helpers.c
> +++ b/client/dbus_helpers.c
> @@ -141,6 +141,7 @@ static void dbus_method_reply(DBusPendingCall *call, void
> *user_data)
> int res = 0;
> DBusMessage *reply;
> DBusMessageIter iter;
> + int exit_value = 0;
>
> __connmanctl_save_rl();
>
> @@ -152,7 +153,7 @@ static void dbus_method_reply(DBusPendingCall *call, void
> *user_data)
> dbus_error_init();
> dbus_set_error_from_message(, reply);
>
> - callback->cb(NULL, err.message, callback->user_data);
> + exit_value = callback->cb(NULL, err.message,
> callback->user_data);
>
> dbus_error_free();
> goto end;
> @@ -164,7 +165,7 @@ static void dbus_method_reply(DBusPendingCall *call, void
> *user_data)
> end:
> __connmanctl_redraw_rl();
> if (__connmanctl_is_interactive() == false && res != -EINPROGRESS)
> - __connmanctl_quit();
> + __connmanctl_quit(exit_value);
>
> g_free(callback);
> dbus_message_unref(reply);
> diff --git a/client/input.c b/client/input.c
> index e59df8a..f310d15 100644
> --- a/client/input.c
> +++ b/client/input.c
> @@ -42,9 +42,11 @@ static bool interactive = false;
> static bool save_input;
> static char *saved_line;
> static int saved_point;
> +static int exit_status = 0;
>
> -void __connmanctl_quit(void)
> +void __connmanctl_quit(int status)
> {
> + exit_status = status;
> if (main_loop)
> g_main_loop_quit(main_loop);
> }
> @@ -264,7 +266,7 @@ int __connmanctl_input_init(int argc, char *argv[])
> main_loop = g_main_loop_new(NULL, FALSE);
> g_main_loop_run(main_loop);
>
> - err = 0;
> + err = exit_status;
> }
>
> if (interactive) {
> diff --git a/client/input.h b/client/input.h
> index c7256a4..2136d13 100644
> --- a/client/input.h
> +++ b/client/input.h
> @@ -29,7 +29,7 @@
> extern "C" {
> #endif
>
> -void __connmanctl_quit(void);
> +void __connmanctl_quit(int status);
> bool
