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>

4 files changed, 27 insertions, 5 deletions

diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index edacdde568d..a7633c92094 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -81,8 +81,10 @@ afr_selfheal_output_xattr (xlator_t *this, afr_transaction_type type,
 	raw[idx] = hton32 (output_dirty[subvol]);
 	ret = dict_set_bin (xattr, AFR_DIRTY, raw,
 			    sizeof(int) * AFR_NUM_CHANGE_LOGS);
-	if (ret)
+	if (ret) {
+                GF_FREE (raw);
 		goto err;
+        }
 
 	/* clear/set pending */
 	for (j = 0; j < priv->child_count; j++) {
@@ -95,8 +97,10 @@ afr_selfheal_output_xattr (xlator_t *this, afr_transaction_type type,
 
 		ret = dict_set_bin (xattr, priv->pending_key[j],
 				    raw, sizeof(int) * AFR_NUM_CHANGE_LOGS);
-		if (ret)
+		if (ret) {
+                        GF_FREE (raw);
 			goto err;
+                }
 	}
 
 	return xattr;
diff --git a/xlators/cluster/dht/src/dht-rename.c b/xlators/cluster/dht/src/dht-rename.c
index b6ed2ae90de..bde0ce83439 100644
--- a/xlators/cluster/dht/src/dht-rename.c
+++ b/xlators/cluster/dht/src/dht-rename.c
@@ -359,6 +359,7 @@ dht_rename_track_for_changelog (xlator_t *this, dict_t *xattr,
                         "Failed to set dictionary value: key = %s,"
                         " path = %s", DHT_CHANGELOG_RENAME_OP_KEY,
                         oldloc->name);
+                GF_FREE (info);
         }
         return ret;
 }
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
index a7fcc9ceca5..ca0507bda44 100644
--- a/xlators/cluster/dht/src/dht-selfheal.c
+++ b/xlators/cluster/dht/src/dht-selfheal.c
@@ -680,6 +680,7 @@ dht_selfheal_dir_xattr_persubvol (call_frame_t *frame, loc_t *loc,
                         "Directory self heal xattr failed:"
                         "%s: (subvol %s) Failed to set xattr dictionary,"
                         " gfid = %s", loc->path, subvol->name, gfid);
+                GF_FREE (disk_layout);
                 goto err;
         }
         disk_layout = NULL;
@@ -2126,6 +2127,8 @@ dht_update_commit_hash_for_layout_resume (call_frame_t *frame, void *cookie,
                                 " dictionary,", local->loc.path,
                                 conf->local_subvols[i]->name);
 
+                        GF_FREE (disk_layout);
+
                         goto err;
                 }
                 disk_layout = NULL;
diff --git a/xlators/cluster/ec/src/ec-helpers.c b/xlators/cluster/ec/src/ec-helpers.c
index 372633df6be..aec0831ffc3 100644
--- a/xlators/cluster/ec/src/ec-helpers.c
+++ b/xlators/cluster/ec/src/ec-helpers.c
@@ -161,6 +161,7 @@ size_t ec_iov_copy_to(void * dst, struct iovec * vector, int32_t count,
 int32_t ec_dict_set_array(dict_t *dict, char *key, uint64_t value[],
                           int32_t size)
 {
+    int         ret = -1;
     uint64_t   *ptr = NULL;
     int32_t     vindex;
     if (value == NULL)
@@ -172,7 +173,10 @@ int32_t ec_dict_set_array(dict_t *dict, char *key, uint64_t value[],
     for (vindex = 0; vindex < size; vindex++) {
          ptr[vindex] = hton64(value[vindex]);
     }
-    return dict_set_bin(dict, key, ptr, sizeof(uint64_t) * size);
+    ret = dict_set_bin(dict, key, ptr, sizeof(uint64_t) * size);
+    if (ret)
+         GF_FREE (ptr);
+    return ret;
 }
 
 
@@ -214,6 +218,7 @@ int32_t ec_dict_del_array(dict_t *dict, char *key, uint64_t value[],
 
 int32_t ec_dict_set_number(dict_t * dict, char * key, uint64_t value)
 {
+    int        ret = -1;
     uint64_t * ptr;
 
     ptr = GF_MALLOC(sizeof(value), gf_common_mt_char);
@@ -224,7 +229,11 @@ int32_t ec_dict_set_number(dict_t * dict, char * key, uint64_t value)
 
     *ptr = hton64(value);
 
-    return dict_set_bin(dict, key, ptr, sizeof(value));
+    ret = dict_set_bin(dict, key, ptr, sizeof(value));
+    if (ret)
+        GF_FREE (ptr);
+
+    return ret;
 }
 
 int32_t ec_dict_del_number(dict_t * dict, char * key, uint64_t * value)
@@ -247,6 +256,7 @@ int32_t ec_dict_del_number(dict_t * dict, char * key, uint64_t * value)
 
 int32_t ec_dict_set_config(dict_t * dict, char * key, ec_config_t * config)
 {
+    int ret = -1;
     uint64_t * ptr, data;
 
     if (config->version > EC_CONFIG_VERSION)
@@ -274,7 +284,11 @@ int32_t ec_dict_set_config(dict_t * dict, char * key, ec_config_t * config)
 
     *ptr = hton64(data);
 
-    return dict_set_bin(dict, key, ptr, sizeof(uint64_t));
+    ret = dict_set_bin(dict, key, ptr, sizeof(uint64_t));
+    if (ret)
+        GF_FREE (ptr);
+
+    return ret;
 }
 
 int32_t ec_dict_del_config(dict_t * dict, char * key, ec_config_t * config)