RedisGraph · swilly22 · Feb 21, 2020 · Feb 20, 2020
diff --git a/src/arithmetic/aggregate.c b/src/arithmetic/aggregate.c
@@ -32,7 +32,7 @@ AggCtx *Agg_CloneCtx(AggCtx *ctx) {
 
 void AggCtx_Free(AggCtx *ctx) {
 	ctx->AggCtx_PrivateData_Free(ctx);
-	SIValue_Free(&ctx->result);
+	SIValue_Free(ctx->result);
 	rm_free(ctx);
 }
 

diff --git a/src/arithmetic/arithmetic_expression.c b/src/arithmetic/arithmetic_expression.c
@@ -253,7 +253,7 @@ static bool _AR_EXP_ValidateInvocation(AR_FuncDesc *fdesc, SIValue *argv, uint a
  * the result of toUpper() is allocated within this tree, and will leak if not freed here. */
 static inline void _AR_EXP_FreeResultsArray(SIValue *results, int count) {
 	for(int i = 0; i < count; i ++) {
-		SIValue_Free(&results[i]);
+		SIValue_Free(results[i]);
 	}
 }
 
@@ -601,7 +601,7 @@ void AR_EXP_Free(AR_ExpNode *root) {
 			AggCtx_Free(root->op.agg_func);
 		}
 	} else if(root->operand.type == AR_EXP_CONSTANT) {
-		SIValue_Free(&root->operand.constant);
+		SIValue_Free(root->operand.constant);
 	}
 	rm_free(root);
 }

diff --git a/src/datatypes/array.c b/src/datatypes/array.c
@@ -77,7 +77,7 @@ void SIArray_Free(SIValue siarray) {
 	uint arrayLen = SIArray_Length(siarray);
 	for(uint i = 0; i < arrayLen; i++) {
 		SIValue value = siarray.array[i];
-		SIValue_Free(&value);
+		SIValue_Free(value);
 	}
 	array_free(siarray.array);
 }
