One small nit
On 12/12/2011 06:00 AM, Fabio M. Di Nitto wrote:
> From: "Fabio M. Di Nitto" <fdinitto@xxxxxxxxxx>
>
> don't leak memory, better error reporting and improve
> status output when there is no quorum configured
>
> Signed-off-by: Fabio M. Di Nitto <fdinitto@xxxxxxxxxx>
> ---
> :100644 100644 2fd2b16... 9ad540d... M tools/corosync-quorumtool.c
> tools/corosync-quorumtool.c | 57 ++++++++++++++++++++++++-------------------
> 1 files changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/tools/corosync-quorumtool.c b/tools/corosync-quorumtool.c
> index 2fd2b16..9ad540d 100644
> --- a/tools/corosync-quorumtool.c
> +++ b/tools/corosync-quorumtool.c
> @@ -83,13 +83,12 @@ static void show_usage(const char *name)
> /*
> * Caller should free the returned string
> */
> -static const char *get_quorum_type(void)
> +static int get_quorum_type(char *quorum_type, size_t quorum_type_len)
> {
> - const char *qtype = NULL;
> - int result;
> - char buf[255];
> - size_t namelen = sizeof(buf);
> + int err;
> hdb_handle_t quorum_handle;
> + char buf[256];
> + size_t namelen = 0;
> confdb_handle_t confdb_handle;
> confdb_callbacks_t callbacks = {
> .confdb_key_change_notify_fn = NULL,
> @@ -97,20 +96,24 @@ static const char *get_quorum_type(void)
> .confdb_object_delete_change_notify_fn = NULL
> };
>
> - if (confdb_initialize(&confdb_handle, &callbacks) != CS_OK) {
> + if ((!quorum_type) || (quorum_type_len <= 0) ||
> + (confdb_initialize(&confdb_handle, &callbacks) != CS_OK)) {
> errno = EINVAL;
> - return NULL;
> + return -1;
> }
> - result = confdb_object_find_start(confdb_handle, OBJECT_PARENT_HANDLE);
> - if (result != CS_OK)
> +
> + memset(quorum_type, 0, quorum_type_len);
> +
> + err = confdb_object_find_start(confdb_handle, OBJECT_PARENT_HANDLE);
> + if (err != CS_OK)
> goto out;
>
if should use {} ie:
if (err != CS_OK) {
goto out
}
Aso the tabbing looks off here.
Regards
-steve
> - result = confdb_object_find(confdb_handle, OBJECT_PARENT_HANDLE, (void *)"quorum", strlen("quorum"), &quorum_handle);
> - if (result != CS_OK)
> + err = confdb_object_find(confdb_handle, OBJECT_PARENT_HANDLE, (void *)"quorum", strlen("quorum"), &quorum_handle);
> + if (err != CS_OK)
> goto out;
>
> - result = confdb_key_get(confdb_handle, quorum_handle, (void *)"provider", strlen("provider"), buf, &namelen);
> - if (result != CS_OK)
> + err = confdb_key_get(confdb_handle, quorum_handle, (void *)"provider", strlen("provider"), buf, &namelen);
> + if (err != CS_OK)
> goto out;
>
> if (namelen >= sizeof(buf)) {
> @@ -118,12 +121,11 @@ static const char *get_quorum_type(void)
> }
> buf[namelen] = '\0';
>
> - /* If strdup returns NULL then we just assume no quorum provider ?? */
> - qtype = strdup(buf);
> + strncpy(quorum_type, buf, quorum_type_len - 1);
>
> out:
> confdb_finalize(confdb_handle);
> - return qtype;
> + return err;
> }
>
> /*
> @@ -132,18 +134,17 @@ out:
> */
> static int using_votequorum(void)
> {
> - const char *quorumtype = get_quorum_type();
> + char quorumtype[256];
> int using_voteq;
>
> - if (!quorumtype)
> - return 0;
> + if (get_quorum_type(quorumtype, sizeof(quorumtype)))
> + return -1;
>
> if (strcmp(quorumtype, "corosync_votequorum") == 0)
> using_voteq = 1;
> else
> using_voteq = 0;
>
> - free((void *)quorumtype);
> return using_voteq;
> }
>
> @@ -272,6 +273,7 @@ static int show_status(void)
> struct votequorum_info info;
> int is_quorate;
> int err;
> + char quorum_type[256];
>
> callbacks.quorum_notify_fn = quorum_notification_fn;
> err=quorum_initialize(&q_handle, &callbacks);
> @@ -311,13 +313,18 @@ quorum_err:
> if (err < 0)
> return err;
>
> + get_quorum_type(quorum_type, sizeof(quorum_type));
> +
> printf("Version: %s\n", VERSION);
> printf("Nodes: %d\n", g_view_list_entries);
> printf("Ring ID: %" PRIu64 "\n", g_ring_id);
> - printf("Quorum type: %s\n", get_quorum_type());
> + if (get_quorum_type(quorum_type, sizeof(quorum_type))) {
> + strncpy(quorum_type, "Not configured", sizeof(quorum_type) - 1);
> + }
> + printf("Quorum type: %s\n", quorum_type);
> printf("Quorate: %s\n", is_quorate?"Yes":"No");
>
> - if (!using_votequorum()) {
> + if (using_votequorum() != 0) {
> return is_quorate;
> }
>
> @@ -379,7 +386,7 @@ static int show_nodes(nodeid_format_t nodeid_format, name_format_t name_format)
> v_callbacks.votequorum_expectedvotes_notify_fn = NULL;
>
> using_vq = using_votequorum();
> - if (using_vq) {
> + if (using_vq > 0) {
> if ( (err=votequorum_initialize(&v_handle, &v_callbacks)) != CS_OK) {
> fprintf(stderr, "votequorum_initialize FAILED: %d, this is probably a configuration error\n", err);
> v_handle = 0;
> @@ -471,7 +478,7 @@ int main (int argc, char *argv[]) {
> command_opt = CMD_SHOWNODES;
> break;
> case 'e':
> - if (using_votequorum()) {
> + if (using_votequorum() > 0) {
> votes = strtol(optarg, &endptr, 0);
> if ((votes == 0 && endptr == optarg) || votes <= 0) {
> fprintf(stderr, "New expected votes value was not valid, try a positive number\n");
> @@ -492,7 +499,7 @@ int main (int argc, char *argv[]) {
> }
> break;
> case 'v':
> - if (using_votequorum()) {
> + if (using_votequorum() > 0) {
> votes = strtol(optarg, &endptr, 0);
> if ((votes == 0 && endptr == optarg) || votes < 0) {
> fprintf(stderr, "New votes value was not valid, try a positive number or zero\n");
_______________________________________________
discuss mailing list
discuss@xxxxxxxxxxxx
http://lists.corosync.org/mailman/listinfo/discuss
