dict: dict_set_bin() should never free the pointer on error

dict_set_bin() is handling the pointer that it passed inconsistently.
Depending on the errors that can occur, the pointer passed to the dict
can be free'd, but there is no guarantee.

It is cleaner to have the caller free the pointer that allocated it and
dict_set_bin() returned an error. When dict_set_bin() returned success,
the given pointer will be free'd when dict_unref() calls data_destroy().

Many callers of dict_set_bin() already take care of free'ing the pointer
on error. The ones that did not, are corrected with this change too.

Change-Id: I39a4f7ebc0cae6d403baba99307d7ce408f25966
BUG: 1242280
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: http://review.gluster.org/11638
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: jiffin tony Thottan <jthottan@redhat.com>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Tested-by: NetBSD Build System <jenkins@build.gluster.org>

5 files changed, 9 insertions, 3 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-locks.c b/xlators/mgmt/glusterd/src/glusterd-locks.c
index e18bd239b89..146092db79b 100644
--- a/xlators/mgmt/glusterd/src/glusterd-locks.c
+++ b/xlators/mgmt/glusterd/src/glusterd-locks.c
@@ -590,8 +590,7 @@ glusterd_mgmt_v3_lock (const char *name, uuid_t uuid, uint32_t *op_errno,
                 gf_msg (this->name, GF_LOG_ERROR, 0,
                         GD_MSG_DICT_SET_FAILED,
                         "Unable to set lock owner in mgmt_v3 lock");
-                if (lock_obj)
-                        GF_FREE (lock_obj);
+                GF_FREE (lock_obj);
                 goto out;
         }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-mgmt.c b/xlators/mgmt/glusterd/src/glusterd-mgmt.c
index 326d8681495..2afa6966d1f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-mgmt.c
+++ b/xlators/mgmt/glusterd/src/glusterd-mgmt.c
@@ -1817,6 +1817,7 @@ glusterd_mgmt_v3_initiate_all_phases (rpcsvc_request_t *req, glusterd_op_t op,
                 gf_msg (this->name, GF_LOG_ERROR, 0,
                         GD_MSG_DICT_SET_FAILED,
                         "Failed to set originator_uuid.");
+                GF_FREE (originator_uuid);
                 goto out;
         }
 
@@ -2051,6 +2052,7 @@ glusterd_mgmt_v3_initiate_snap_phases (rpcsvc_request_t *req, glusterd_op_t op,
                 gf_msg (this->name, GF_LOG_ERROR, 0,
                         GD_MSG_DICT_SET_FAILED,
                         "Failed to set originator_uuid.");
+                GF_FREE (originator_uuid);
                 goto out;
         }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index 07a407bd2bb..68cffefdc94 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -5142,6 +5142,7 @@ glusterd_op_ac_stage_op (glusterd_op_sm_event_t *event, void *ctx)
                 gf_msg (this->name, GF_LOG_ERROR, 0,
                         GD_MSG_DICT_SET_FAILED,
                        "Failed to set transaction id.");
+                GF_FREE (txn_id);
                 goto out;
         }
 
@@ -5256,6 +5257,7 @@ glusterd_op_ac_commit_op (glusterd_op_sm_event_t *event, void *ctx)
                 gf_msg (this->name, GF_LOG_ERROR, 0,
                         GD_MSG_DICT_SET_FAILED,
                        "Failed to set transaction id.");
+                GF_FREE (txn_id);
                 goto out;
         }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
index 4ba7d8d51e9..70ee7364536 100644
--- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
+++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
@@ -868,6 +868,7 @@ glusterd_mgmt_v3_initiate_replace_brick_cmd_phases (rpcsvc_request_t *req,
                 gf_msg (this->name, GF_LOG_ERROR, 0,
                         GD_MSG_DICT_SET_FAILED,
                         "Failed to set originator_uuid.");
+                GF_FREE (originator_uuid);
                 goto out;
         }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 501e9de7169..ffc6160e280 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -7308,8 +7308,10 @@ glusterd_append_status_dicts (dict_t *dst, dict_t *src)
                 snprintf (sts_val_name, sizeof(sts_val_name), "status_value%d", i + dst_count);
 
                 ret = dict_set_bin (dst, sts_val_name, dst_sts_val, sizeof(gf_gsync_status_t));
-                if (ret)
+                if (ret) {
+                        GF_FREE (dst_sts_val);
                         goto out;
+                }
         }
 
         ret = dict_set_int32 (dst, "gsync-count", dst_count+src_count);