THRIFT-5157 Fix memory leak in c_glib unit tests
Client: c_glib
Patch: wangyunjian
Signed-off-by: wangyunjian <wangyunjian@huawei.com>
diff --git a/test/c_glib/src/test_client.c b/test/c_glib/src/test_client.c
index 7126a86..76e0ad7 100644
--- a/test/c_glib/src/test_client.c
+++ b/test/c_glib/src/test_client.c
@@ -75,39 +75,6 @@
return result;
}
-/**
- * It gets a multiplexed protocol which uses a concrete protocol underneath
- * @param protocol_name the fully qualified protocol path (e.g. "binary:multi")
- * @param transport the underlying transport
- * @param service_name the single supported service name
- * @todo need to allow multiple services to fully test multiplexed
- * @return a multiplexed protocol wrapping the correct underlying protocol
- */
-ThriftProtocol *
-get_multiplexed_protocol(gchar *protocol_name, ThriftTransport *transport, gchar *service_name)
-{
- ThriftProtocol * multiplexed_protocol = NULL;
-
- if ( strncmp(protocol_name, "binary:", 7) == 0) {
- multiplexed_protocol = g_object_new (THRIFT_TYPE_BINARY_PROTOCOL,
- "transport", transport,
- NULL);
- } else if ( strncmp(protocol_name, "compact:", 8) == 0) {
- multiplexed_protocol = g_object_new (THRIFT_TYPE_COMPACT_PROTOCOL,
- "transport", transport,
- NULL);
- } else {
- fprintf(stderr, "Unknown multiplex protocol name: %s\n", protocol_name);
- return NULL;
- }
-
- return g_object_new (THRIFT_TYPE_MULTIPLEXED_PROTOCOL,
- "transport", transport,
- "protocol", multiplexed_protocol,
- "service-name", service_name,
- NULL);
-}
-
int
main (int argc, char **argv)
{
@@ -151,6 +118,7 @@
ThriftTransport *transport = NULL;
ThriftProtocol *protocol = NULL;
ThriftProtocol *protocol2 = NULL; // for multiplexed tests
+ ThriftProtocol *multiplexed_protocol = NULL;
TTestThriftTestIf *test_client = NULL;
TTestSecondServiceIf *second_service = NULL; // for multiplexed tests
@@ -179,6 +147,8 @@
&argv,
&error)) {
fprintf (stderr, "%s\n", error->message);
+ g_clear_error (&error);
+ g_option_context_free (option_context);
return 255;
}
g_option_context_free (option_context);
@@ -279,24 +249,47 @@
"transport", socket,
NULL);
- if(protocol_type==THRIFT_TYPE_MULTIPLEXED_PROTOCOL) {
+ if (protocol_type == THRIFT_TYPE_MULTIPLEXED_PROTOCOL) {
// TODO: A multiplexed test should also test "Second" (see Java TestServer)
// The context comes from the name of the thrift file. If multiple thrift
// schemas are used we have to redo the way this is done.
- protocol = get_multiplexed_protocol(protocol_name, transport, "ThriftTest");
- if (NULL == protocol) {
+ if (strncmp(protocol_name, "binary:", 7) == 0) {
+ multiplexed_protocol = g_object_new (THRIFT_TYPE_BINARY_PROTOCOL,
+ "transport", transport,
+ NULL);
+ } else if (strncmp(protocol_name, "compact:", 8) == 0) {
+ multiplexed_protocol = g_object_new (THRIFT_TYPE_COMPACT_PROTOCOL,
+ "transport", transport,
+ NULL);
+ } else {
+ fprintf(stderr, "Unknown multiplex protocol name: %s\n", protocol_name);
g_clear_object (&transport);
g_clear_object (&socket);
return 252;
}
+ protocol = g_object_new (THRIFT_TYPE_MULTIPLEXED_PROTOCOL,
+ "transport", transport,
+ "protocol", multiplexed_protocol,
+ "service-name", "ThriftTest",
+ NULL);;
+ if (NULL == protocol) {
+ g_clear_object (&multiplexed_protocol);
+ g_clear_object (&transport);
+ g_clear_object (&socket);
+ return 251;
+ }
// Make a second protocol and client running on the same multiplexed transport
- protocol2 = get_multiplexed_protocol(protocol_name, transport, "SecondService");
- second_service = g_object_new (T_TEST_TYPE_SECOND_SERVICE_CLIENT,
- "input_protocol", protocol2,
- "output_protocol", protocol2,
- NULL);
+ protocol2 = g_object_new (THRIFT_TYPE_MULTIPLEXED_PROTOCOL,
+ "transport", transport,
+ "protocol", multiplexed_protocol,
+ "service-name", "SecondService",
+ NULL);
+ second_service = g_object_new (T_TEST_TYPE_SECOND_SERVICE_CLIENT,
+ "input_protocol", protocol2,
+ "output_protocol", protocol2,
+ NULL);
}else{
protocol = g_object_new (protocol_type,
"transport", transport,
@@ -1374,6 +1367,8 @@
byte_thing,
i32_thing,
i64_thing);
+ if (string != NULL)
+ g_free (string);
}
printf ("}");
g_ptr_array_unref (xtructs);
@@ -1796,7 +1791,7 @@
g_error_free (error);
error = NULL;
- return 1;
+ goto out;
}
}
@@ -1810,10 +1805,12 @@
printf ("Max time: %" PRIu64 " us\n", time_max_usec);
printf ("Avg time: %" PRIu64 " us\n", time_avg_usec);
+out:
g_clear_object(&second_service);
g_clear_object(&protocol2);
g_clear_object(&test_client);
g_clear_object(&protocol);
+ g_clear_object(&multiplexed_protocol);
g_clear_object(&transport);
g_clear_object(&socket);
diff --git a/test/c_glib/src/test_server.c b/test/c_glib/src/test_server.c
index 0819b8c..c949530 100644
--- a/test/c_glib/src/test_server.c
+++ b/test/c_glib/src/test_server.c
@@ -131,6 +131,8 @@
&argv,
&error) == FALSE) {
fprintf (stderr, "%s\n", error->message);
+ g_clear_error (&error);
+ g_option_context_free (option_context);
return 255;
}
g_option_context_free (option_context);
@@ -282,11 +284,11 @@
if (!sigint_received) {
g_message ("thrift_server_serve: %s",
error != NULL ? error->message : "(null)");
- g_clear_error (&error);
}
puts ("done.");
+ g_clear_error (&error);
g_object_unref (server);
g_object_unref (protocol_factory);
g_object_unref (transport_factory);
diff --git a/test/c_glib/src/thrift_test_handler.c b/test/c_glib/src/thrift_test_handler.c
index f6c759e..ccfd6c2 100644
--- a/test/c_glib/src/thrift_test_handler.c
+++ b/test/c_glib/src/thrift_test_handler.c
@@ -367,7 +367,7 @@
printf ("testList({");
for (i = 0; i < thing->len; i += 1) {
gint32 value;
- gint32 *new_value;
+ gint32 new_value;
if (first)
first = FALSE;
@@ -377,9 +377,8 @@
value = g_array_index (thing, gint32, i);
printf ("%d", value);
- new_value = g_malloc (sizeof *new_value);
- *new_value = value;
- g_array_append_val (*_return, *new_value);
+ new_value = value;
+ g_array_append_val (*_return, new_value);
}
printf ("})\n");
@@ -591,6 +590,9 @@
byte_thing,
i32_thing,
i64_thing);
+ if (string_thing != NULL) {
+ g_free (string_thing);
+ }
}
printf ("}");
g_ptr_array_unref (xtructs);
@@ -624,10 +626,10 @@
printf ("testMulti()\n");
g_object_set (*_return,
- "string_thing", g_strdup ("Hello2"),
- "byte_thing", arg0,
- "i32_thing", arg1,
- "i64_thing", arg2,
+ "string_thing", "Hello2",
+ "byte_thing", arg0,
+ "i32_thing", arg1,
+ "i64_thing", arg2,
NULL);
return TRUE;
@@ -654,7 +656,7 @@
argument, set *error to NULL and return FALSE */
*err1 = g_object_new (T_TEST_TYPE_XCEPTION,
"errorCode", 1001,
- "message", g_strdup (arg),
+ "message", arg,
NULL);
*error = NULL;
result = FALSE;
@@ -676,7 +678,7 @@
/* This code is duplicated from the C++ test suite, though it
appears to serve no purpose */
xtruct = g_object_new (T_TEST_TYPE_XTRUCT,
- "string_thing", g_strdup (arg),
+ "string_thing", arg,
NULL);
g_object_unref (xtruct);
@@ -709,7 +711,7 @@
if (strncmp (arg0, "Xception", 8) == 0 && strlen(arg0) == 8) {
*err1 = g_object_new (T_TEST_TYPE_XCEPTION,
"errorCode", 1001,
- "message", g_strdup ("This is an Xception"),
+ "message", "This is an Xception",
NULL);
result = FALSE;
}
@@ -722,7 +724,7 @@
"struct_thing", &struct_thing,
NULL);
g_object_set (struct_thing,
- "string_thing", g_strdup ("This is an Xception2"),
+ "string_thing", "This is an Xception2",
NULL);
g_object_set (*err2,
"struct_thing", struct_thing,
@@ -733,7 +735,7 @@
}
else {
g_object_set (*_return,
- "string_thing", g_strdup (arg1),
+ "string_thing", arg1,
NULL);
result = TRUE;
}