I will add my 2 cents, see comments inline
On 09/26/2011 02:58 PM, Russell Bryant wrote:
A couple of very minor comments inline ...
ACK. Nice work!
On 09/26/2011 08:26 AM, Radek Novacek wrote:
> * add errors.h header file that contains result codes used by matahari
> functions and mh_result_to_str function for converting result code to
> string.
> * mh_sysconfig_run_* methods now return different error code for
> different type of errors.
> ---
> src/host/host-qmf.cpp | 5 +-
> src/include/matahari/agent.h | 5 +-
> src/include/matahari/errors.h | 51 +++++++++++++++++++++
> src/include/matahari/sysconfig.h | 17 ++++---
> src/lib/sysconfig.c | 4 +-
> src/lib/sysconfig_linux.c | 90 +++++++++++++++++++++----------------
> src/lib/sysconfig_private.h | 4 +-
> src/lib/sysconfig_windows.c | 17 ++++---
> src/lib/utilities.c | 21 +++++++++
> src/network/network-qmf.cpp | 3 +-
> src/service/service-qmf.cpp | 13 +++---
> src/sysconfig/sysconfig-qmf.cpp | 43 +++++++-----------
> 12 files changed, 178 insertions(+), 95 deletions(-)
> create mode 100644 src/include/matahari/errors.h
>
> diff --git a/src/host/host-qmf.cpp b/src/host/host-qmf.cpp
> index cac7d43..618cf9c 100644
> --- a/src/host/host-qmf.cpp
> +++ b/src/host/host-qmf.cpp
> @@ -34,6 +34,7 @@ extern "C" {
> #include<sigar.h>
> #include "matahari/host.h"
> #include "matahari/logging.h"
> +#include "matahari/errors.h"
> }
>
> class HostAgent : public MatahariAgent
> @@ -136,7 +137,7 @@ HostAgent::invoke(qmf::AgentSession session, qmf::AgentEvent
event,
> _instance.setProperty("uuid",
mh_host_get_uuid("Filesystem"));
>
> } else {
> - session.raiseException(event, "No UUID supplied");
> + session.raiseException(event, mh_result_to_str(MH_RES_INVALID_ARGS));
> goto bail;
> }
>
> @@ -154,7 +155,7 @@ HostAgent::invoke(qmf::AgentSession session, qmf::AgentEvent
event,
> }
>
> } else {
> - session.raiseException(event, MH_NOT_IMPLEMENTED);
> + session.raiseException(event, mh_result_to_str(MH_RES_NOT_IMPLEMENTED));
> goto bail;
> }
>
> diff --git a/src/include/matahari/agent.h b/src/include/matahari/agent.h
> index 4175809..6004814 100644
> --- a/src/include/matahari/agent.h
> +++ b/src/include/matahari/agent.h
> @@ -44,8 +44,9 @@ int required_argument = 1;
> #endif
> }
>
> -#define MH_NOT_IMPLEMENTED "Not implemented"
> -#define MH_INVALID_ARGS "Invalid Arguments"
> +// For backward compatibility
> +#define MH_NOT_IMPLEMENTED (mh_result_to_str(MH_RES_NOT_IMPLEMENTED))
> +#define MH_INVALID_ARGS (mh_result_to_str(MH_RES_INVALID_ARGS))
>
> #ifndef NS_MAXDNAME
> # define NS_MAXDNAME 1025
> diff --git a/src/include/matahari/errors.h b/src/include/matahari/errors.h
> new file mode 100644
> index 0000000..550978d
> --- /dev/null
> +++ b/src/include/matahari/errors.h
> @@ -0,0 +1,51 @@
> +/* errors.h - Copyright (C) 2011 Red Hat, Inc.
> + *
> + * Written by Radek Novacek<rnovacek(a)redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This software is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +/**
> + * \file
> + * \brief Error API
> + * \ingroup coreapi
> + */
> +
> +#ifndef __MH_ERRORS_H__
> +#define __MH_ERRORS_H__
> +
> +/**
> + * Global enumeration with result codes.
> + */
> +enum mh_result {
> + MH_RES_SUCCESS = 0,
> + MH_RES_NOT_IMPLEMENTED = -1,
> + MH_RES_INVALID_ARGS = -2,
> + MH_RES_DOWNLOAD_ERROR = -3,
> + MH_RES_BACKEND_ERROR = -4,
> + MH_RES_OTHER_ERROR = -5
> +};
I don't see any reason to have values going from 0 to -inf.
What about something like:
enum mh_result {
MH_RES_SUCCES,
MH_RES_NOT_IMPLEMENTED,
...
};
The reason follows down in getting associated string with reason
> +
> +/**
> + * Get string associated with result code.
> + *
> + * \param[in] res result code, see enum mh_result
> + *
> + * \return string describing the result
> + */
> +const char *
> +mh_result_to_str(enum mh_result res);
> +
> +#endif
> diff --git a/src/include/matahari/sysconfig.h b/src/include/matahari/sysconfig.h
> index 86998a1..f19f439 100644
> --- a/src/include/matahari/sysconfig.h
> +++ b/src/include/matahari/sysconfig.h
> @@ -29,6 +29,8 @@
> #include<stdint.h>
> #include<glib.h>
>
> +#include "matahari/errors.h"
> +
> /*! Supported FLAGS */
> #define MH_SYSCONFIG_FLAG_FORCE (1<< 0)
>
> @@ -53,17 +55,17 @@ typedef void (*mh_sysconfig_result_cb)(void *data, int res);
> * final result of the request.
> * \param[in] cb_data custom data to be passed to the result callback.
> *
> - * \retval 0 for success
> - * \retval -1 for failure
> + * \return See enum mh_result
> */
> -int
> +enum mh_result
> mh_sysconfig_run_uri(const char *uri, uint32_t flags, const char *scheme, const
char *key,
> mh_sysconfig_result_cb result_cb, void *cb_data);
>
> /**
> * Process a text blob
> *
> - * \param[in] string the text blob to process, accepts a range of data from XML to
Puppet classes
> + * \param[in] string the text blob to process, accepts a range of data from XML
> + * to Puppet classes or newline-separated Augeas commands.
> * \param[in] flags flags used
> * \param[in] scheme the type of configuration i.e. puppet
> * \param[in] key configuration key for keeping track of existing
> @@ -73,10 +75,9 @@ mh_sysconfig_run_uri(const char *uri, uint32_t flags, const char
*scheme, const
> * final result of the request.
> * \param[in] cb_data custom data to be passed to the result callback.
> *
> - * \retval 0 for success
> - * \retval -1 for failure
> - */
> -int
> + * \return See enum mh_result
> + */
> +enum mh_result
> mh_sysconfig_run_string(const char *string, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb,
> void *cb_data);
> diff --git a/src/lib/sysconfig.c b/src/lib/sysconfig.c
> index c4b7303..c32e498 100644
> --- a/src/lib/sysconfig.c
> +++ b/src/lib/sysconfig.c
> @@ -141,7 +141,7 @@ mh_sysconfig_is_configured(const char *key)
> return get_key(key);
> }
>
> -int
> +enum mh_result
> mh_sysconfig_run_uri(const char *uri, uint32_t flags, const char *scheme, const
char *key,
> mh_sysconfig_result_cb result_cb, void *cb_data)
> {
> @@ -153,7 +153,7 @@ mh_sysconfig_run_uri(const char *uri, uint32_t flags, const char
*scheme, const
> return sysconfig_os_run_uri(uri, flags, scheme, key, result_cb, cb_data);
> }
>
> -int
> +enum mh_result
> mh_sysconfig_run_string(const char *string, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb,
> void *cb_data)
> diff --git a/src/lib/sysconfig_linux.c b/src/lib/sysconfig_linux.c
> index 421fb66..32c08b2 100644
> --- a/src/lib/sysconfig_linux.c
> +++ b/src/lib/sysconfig_linux.c
> @@ -54,13 +54,13 @@ action_data_free(struct action_data *action_data)
> * \internal
> * \note This function is not thread-safe.
> */
> -static int
> +static enum mh_result
> sysconfig_os_download(const char *uri, FILE *fp)
> {
> CURL *curl;
> CURLcode curl_res;
> long response = 0;
> - int res = 0;
> + enum mh_result res = MH_RES_SUCCESS;
> static int curl_init = 0;
>
> if (!curl_init) {
> @@ -68,34 +68,34 @@ sysconfig_os_download(const char *uri, FILE *fp)
>
> if (curl_res != CURLE_OK) {
> mh_err("curl_global_init failed: %d", curl_res);
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> curl_init = 1;
> }
>
> if (!(curl = curl_easy_init())) {
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> curl_res = curl_easy_setopt(curl, CURLOPT_URL, uri);
> if (curl_res != CURLE_OK) {
> mh_warn("curl_easy_setopt of URI '%s' failed. (%d)",
uri, curl_res);
> - res = -1;
> + res = MH_RES_OTHER_ERROR;
> goto return_cleanup;
> }
>
> curl_res = curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp);
> if (curl_res != CURLE_OK) {
> mh_warn("curl_easy_setopt of WRITEDATA '%p' failed.
(%d)", fp, curl_res);
> - res = -1;
> + res = MH_RES_OTHER_ERROR;
> goto return_cleanup;
> }
>
> curl_res = curl_easy_perform(curl);
> if (curl_res != CURLE_OK) {
> mh_warn("curl request for URI '%s' failed. (%d)", uri,
curl_res);
> - res = -1;
> + res = MH_RES_DOWNLOAD_ERROR;
> goto return_cleanup;
> }
>
> @@ -103,12 +103,12 @@ sysconfig_os_download(const char *uri, FILE *fp)
> curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE,&response);
> if (curl_res != CURLE_OK) {
> mh_warn("curl_easy_getinfo for RESPONSE_CODE failed. (%d)",
curl_res);
> - res = -1;
> + res = MH_RES_DOWNLOAD_ERROR;
> goto return_cleanup;
> }
> if (response< 200 || response> 299) {
> mh_warn("curl request for URI '%s' got response
%ld", uri, response);
> - res = -1;
> + res = MH_RES_DOWNLOAD_ERROR;
> }
> }
>
> @@ -196,7 +196,7 @@ return_cleanup:
> return res;
> }
>
> -static int
> +static enum mh_result
> run_puppet(const char *uri, const char *data, const char *key,
> mh_sysconfig_result_cb result_cb, void *cb_data)
> {
> @@ -205,9 +205,10 @@ run_puppet(const char *uri, const char *data, const char *key,
> svc_action_t *action = NULL;
> struct action_data *action_data = NULL;
> int use_apply = 0;
> + enum mh_result res = MH_RES_SUCCESS;
>
> if (check_puppet(&use_apply)) {
> - return -1;
> + return MH_RES_BACKEND_ERROR;
> }
>
> if (uri) {
> @@ -218,20 +219,20 @@ run_puppet(const char *uri, const char *data, const char *key,
>
> fd = mkstemp(filename);
> if (fd< 0) {
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> fp = fdopen(fd, "w+b");
> if (fp == NULL) {
> close(fd);
> unlink(filename);
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> - if ((sysconfig_os_download(uri, fp)) != 0) {
> + if ((res = sysconfig_os_download(uri, fp)) != 0) {
> fclose(fp);
> unlink(filename);
> - return -1;
> + return res;
> }
>
> fclose(fp);
> @@ -239,7 +240,7 @@ run_puppet(const char *uri, const char *data, const char *key,
> snprintf(filename, sizeof(filename), "puppet_conf_%u",
g_random_int());
> g_file_set_contents(filename, data, strlen(data), NULL);
> } else {
> - return -1;
> + return MH_RES_INVALID_ARGS;
> }
>
> if (use_apply) {
> @@ -252,6 +253,7 @@ run_puppet(const char *uri, const char *data, const char *key,
> }
>
> if (!(action = mh_services_action_create_generic("puppet", args)))
{
> + res = MH_RES_BACKEND_ERROR;
> goto return_failure;
> }
>
> @@ -267,10 +269,11 @@ run_puppet(const char *uri, const char *data, const char *key,
> mh_info("Running puppet %s%s", use_apply ? "apply " :
"", filename);
>
> if (services_action_async(action, action_cb) == FALSE) {
> + res = MH_RES_BACKEND_ERROR;
> goto return_failure;
> }
>
> - return 0;
> + return MH_RES_SUCCESS;
>
> return_failure:
> if (action) {
> @@ -289,10 +292,10 @@ return_failure:
>
> unlink(filename);
>
> - return -1;
> + return res;
> }
>
> -static int
> +static enum mh_result
> run_augeas(const char *uri, const char *data, const char *key,
> mh_sysconfig_result_cb result_cb, void *cb_data)
> {
> @@ -304,6 +307,7 @@ run_augeas(const char *uri, const char *data, const char *key,
> augeas *aug;
> int result;
> char *value = NULL;
> + enum mh_result res = MH_RES_SUCCESS;
>
> if (uri) {
> snprintf(filename, sizeof(filename), "%s",
"augeas_conf_XXXXXX");
> @@ -311,7 +315,7 @@ run_augeas(const char *uri, const char *data, const char *key,
> fd = mkstemp(filename);
> if (fd< 0) {
> mh_err("Unable to create temporary file");
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> fp = fdopen(fd, "w+b");
> @@ -319,14 +323,14 @@ run_augeas(const char *uri, const char *data, const char *key,
> close(fd);
> unlink(filename);
> mh_err("Unable to open temporary file");
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> - if ((sysconfig_os_download(uri, fp)) != 0) {
> + if ((res = sysconfig_os_download(uri, fp)) != 0) {
> fclose(fp);
> unlink(filename);
> mh_err("Unable to download file from uri %s", uri);
> - return -1;
> + return res;
> }
>
> fclose(fp);
> @@ -334,7 +338,7 @@ run_augeas(const char *uri, const char *data, const char *key,
> if (!g_file_get_contents(filename,&text, NULL,&err)) {
> mh_err("Unable to read downloaded file: %s", err ?
err->message : "Unknown error");
> unlink(filename);
> - return -1;
> + return MH_RES_DOWNLOAD_ERROR;
> }
>
> unlink(filename);
> @@ -343,7 +347,7 @@ run_augeas(const char *uri, const char *data, const char *key,
> text = g_strdup(data);
> } else {
> mh_err("No uri/data provided for augeas");
> - return -1;
> + return MH_RES_INVALID_ARGS;
> }
>
> snprintf(filename, sizeof(filename), "%s",
"augeas_conf_XXXXXX");
> @@ -351,7 +355,7 @@ run_augeas(const char *uri, const char *data, const char *key,
> fd = mkstemp(filename);
> if (fd< 0) {
> mh_err("Unable to create temporary file");
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> fp = fdopen(fd, "w+b");
> @@ -359,10 +363,18 @@ run_augeas(const char *uri, const char *data, const char *key,
> close(fd);
> unlink(filename);
> mh_err("Unable to open temporary file");
> - return -1;
> + return MH_RES_OTHER_ERROR;
> }
>
> aug = aug_init("", "", AUG_SAVE_BACKUP);
> + if (!aug) {
> + g_free(text);
> + fclose(fp);
> + unlink(filename);
> + mh_err("Unable to initialize augeas");
> + return MH_RES_BACKEND_ERROR;
> + }
> +
> result = aug_srun(aug, fp, text);
>
> mh_info("run_augeas for key \"%s\" exited with status %d and
data \"%s\"", key, result, text);
> @@ -375,13 +387,13 @@ run_augeas(const char *uri, const char *data, const char *key,
> if (result< 0) {
> unlink(filename);
> mh_err("Augeas command failed");
> - return -1;
> + return MH_RES_BACKEND_ERROR;
> }
>
> if (!g_file_get_contents(filename,&value, NULL,&err)) {
> mh_err("Unable to read augeas results: %s", err->message);
> unlink(filename);
> - return -1;
> + return MH_RES_BACKEND_ERROR;
> }
>
> mh_sysconfig_set_configured(key, value);
> @@ -405,18 +417,18 @@ sysconfig_os_query_augeas(const char *query)
> return data;
> }
>
> -int
> +enum mh_result
> sysconfig_os_run_uri(const char *uri, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb, void *cb_data)
> {
> - int rc = 0;
> + enum mh_result rc = MH_RES_SUCCESS;
>
> if (mh_sysconfig_is_configured(key)&& !(flags&
MH_SYSCONFIG_FLAG_FORCE)) {
> /*
> * Already configured and not being forced. Report success now.
> */
> - result_cb(cb_data, 0);
> - return 0;
> + result_cb(cb_data, rc);
> + return rc;
> }
>
> if (strcasecmp(scheme, "puppet") == 0) {
> @@ -424,24 +436,24 @@ sysconfig_os_run_uri(const char *uri, uint32_t flags, const
char *scheme,
> } else if (strcasecmp(scheme, "augeas") == 0) {
> rc = run_augeas(uri, NULL, key, result_cb, cb_data);
> } else {
> - rc = -1;
> + rc = MH_RES_INVALID_ARGS;
> }
>
> return rc;
> }
>
> -int
> +enum mh_result
> sysconfig_os_run_string(const char *string, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb, void *cb_data)
> {
> - int rc = 0;
> + enum mh_result rc = MH_RES_SUCCESS;
>
> if (mh_sysconfig_is_configured(key)&& !(flags&
MH_SYSCONFIG_FLAG_FORCE)) {
> /*
> * Already configured and not being forced. Report success now.
> */
> - result_cb(cb_data, 0);
> - return 0;
> + result_cb(cb_data, rc);
> + return rc;
> }
>
> if (!strcasecmp(scheme, "puppet")) {
> @@ -449,7 +461,7 @@ sysconfig_os_run_string(const char *string, uint32_t flags, const
char *scheme,
> } else if (strcasecmp(scheme, "augeas") == 0) {
> rc = run_augeas(NULL, string, key, result_cb, cb_data);
> } else {
> - rc = -1;
> + rc = MH_RES_INVALID_ARGS;
> }
>
> return rc;
> diff --git a/src/lib/sysconfig_private.h b/src/lib/sysconfig_private.h
> index 260a459..fa99e19 100644
> --- a/src/lib/sysconfig_private.h
> +++ b/src/lib/sysconfig_private.h
> @@ -19,12 +19,12 @@
> #ifndef __MH_SYSCONFIG_PRIVATE_H_
> #define __MH_SYSCONFIG_PRIVATE_H_
>
> -int
> +enum mh_result
> sysconfig_os_run_uri(const char *uri, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb,
> void *cb_data);
>
> -int
> +enum mh_result
> sysconfig_os_run_string(const char *string, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb,
> void *cb_data);
> diff --git a/src/lib/sysconfig_windows.c b/src/lib/sysconfig_windows.c
> index 00b5dce..4e650aa 100644
> --- a/src/lib/sysconfig_windows.c
> +++ b/src/lib/sysconfig_windows.c
> @@ -28,7 +28,7 @@
>
> MH_TRACE_INIT_DATA(mh_sysconfig);
>
> -static int
> +static enum mh_result
> run_regedit(char *registry_file)
> {
> gchar *cmd[] = {
> @@ -46,20 +46,23 @@ run_regedit(char *registry_file)
>
> if (ret == FALSE) {
> g_error_free(error);
> - return -1;
> + return MH_RES_BACKEND_ERROR;
> }
>
> - return res;
> + if (WIFEXITED(res))
> + return MH_RES_SUCCESS;
> + else
> + return MH_RES_BACKEND_ERROR;
> }
>
> -int
> +enum mh_result
> sysconfig_os_run_uri(const char *uri, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb, void *cb_data)
> {
> - return -1;
> + return MH_RES_NOT_IMPLEMENTED;
> }
>
> -int
> +enum mh_result
> sysconfig_os_run_string(const char *string, uint32_t flags, const char *scheme,
> const char *key, mh_sysconfig_result_cb result_cb, void *cb_data)
> {
> @@ -79,7 +82,7 @@ sysconfig_os_run_string(const char *string, uint32_t flags, const
char *scheme,
> return res;
> }
>
> - return -1;
> + return MH_RES_INVALID_ARGS;
> }
>
> char *
> diff --git a/src/lib/utilities.c b/src/lib/utilities.c
> index 4fe6bee..187d0bb 100644
> --- a/src/lib/utilities.c
> +++ b/src/lib/utilities.c
> @@ -29,6 +29,7 @@
>
> #include "matahari/logging.h"
> #include "matahari/utilities.h"
> +#include "matahari/errors.h"
> #include "utilities_private.h"
>
> MH_TRACE_INIT_DATA(mh_core);
> @@ -522,3 +523,23 @@ mh_string_copy(char *dst, const char *src, size_t dst_len)
>
> return ret;
> }
The reason is here. We can use global defined const char* array of
strings:
const char* MH_RES_MSG[] =
{
"Success",
"Not Implemented",
...
}
If the definitions of MH_RES_MSG and mh_result will be one before other,
you can easily see which one is in relation to other. mh_result_to_str
will not be needed.
> +
> +const char *
> +mh_result_to_str(enum mh_result res)
> +{
> + switch (res) {
> + case MH_RES_SUCCESS:
> + return "Success";
> + case MH_RES_NOT_IMPLEMENTED:
> + return "Not implemented";
> + case MH_RES_INVALID_ARGS:
> + return "Invalid Arguments";
> + case MH_RES_DOWNLOAD_ERROR:
> + return "Unable to download file";
> + case MH_RES_BACKEND_ERROR:
> + return "Error in backend";
> + case MH_RES_OTHER_ERROR:
> + return "Unknown error";
Hmmm ... maybe "General error"? Unknown makes it sound like it's a bug
if this string gets returned.
> + }
> + return "FIXME: result code don't have message assigned!";
s/don't/doesn't/
> +}
> diff --git a/src/network/network-qmf.cpp b/src/network/network-qmf.cpp
> index 2f53c1c..a6271c2 100644
> --- a/src/network/network-qmf.cpp
> +++ b/src/network/network-qmf.cpp
> @@ -32,6 +32,7 @@ extern "C" {
> #include "matahari/logging.h"
> #include "matahari/network.h"
> #include "matahari/host.h"
> +#include "matahari/errors.h"
> };
>
> class NetAgent : public MatahariAgent
> @@ -150,7 +151,7 @@ NetAgent::invoke(qmf::AgentSession session, qmf::AgentEvent
event,
> args["iface"].asString().c_str(),
> buf, sizeof(buf)));
> } else {
> - session.raiseException(event, MH_NOT_IMPLEMENTED);
> + session.raiseException(event, mh_result_to_str(MH_RES_NOT_IMPLEMENTED));
> goto bail;
> }
>
> diff --git a/src/service/service-qmf.cpp b/src/service/service-qmf.cpp
> index 529caaa..63f5db2 100644
> --- a/src/service/service-qmf.cpp
> +++ b/src/service/service-qmf.cpp
> @@ -33,6 +33,7 @@ extern "C" {
> extern "C" {
> #include "matahari/logging.h"
> #include "matahari/services.h"
> +#include "matahari/errors.h"
> }
>
> #include<iostream>
> @@ -304,7 +305,7 @@ SrvAgent::invoke_services(qmf::AgentSession session,
qmf::AgentEvent event,
> default_timeout_ms);
>
> if (!op) {
> - session.raiseException(event, MH_INVALID_ARGS);
> + session.raiseException(event, mh_result_to_str(MH_RES_INVALID_ARGS));
> } else {
> action_async(SRV_SERVICES, session, event, op, true);
> }
> @@ -318,7 +319,7 @@ SrvAgent::invoke_services(qmf::AgentSession session,
qmf::AgentEvent event,
> args["name"].asString().c_str(), methodName.c_str(), 0,
args["timeout"].asInt32());
>
> if (!op) {
> - session.raiseException(event, MH_INVALID_ARGS);
> + session.raiseException(event, mh_result_to_str(MH_RES_INVALID_ARGS));
> } else {
> action_async(SRV_SERVICES, session, event, op, true);
> }
> @@ -326,7 +327,7 @@ SrvAgent::invoke_services(qmf::AgentSession session,
qmf::AgentEvent event,
> return TRUE;
>
> } else {
> - session.raiseException(event, MH_NOT_IMPLEMENTED);
> + session.raiseException(event, mh_result_to_str(MH_RES_NOT_IMPLEMENTED));
> return TRUE;
> }
>
> @@ -411,7 +412,7 @@ SrvAgent::invoke_resources(qmf::AgentSession session,
qmf::AgentEvent event,
>
> if(valid_standard == false) {
> mh_err("%s is not a known resource standard",
args["standard"].asString().c_str());
> - session.raiseException(event, MH_NOT_IMPLEMENTED);
> + session.raiseException(event,
mh_result_to_str(MH_RES_NOT_IMPLEMENTED));
> return TRUE;
> }
>
> @@ -441,7 +442,7 @@ SrvAgent::invoke_resources(qmf::AgentSession session,
qmf::AgentEvent event,
> interval, timeout, params);
>
> if (!op) {
> - session.raiseException(event, MH_INVALID_ARGS);
> + session.raiseException(event, mh_result_to_str(MH_RES_INVALID_ARGS));
> return TRUE;
> }
>
> @@ -459,7 +460,7 @@ SrvAgent::invoke_resources(qmf::AgentSession session,
qmf::AgentEvent event,
> args["interval"].asInt32());
>
> } else {
> - session.raiseException(event, MH_NOT_IMPLEMENTED);
> + session.raiseException(event, mh_result_to_str(MH_RES_NOT_IMPLEMENTED));
> return TRUE;
> }
>
> diff --git a/src/sysconfig/sysconfig-qmf.cpp b/src/sysconfig/sysconfig-qmf.cpp
> index 840c89e..bf81414 100644
> --- a/src/sysconfig/sysconfig-qmf.cpp
> +++ b/src/sysconfig/sysconfig-qmf.cpp
> @@ -120,37 +120,28 @@ ConfigAgent::invoke(qmf::AgentSession session, qmf::AgentEvent
event, gpointer u
>
> qpid::types::Variant::Map& args = event.getArguments();
>
> - if (methodName == "run_uri") {
> + if (methodName == "run_uri" || methodName == "run_string")
{
> AsyncCB *action_data = new AsyncCB(args["key"].asString(),
event, session);
> - int res;
> + mh_result res;
>
> - res = mh_sysconfig_run_uri(args["uri"].asString().c_str(),
> - args["flags"].asUint32(),
> - args["scheme"].asString().c_str(),
> - args["key"].asString().c_str(), AsyncCB::result_cb,
action_data);
> + if (methodName == "run_uri")
> + res = mh_sysconfig_run_uri(args["uri"].asString().c_str(),
> + args["flags"].asUint32(),
> + args["scheme"].asString().c_str(),
> + args["key"].asString().c_str(), AsyncCB::result_cb,
action_data);
> + else
> + res =
mh_sysconfig_run_string(args["text"].asString().c_str(),
> + args["flags"].asUint32(),
> + args["scheme"].asString().c_str(),
> + args["key"].asString().c_str(), AsyncCB::result_cb,
action_data);
>
> - if (res) {
> - session.raiseException(event, MH_INVALID_ARGS);
> - delete action_data;
> - goto bail;
> - } else {
> - async = true;
> - }
> - } else if (methodName == "run_string") {
> - AsyncCB *action_data = new AsyncCB(args["key"].asString(), event,
session);
> - int res;
>
> - res = mh_sysconfig_run_string(args["text"].asString().c_str(),
> - args["flags"].asUint32(),
> - args["scheme"].asString().c_str(),
> - args["key"].asString().c_str(), AsyncCB::result_cb,
action_data);
> -
> - if (res) {
> - session.raiseException(event, MH_INVALID_ARGS);
> + if (res == MH_RES_SUCCESS) {
> + async = true;
> + } else {
> + session.raiseException(event, mh_result_to_str(res));
> delete action_data;
> goto bail;
> - } else {
> - async = true;
> }
> } else if (methodName == "query") {
> char *data = NULL;
> @@ -163,7 +154,7 @@ ConfigAgent::invoke(qmf::AgentSession session, qmf::AgentEvent
event, gpointer u
> status =
mh_sysconfig_is_configured(args["key"].asString().c_str());
> event.addReturnArgument("status", status ? status :
"unknown");
> } else {
> - session.raiseException(event, MH_NOT_IMPLEMENTED);
> + session.raiseException(event, mh_result_to_str(MH_RES_NOT_IMPLEMENTED));
> goto bail;
> }
>