diff --git a/src/execution_plan/ops/op_procedure_call.c b/src/execution_plan/ops/op_procedure_call.c
@@ -123,7 +123,7 @@ static Record ProcCallConsume(OpBase *opBase) {
 
 		// Evaluate arguments, free args from previous call.
 		uint arg_count = array_len(op->args);
-		for(uint i = 0; i < arg_count; i++) SIValue_Free(op->args + i);
+		for(uint i = 0; i < arg_count; i++) SIValue_Free(op->args[i]);
 
 		array_clear(op->args);
 		for(uint i = 0; i < op->arg_count; i++) {
@@ -170,7 +170,7 @@ static void ProcCallFree(OpBase *ctx) {
 
 	if(op->args) {
 		uint arg_count = array_len(op->args);
-		for(uint i = 0; i < arg_count; i++) SIValue_Free(op->args + i);
+		for(uint i = 0; i < arg_count; i++) SIValue_Free(op->args[i]);
 		array_free(op->args);
 		op->args = NULL;
 	}
@@ -193,3 +193,4 @@ static void ProcCallFree(OpBase *ctx) {
 		op->yield_exps = NULL;
 	}
 }
+
diff --git a/src/execution_plan/ops/op_unwind.c b/src/execution_plan/ops/op_unwind.c
@@ -95,7 +95,7 @@ static Record UnwindConsume(OpBase *opBase) {
 		OpBase_DeleteRecord(op->currentRecord);
 		op->currentRecord = r;
 		// Free old list.
-		SIValue_Free(&op->list);
+		SIValue_Free(op->list);
 
 		// Reset index and set list.
 		op->listIdx = 0;
@@ -116,7 +116,7 @@ static OpResult UnwindReset(OpBase *ctx) {
 
 static void UnwindFree(OpBase *ctx) {
 	OpUnwind *op = (OpUnwind *)ctx;
-	SIValue_Free(&op->list);
+	SIValue_Free(op->list);
 	op->list = SI_NullVal();
 
 	if(op->exp) {

diff --git a/src/execution_plan/ops/op_update.c b/src/execution_plan/ops/op_update.c
@@ -116,7 +116,7 @@ static void _CommitUpdates(OpUpdate *op) {
 		} else {
 			_UpdateEdge(op, ctx);
 		}
-		SIValue_Free(&ctx->new_value);
+		SIValue_Free(ctx->new_value);
 	}
 
 	if(op->stats) op->stats->properties_set += op->pending_updates_count;

diff --git a/src/execution_plan/ops/shared/create_functions.c b/src/execution_plan/ops/shared/create_functions.c
@@ -175,7 +175,7 @@ void PendingPropertiesFree(PendingProperties *props) {
 	if(props == NULL) return;
 	// The 'keys' array belongs to the original PropertyMap, so shouldn't be freed here.
 	for(uint j = 0; j < props->property_count; j ++) {
-		SIValue_Free(&props->values[j]);
+		SIValue_Free(props->values[j]);
 	}
 	rm_free(props->values);
 	rm_free(props);

diff --git a/src/execution_plan/record.c b/src/execution_plan/record.c
@@ -277,7 +277,7 @@ void Record_FreeEntries(Record r) {
 	for(uint i = 0; i < length; i++) {
 		// Free any allocations held by this Record.
 		if(r->entries[i].type == REC_TYPE_SCALAR) {
-			SIValue_Free(&r->entries[i].value.s);
+			SIValue_Free(r->entries[i].value.s);
 		}
 	}
 }

diff --git a/src/filter_tree/filter_tree.c b/src/filter_tree/filter_tree.c
@@ -174,8 +174,8 @@ int _applyPredicateFilters(const FT_FilterNode *root, const Record r) {
 
 	int ret = _applyFilter(&lhs, &rhs, root->pred.op);
 
-	SIValue_Free(&lhs);
-	SIValue_Free(&rhs);
+	SIValue_Free(lhs);
+	SIValue_Free(rhs);
 
 	return ret;
 }

diff --git a/src/graph/entities/graph_entity.c b/src/graph/entities/graph_entity.c
@@ -26,7 +26,7 @@ static void _GraphEntity_RemoveProperty(const GraphEntity *e, Attribute_ID attr_
 	int prop_count = e->entity->prop_count;
 	for(int i = 0; i < prop_count; i++) {
 		if(attr_id == e->entity->properties[i].id) {
-			SIValue_Free(&(e->entity->properties[i].value));
+			SIValue_Free(e->entity->properties[i].value);
 			e->entity->prop_count--;
 
 			if(e->entity->prop_count == 0) {
@@ -87,7 +87,7 @@ void GraphEntity_SetProperty(const GraphEntity *e, Attribute_ID attr_id, SIValue
 
 	SIValue *prop = GraphEntity_GetProperty(e, attr_id);
 	assert(prop != PROPERTY_NOTFOUND);
-	SIValue_Free(prop);
+	SIValue_Free(*prop);
 	*prop = SI_CloneValue(value);
 }
 
@@ -206,7 +206,7 @@ void GraphEntity_ToString(const GraphEntity *e, char **buffer, size_t *bufferLen
 void FreeEntity(Entity *e) {
 	assert(e);
 	if(e->properties != NULL) {
-		for(int i = 0; i < e->prop_count; i++) SIValue_Free(&e->properties[i].value);
+		for(int i = 0; i < e->prop_count; i++) SIValue_Free(e->properties[i].value);
 		rm_free(e->properties);
 		e->properties = NULL;
 	}

diff --git a/src/grouping/group.c b/src/grouping/group.c
@@ -40,7 +40,7 @@ void FreeGroup(Group *g) {
 	if(g->r) Record_FreeEntries(g->r);  // Will be freed by Record owner.
 	if(g->keys) {
 		for(int i = 0; i < g->key_count; i ++) {
-			SIValue_Free(&g->keys[i]);
+			SIValue_Free(g->keys[i]);
 		}
 		rm_free(g->keys);
 	}

diff --git a/src/resultset/formatters/resultset_replycompact.c b/src/resultset/formatters/resultset_replycompact.c
@@ -214,12 +214,12 @@ static void _ResultSet_CompactReplyWithPath(RedisModuleCtx *ctx, GraphContext *g
 	RedisModule_ReplyWithArray(ctx, 2);
 	SIValue nodes = SIPath_Nodes(path);
 	_ResultSet_CompactReplyWithSIValue(ctx, gc, nodes);
-	SIValue_Free(&nodes);
+	SIValue_Free(nodes);
 	// Second array type and value.
 	RedisModule_ReplyWithArray(ctx, 2);
 	SIValue relationships = SIPath_Relationships(path);
 	_ResultSet_CompactReplyWithSIValue(ctx, gc, relationships);
-	SIValue_Free(&relationships);
+	SIValue_Free(relationships);
 }
 
 void ResultSet_EmitCompactRecord(RedisModuleCtx *ctx, GraphContext *gc, const Record r,

diff --git a/src/resultset/formatters/resultset_replyverbose.c b/src/resultset/formatters/resultset_replyverbose.c
@@ -162,7 +162,7 @@ static void _ResultSet_VerboseReplyWithArray(RedisModuleCtx *ctx, SIValue array)
 static void _ResultSet_VerboseReplyWithPath(RedisModuleCtx *ctx, SIValue path) {
 	SIValue path_array = SIPath_ToList(path);
 	_ResultSet_VerboseReplyWithArray(ctx, path_array);
-	SIValue_Free(&path_array);
+	SIValue_Free(path_array);
 }
 
 void ResultSet_EmitVerboseRecord(RedisModuleCtx *ctx, GraphContext *gc, const Record r,

diff --git a/src/value.c b/src/value.c
@@ -604,21 +604,21 @@ XXH64_hash_t SIValue_HashCode(SIValue v) {
 	return hashCode;
 }
 
-void SIValue_Free(SIValue *v) {
+void SIValue_Free(SIValue v) {
 	// The free routine only performs work if it owns a heap allocation.
-	if(v->allocation != M_SELF) return;
+	if(v.allocation != M_SELF) return;
 
-	switch(v->type) {
+	switch(v.type) {
 	case T_STRING:
-		rm_free(v->stringval);
-		v->stringval = NULL;
+		rm_free(v.stringval);
+		v.stringval = NULL;
 		return;
 	case T_NODE:
 	case T_EDGE:
-		rm_free(v->ptrval);
+		rm_free(v.ptrval);
 		return;
 	case T_ARRAY:
-		SIArray_Free(*v);
+		SIArray_Free(v);
 	default:
 		return;
 	}

diff --git a/src/value.h b/src/value.h
@@ -168,4 +168,5 @@ XXH64_hash_t SIValue_HashCode(SIValue v);
 
 /* Free an SIValue's internal property if that property is a heap allocation owned
  * by this object. */
-void SIValue_Free(SIValue *v);
+void SIValue_Free(SIValue v);
+
diff --git a/tests/unit/test_value.cpp b/tests/unit/test_value.cpp
@@ -54,7 +54,7 @@ TEST_F(ValueTest, TestNumerics) {
 	ASSERT_TRUE(v.type == T_DOUBLE);
 	ASSERT_EQ(v.doubleval, 10);
 
-	SIValue_Free(&v);
+	SIValue_Free(v);
 }
 
 TEST_F(ValueTest, TestStrings) {
@@ -64,14 +64,14 @@ TEST_F(ValueTest, TestStrings) {
 	v = SIValue_FromString(str);
 	ASSERT_TRUE(v.type == T_STRING);
 	ASSERT_STREQ(v.stringval, "Test!");
-	SIValue_Free(&v);
+	SIValue_Free(v);
 
 	/* Out of double range */
 	str = "1.0001e10001";
 	v = SIValue_FromString(str);
 	ASSERT_TRUE(v.type == T_STRING);
 	ASSERT_STREQ(v.stringval, "1.0001e10001");
-	SIValue_Free(&v);
+	SIValue_Free(v);
 }
 
 // Idempotence and correctness tests for null, bool, long, double, edge, node, array.
@@ -338,3 +338,4 @@ TEST_F(ValueTest, TestSet) {
 	ASSERT_EQ(Set_Size(set), 0);
 	Set_Free(set);
 }
+