THRIFT-3464 Fix several defects in c_glib code generator
Client: c_glib compiler
Patch: Nobuaki Sukegawa
This closes #724
diff --git a/compiler/cpp/src/generate/t_c_glib_generator.cc b/compiler/cpp/src/generate/t_c_glib_generator.cc
index e6b48a4..22d7914 100644
--- a/compiler/cpp/src/generate/t_c_glib_generator.cc
+++ b/compiler/cpp/src/generate/t_c_glib_generator.cc
@@ -23,6 +23,7 @@
#include <fstream>
#include <iostream>
+#include <stdexcept>
#include <string>
#include <vector>
@@ -122,12 +123,14 @@
/* helper functions */
bool is_complex_type(t_type* ttype);
+ bool is_numeric(t_type* ttype);
string type_name(t_type* ttype, bool in_typedef = false, bool is_const = false);
string property_type_name(t_type* ttype, bool in_typedef = false, bool is_const = false);
- string base_type_name(t_base_type* type);
+ string base_type_name(t_type* type);
string type_to_enum(t_type* type);
string constant_literal(t_type* type, t_const_value* value);
string constant_value(string name, t_type* type, t_const_value* value);
+ string constant_value_with_storage(string name, t_type* type, t_const_value* value);
string function_signature(t_function* tfunction);
string argument_list(t_struct* tstruct);
string xception_list(t_struct* tstruct);
@@ -136,10 +139,14 @@
bool pointer = false,
bool constant = false,
bool reference = false);
- void declare_local_variable(ofstream& out, t_type* ttype, string& base_name);
+ void declare_local_variable(ofstream& out, t_type* ttype, string& base_name, bool for_hash_table);
+ void declore_local_variable_for_write(ofstream& out, t_type* ttype, string& base_name);
/* generation functions */
- void generate_const_initializer(string name, t_type* type, t_const_value* value);
+ void generate_const_initializer(string name,
+ t_type* type,
+ t_const_value* value,
+ bool top_level = false);
void generate_service_helpers(t_service* tservice);
void generate_service_client(t_service* tservice);
void generate_service_handler(t_service* tservice);
@@ -348,10 +355,17 @@
f_types_impl_ << "{" << endl;
f_types_impl_ << " static __thread char buf[16] = {0};" << endl;
f_types_impl_ << " switch(value) {" << endl;
+ std::set<int> done;
for (c_iter = constants.begin(); c_iter != constants.end(); ++c_iter) {
- f_types_impl_ << " case " << this->nspace_uc << name_uc << "_" << (*c_iter)->get_name() << ":"
- << "return \"" << this->nspace_uc << name_uc << "_" << (*c_iter)->get_name()
- << "\";" << endl;
+ int value = (*c_iter)->get_value();
+ // Skipping duplicate value
+ if (done.find(value) == done.end()) {
+ done.insert(value);
+ f_types_impl_ << " case " << this->nspace_uc << name_uc << "_" << (*c_iter)->get_name()
+ << ":"
+ << "return \"" << this->nspace_uc << name_uc << "_" << (*c_iter)->get_name()
+ << "\";" << endl;
+ }
}
f_types_impl_ << " default: g_snprintf(buf, 16, \"%d\", value); return buf;" << endl;
f_types_impl_ << " }" << endl;
@@ -373,10 +387,15 @@
t_type* type = (*c_iter)->get_type();
t_const_value* value = (*c_iter)->get_value();
+ if (is_complex_type(type)) {
+ f_types_ << type_name(type) << indent() << this->nspace_lc << name_lc
+ << "_constant();" << endl;
+ }
+
f_types_ << indent() << "#define " << this->nspace_uc << name_uc << " "
<< constant_value(name_lc, type, value) << endl;
- generate_const_initializer(name_lc, type, value);
+ generate_const_initializer(name_lc, type, value, true);
}
f_types_ << endl;
@@ -521,12 +540,16 @@
return ttype->is_container() || ttype->is_struct() || ttype->is_xception();
}
+bool t_c_glib_generator::is_numeric(t_type* ttype) {
+ return ttype->is_enum() || (ttype->is_base_type() && !ttype->is_string());
+}
+
/**
* Maps a Thrift t_type to a C type.
*/
string t_c_glib_generator::type_name(t_type* ttype, bool in_typedef, bool is_const) {
if (ttype->is_base_type()) {
- string bname = base_type_name((t_base_type*)ttype);
+ string bname = base_type_name(ttype);
if (is_const) {
return "const " + bname;
@@ -550,28 +573,12 @@
// TODO: discuss whether or not to implement TSet, THashSet or GHashSet
cname = "GHashTable";
} else if (ttype->is_list()) {
- // TODO: investigate other implementations besides GPtrArray
- cname = "GPtrArray";
t_type* etype = ((t_list*)ttype)->get_elem_type();
- if (etype->is_base_type()) {
- t_base_type::t_base tbase = ((t_base_type*)etype)->get_base();
- switch (tbase) {
- case t_base_type::TYPE_VOID:
- throw "compiler error: cannot determine array type";
- case t_base_type::TYPE_BOOL:
- case t_base_type::TYPE_I8:
- case t_base_type::TYPE_I16:
- case t_base_type::TYPE_I32:
- case t_base_type::TYPE_I64:
- case t_base_type::TYPE_DOUBLE:
- cname = "GArray";
- break;
- case t_base_type::TYPE_STRING:
- break;
- default:
- throw "compiler error: no array info for type";
- }
+ if (etype->is_void()) {
+ throw std::runtime_error("compiler error: list element type cannot be void");
}
+ // TODO: investigate other implementations besides GPtrArray
+ cname = is_numeric(etype) ? "GArray" : "GPtrArray";
}
/* Omit the dereference operator if we are aliasing this type within a
@@ -638,14 +645,20 @@
/**
* Maps a Thrift primitive to a C primitive.
*/
-string t_c_glib_generator::base_type_name(t_base_type* type) {
- t_base_type::t_base tbase = type->get_base();
-
+string t_c_glib_generator::base_type_name(t_type* type) {
+ if (type->is_enum()) {
+ return type_name(type);
+ }
+ if (!type->is_base_type()) {
+ throw std::invalid_argument("Only base types are suppported.");
+ }
+ t_base_type* base_type = reinterpret_cast<t_base_type*>(type);
+ t_base_type::t_base tbase = base_type->get_base();
switch (tbase) {
case t_base_type::TYPE_VOID:
return "void";
case t_base_type::TYPE_STRING:
- if (type->is_binary()) {
+ if (base_type->is_binary()) {
return "GByteArray *";
} else {
return "gchar *";
@@ -663,7 +676,8 @@
case t_base_type::TYPE_DOUBLE:
return "gdouble";
default:
- throw "compiler error: no C base type name for base type " + t_base_type::t_base_name(tbase);
+ throw std::logic_error("compiler error: no C base type name for base type "
+ + t_base_type::t_base_name(tbase));
}
}
@@ -809,8 +823,7 @@
}
} else if (type->is_enum()) {
render << "(" << type_name(type) << ")" << value->get_integer();
- } else if (type->is_struct() || type->is_xception() || type->is_list() || type->is_set()
- || type->is_map()) {
+ } else if (is_complex_type(type)) {
render << "(" << this->nspace_lc << to_lower_case(name) << "_constant())";
} else {
render << "NULL /* not supported */";
@@ -944,16 +957,34 @@
return result;
}
+string t_c_glib_generator::constant_value_with_storage(string fname,
+ t_type* etype,
+ t_const_value* value) {
+ ostringstream render;
+ if (is_numeric(etype)) {
+ render << " " << type_name(etype) << " *" << fname << " = "
+ << "g_new (" << base_type_name(etype) << ", 1);" << endl
+ << " *" << fname << " = " << constant_value(fname, (t_type*)etype, value) << ";"
+ << endl;
+ } else {
+ render << " " << type_name(etype) << " " << fname << " = "
+ << constant_value(fname, (t_type*)etype, value) << ";" << endl;
+ }
+ return render.str();
+}
+
/**
* Generates C code that initializes complex constants.
*/
void t_c_glib_generator::generate_const_initializer(string name,
t_type* type,
- t_const_value* value) {
+ t_const_value* value,
+ bool top_level) {
string name_u = initial_caps_to_underscores(name);
string name_lc = to_lower_case(name_u);
string type_u = initial_caps_to_underscores(type->get_name());
string type_uc = to_upper_case(type_u);
+ string maybe_static = top_level ? "" : "static ";
if (type->is_struct() || type->is_xception()) {
const vector<t_field*>& fields = ((t_struct*)type)->get_members();
@@ -971,6 +1002,7 @@
if ((*f_iter)->get_name() == v_iter->first->get_string()) {
field_type = (*f_iter)->get_type();
field_name = (*f_iter)->get_name();
+ break;
}
}
if (field_type == NULL) {
@@ -991,7 +1023,7 @@
}
// implement the initializer
- f_types_impl_ << "static " << this->nspace << type->get_name() << " *"
+ f_types_impl_ << maybe_static << this->nspace << type->get_name() << " *"
<< endl
<< this->nspace_lc << name_lc << "_constant (void)" << endl;
scope_up(f_types_impl_);
@@ -1003,13 +1035,33 @@
<< "TYPE_" << type_uc << ", NULL);" << endl
<< initializers.str();
scope_down(f_types_impl_);
+
+ for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
+ t_type* field_type = NULL;
+ string field_name = "";
+
+ for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
+ if ((*f_iter)->get_name() == v_iter->first->get_string()) {
+ field_type = (*f_iter)->get_type();
+ field_name = (*f_iter)->get_name();
+ break;
+ }
+ }
+ if (field_type == NULL) {
+ throw "type error: " + type->get_name() + " has no field "
+ + v_iter->first->get_string();
+ }
+ field_name = tmp(field_name);
+ }
+
f_types_impl_ << indent() << "return constant;" << endl;
scope_down(f_types_impl_);
f_types_impl_ << endl;
} else if (type->is_list()) {
string list_type = "GPtrArray *";
- // TODO: This initialization should contain a free function for container
- string list_initializer = "g_ptr_array_new();";
+ string free_func
+ = generate_free_func_from_type(reinterpret_cast<t_list*>(type)->get_elem_type());
+ string list_initializer = "g_ptr_array_new_with_free_func (" + free_func + ");";
string list_appender = "g_ptr_array_add";
bool list_variable = false;
@@ -1040,6 +1092,10 @@
default:
throw "compiler error: no array info for type";
}
+ } else if (etype->is_enum()) {
+ list_type = "GArray *";
+ list_appender = "g_array_append_val";
+ list_variable = true;
}
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
@@ -1059,7 +1115,7 @@
}
}
- f_types_impl_ << "static " << list_type << endl
+ f_types_impl_ << maybe_static << list_type << endl
<< this->nspace_lc << name_lc << "_constant (void)" << endl;
scope_up(f_types_impl_);
f_types_impl_ << indent() << "static " << list_type << " constant = NULL;"
@@ -1085,26 +1141,20 @@
for (v_iter = val.begin(); v_iter != val.end(); ++v_iter) {
string fname = tmp(name);
+ string ptr = is_numeric(etype) ? "*" : "";
generate_const_initializer(fname, etype, (*v_iter));
- initializers << " " << type_name(etype) << " " << fname << " = "
- << constant_value(fname, (t_type*)etype, (*v_iter)) << ";"
- << endl;
- appenders << " g_hash_table_insert (constant, &" << fname << ", &"
- << fname << ");" << endl;
+ initializers << constant_value_with_storage(fname, (t_type*)etype, *v_iter);
+ appenders << " g_hash_table_insert (constant, " << fname << ", 0);" << endl;
}
- f_types_impl_ << "static GHashTable *" << endl
+ f_types_impl_ << maybe_static << "GHashTable *" << endl
<< this->nspace_lc << name_lc << "_constant (void)" << endl;
scope_up(f_types_impl_);
f_types_impl_ << indent() << "static GHashTable *constant = NULL;" << endl
<< indent() << "if (constant == NULL)" << endl;
scope_up(f_types_impl_);
- f_types_impl_ << initializers.str()
- << endl
- // TODO: This initialization should contain a free function
- // for elements
- << indent() << "constant = g_hash_table_new (NULL, NULL);"
- << endl
+ f_types_impl_ << initializers.str() << endl
+ << indent() << "constant = " << generate_new_hash_from_type(etype, NULL) << endl
<< appenders.str();
scope_down(f_types_impl_);
f_types_impl_ << indent() << "return constant;" << endl;
@@ -1113,8 +1163,8 @@
} else if (type->is_map()) {
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
- const vector<t_const_value*>& val = value->get_list();
- vector<t_const_value*>::const_iterator v_iter;
+ const map<t_const_value*, t_const_value*>& val = value->get_map();
+ map<t_const_value*, t_const_value*>::const_iterator v_iter;
ostringstream initializers;
ostringstream appenders;
@@ -1122,32 +1172,22 @@
string fname = tmp(name);
string kname = fname + "key";
string vname = fname + "val";
- generate_const_initializer(kname, ktype, (*v_iter));
- generate_const_initializer(vname, vtype, (*v_iter));
+ generate_const_initializer(kname, ktype, v_iter->first);
+ generate_const_initializer(vname, vtype, v_iter->second);
- initializers << " " << type_name(ktype) << " " << kname << " = "
- << constant_value(kname, (t_type*)ktype, (*v_iter)) << ";"
- << endl
- << " " << type_name(vtype) << " " << vname << " = "
- << constant_value(vname, (t_type*)vtype, (*v_iter)) << ";"
- << endl;
- appenders << " g_hash_table_insert (constant, &" << fname << ", &"
- << fname << ");"
- << endl;
+ initializers << constant_value_with_storage(kname, (t_type*)ktype, v_iter->first);
+ initializers << constant_value_with_storage(vname, (t_type*)vtype, v_iter->second);
+ appenders << " g_hash_table_insert (constant, " << kname << ", " << vname << ");" << endl;
}
- f_types_impl_ << "static GHashTable *" << endl
+ f_types_impl_ << maybe_static << "GHashTable *" << endl
<< this->nspace_lc << name_lc << "_constant (void)" << endl;
scope_up(f_types_impl_);
f_types_impl_ << indent() << "static GHashTable *constant = NULL;" << endl
<< indent() << "if (constant == NULL)" << endl;
scope_up(f_types_impl_);
- f_types_impl_ << initializers.str()
- << endl
- // TODO: This initialization should contain a free function
- // for elements
- << indent() << "constant = g_hash_table_new (NULL, NULL);"
- << endl
+ f_types_impl_ << initializers.str() << endl
+ << indent() << "constant = " << generate_new_hash_from_type(ktype, vtype) << endl
<< appenders.str();
scope_down(f_types_impl_);
f_types_impl_ << indent() << "return constant;" << endl;
@@ -2334,8 +2374,7 @@
t_type* elem_type = ((t_list*)return_type)->get_elem_type();
f_service_ << indent();
- if ((elem_type->is_base_type() && !elem_type->is_string())
- || elem_type->is_enum()) {
+ if (is_numeric(elem_type)) {
f_service_ << "g_array_unref";
} else {
f_service_ << "g_ptr_array_unref";
@@ -2470,8 +2509,7 @@
t_type* elem_type = ((t_list*)arg_type)->get_elem_type();
f_service_ << indent();
- if ((elem_type->is_base_type() && !elem_type->is_string())
- || elem_type->is_enum()) {
+ if (is_numeric(elem_type)) {
f_service_ << "g_array_unref";
} else {
f_service_ << "g_ptr_array_unref";
@@ -2650,7 +2688,7 @@
scope_up(f_service_);
f_service_ << indent() << this->nspace << service_name_ << "Processor *self = " << this->nspace_uc
<< service_name_uc << "_PROCESSOR (gobject);" << endl << endl << indent()
- << "g_hash_table_destroy (self->process_map);" << endl << endl << indent()
+ << "thrift_safe_hash_table_destroy (self->process_map);" << endl << endl << indent()
<< "G_OBJECT_CLASS (" << class_name_lc << "_parent_class)"
"->finalize (gobject);" << endl;
scope_down(f_service_);
@@ -2894,7 +2932,7 @@
// Lists of base types other than strings are represented as GArrays;
// all others as GPtrArrays
- if (elem_type->is_base_type() && !elem_type->is_string()) {
+ if (is_numeric(elem_type)) {
release_function_name = "g_array_unref";
} else {
release_function_name = "g_ptr_array_unref";
@@ -3125,8 +3163,7 @@
if (t->is_list()) {
const vector<t_const_value*>& list = member_value->get_list();
- if ((etype->is_base_type() && !etype->is_string())
- || etype->is_enum()) {
+ if (is_numeric(etype)) {
indent(f_types_impl_) <<
"g_array_append_vals (object->" << name << ", &__default_" <<
name << ", " << list.size() << ");" << endl;
@@ -3202,6 +3239,8 @@
default:
throw "compiler error: no array info for type";
}
+ } else if (etype->is_enum()) {
+ destructor_function = "g_array_unref";
}
f_types_impl_ << indent() << "if (tobject->" << name << " != NULL)" << endl;
@@ -3687,8 +3726,8 @@
break;
case t_base_type::TYPE_STRING:
if (((t_base_type*)type)->is_binary()) {
- out << "binary (protocol, ((GByteArray *) " << name << ")->data, ((GByteArray *) " << name
- << ")->len";
+ out << "binary (protocol, " << name << " ? ((GByteArray *) " << name << ")->data : NULL, "
+ << name << " ? ((GByteArray *) " << name << ")->len : 0";
} else {
out << "string (protocol, " << name;
}
@@ -3696,17 +3735,15 @@
default:
throw "compiler error: no C writer for base type " + t_base_type::t_base_name(tbase) + name;
}
- } else if (type->is_enum()) {
+ } else {
out << "i32 (protocol, (gint32) " << name;
}
out << ", error)) < 0)" << endl
<< indent() << " return " << error_ret << ";" << endl
- << indent() << "xfer += ret;" << endl
- << endl;
+ << indent() << "xfer += ret;" << endl << endl;
} else {
- printf("DO NOT KNOW HOW TO SERIALIZE FIELD '%s' TYPE '%s'\n",
- name.c_str(),
- type_name(type).c_str());
+ throw std::logic_error("DO NOT KNOW HOW TO SERIALIZE FIELD '" + name + "' TYPE '"
+ + type_name(type));
}
}
@@ -3737,8 +3774,8 @@
string keyname = tmp("key");
string valname = tmp("val");
- declare_local_variable(out, tkey, keyname);
- declare_local_variable(out, tval, valname);
+ declore_local_variable_for_write(out, tkey, keyname);
+ declore_local_variable_for_write(out, tval, valname);
/* If either the key or value type is a typedef, find its underlying type so
we can correctly determine how to generate a pointer to it */
@@ -3906,62 +3943,20 @@
string cast = "";
string name = "g_ptr_array_index ((GPtrArray *) " + list + ", " + index + ")";
- if (ttype->is_base_type()) {
- t_base_type::t_base tbase = ((t_base_type*)ttype)->get_base();
- switch (tbase) {
- case t_base_type::TYPE_VOID:
- throw "compiler error: cannot determine array type";
- break;
- case t_base_type::TYPE_BOOL:
- name = "g_array_index (" + list + ", gboolean, " + index + ")";
- break;
- case t_base_type::TYPE_I8:
- name = "g_array_index (" + list + ", gint8, " + index + ")";
- break;
- case t_base_type::TYPE_I16:
- name = "g_array_index (" + list + ", gint16, " + index + ")";
- break;
- case t_base_type::TYPE_I32:
- name = "g_array_index (" + list + ", gint32, " + index + ")";
- break;
- case t_base_type::TYPE_I64:
- name = "g_array_index (" + list + ", gint64, " + index + ")";
- break;
- case t_base_type::TYPE_DOUBLE:
- name = "g_array_index (" + list + ", gdouble, " + index + ")";
- break;
- case t_base_type::TYPE_STRING:
- cast = "(gchar*)";
- break;
- default:
- throw "compiler error: no array info for type";
- }
+ if (ttype->is_void()) {
+ throw std::runtime_error("compiler error: list element type cannot be void");
+ } else if (is_numeric(ttype)) {
+ name = "g_array_index (" + list + ", " + base_type_name(ttype) + ", " + index + ")";
+ } else if (ttype->is_string()) {
+ cast = "(gchar*)";
} else if (ttype->is_map() || ttype->is_set()) {
cast = "(GHashTable*)";
} else if (ttype->is_list()) {
- t_type* base = ((t_list*)ttype)->get_elem_type();
- if (base->is_base_type()) {
- switch (((t_base_type*)base)->get_base()) {
- case t_base_type::TYPE_VOID:
- throw "compiler error: cannot determine array type";
- break;
- case t_base_type::TYPE_BOOL:
- case t_base_type::TYPE_I8:
- case t_base_type::TYPE_I16:
- case t_base_type::TYPE_I32:
- case t_base_type::TYPE_I64:
- case t_base_type::TYPE_DOUBLE:
- cast = "(GArray*)";
- break;
- case t_base_type::TYPE_STRING:
- cast = "(GPtrArray*)";
- break;
- default:
- throw "Compiler error: no array info for type";
- }
- } else {
- cast = "(GPtrArray*)";
+ t_type* etype = ((t_list*)ttype)->get_elem_type();
+ if (etype->is_void()) {
+ throw std::runtime_error("compiler error: list element type cannot be void");
}
+ cast = is_numeric(etype) ? "(GArray*)" : "(GPtrArray*)";
}
t_field efield(ttype, "(" + cast + name + ")");
@@ -3978,7 +3973,8 @@
t_type* type = get_true_type(tfield->get_type());
if (type->is_void()) {
- throw "CANNOT GENERATE DESERIALIZE CODE FOR void TYPE: " + prefix + tfield->get_name();
+ throw std::runtime_error("CANNOT GENERATE DESERIALIZE CODE FOR void TYPE: " + prefix
+ + tfield->get_name());
}
string name = prefix + tfield->get_name() + suffix;
@@ -4047,9 +4043,8 @@
<< indent() << " return " << error_ret << ";" << endl << indent() << "xfer += ret;" << endl
<< indent() << name << " = (" << type_name(type) << ")" << t << ";" << endl;
} else {
- printf("DO NOT KNOW HOW TO DESERIALIZE FIELD '%s' TYPE '%s'\n",
- tfield->get_name().c_str(),
- type_name(type).c_str());
+ throw std::logic_error("DO NOT KNOW HOW TO SERIALIZE FIELD '" + tfield->get_name() + "' TYPE '"
+ + type_name(type));
}
// if the type is not required and this is a thrift struct (no prefix),
@@ -4196,14 +4191,13 @@
scope_down(out);
}
-void t_c_glib_generator::declare_local_variable(ofstream& out, t_type* ttype, string& name) {
+void t_c_glib_generator::declare_local_variable(ofstream& out, t_type* ttype, string& name, bool for_hash_table) {
string tname = type_name(ttype);
/* If the given type is a typedef, find its underlying type so we
can correctly determine how to generate a pointer to it */
ttype = get_true_type(ttype);
-
- string ptr = ttype->is_string() || !ttype->is_base_type() ? "" : "*";
+ string ptr = !is_numeric(ttype) ? "" : "*";
if (ttype->is_map()) {
t_map* tmap = (t_map*)ttype;
@@ -4213,14 +4207,24 @@
t_list* tlist = (t_list*)ttype;
out << indent() << tname << ptr << " " << name << " = "
<< generate_new_array_from_type(tlist->get_elem_type()) << endl;
- } else if (ttype->is_enum()) {
- out << indent() << tname << ptr << " " << name << ";" << endl;
+ } else if (for_hash_table && ttype->is_enum()) {
+ out << indent() << tname << " " << name << ";" << endl;
} else {
out << indent() << tname << ptr << " " << name
<< (ptr != "" ? " = g_new (" + tname + ", 1)" : " = NULL") << ";" << endl;
}
}
+void t_c_glib_generator::declore_local_variable_for_write(ofstream& out,
+ t_type* ttype,
+ string& name) {
+ string tname = type_name(ttype);
+ ttype = get_true_type(ttype);
+ string ptr = ttype->is_string() || !ttype->is_base_type() ? " " : "* ";
+ string init_val = ttype->is_enum() ? "" : " = NULL";
+ out << indent() << tname << ptr << name << init_val << ";" << endl;
+}
+
void t_c_glib_generator::generate_deserialize_map_element(ofstream& out,
t_map* tmap,
string prefix,
@@ -4230,8 +4234,8 @@
string keyname = tmp("key");
string valname = tmp("val");
- declare_local_variable(out, tkey, keyname);
- declare_local_variable(out, tval, valname);
+ declare_local_variable(out, tkey, keyname, true);
+ declare_local_variable(out, tval, valname, true);
/* If either the key or value type is a typedef, find its underlying
type so we can correctly determine how to generate a pointer to
@@ -4248,8 +4252,11 @@
t_field fval(tval, tval_ptr + valname);
generate_deserialize_field(out, &fval, "", "", error_ret);
+ indent(out) << "if (" << prefix << " && " << keyname << ")" << endl;
+ indent_up();
indent(out) << "g_hash_table_insert ((GHashTable *)" << prefix << ", (gpointer) " << keyname
<< ", (gpointer) " << valname << ");" << endl;
+ indent_down();
}
void t_c_glib_generator::generate_deserialize_set_element(ofstream& out,
@@ -4260,13 +4267,16 @@
string elem = tmp("_elem");
string telem_ptr = telem->is_string() || !telem->is_base_type() ? "" : "*";
- declare_local_variable(out, telem, elem);
+ declare_local_variable(out, telem, elem, true);
t_field felem(telem, telem_ptr + elem);
generate_deserialize_field(out, &felem, "", "", error_ret);
+ indent(out) << "if (" << prefix << " && " << elem << ")" << endl;
+ indent_up();
indent(out) << "g_hash_table_insert ((GHashTable *) " << prefix << ", (gpointer) " << elem
- << ", (gpointer) 1);" << endl;
+ << ", (gpointer) " << elem << ");" << endl;
+ indent_down();
}
void t_c_glib_generator::generate_deserialize_list_element(ofstream& out,
@@ -4275,38 +4285,22 @@
string index,
int error_ret) {
(void)index;
- t_type* ttype = tlist->get_elem_type();
+ t_type* ttype = get_true_type(tlist->get_elem_type());
string elem = tmp("_elem");
- string telem_ptr = ttype->is_string() || !ttype->is_base_type() ? "" : "*";
+ string telem_ptr = !is_numeric(ttype) ? "" : "*";
- declare_local_variable(out, ttype, elem);
+ declare_local_variable(out, ttype, elem, false);
t_field felem(ttype, telem_ptr + elem);
generate_deserialize_field(out, &felem, "", "", error_ret);
- indent(out);
-
- if (ttype->is_base_type()) {
- t_base_type::t_base tbase = ((t_base_type*)ttype)->get_base();
- switch (tbase) {
- case t_base_type::TYPE_VOID:
- throw "compiler error: cannot determine array type";
- case t_base_type::TYPE_STRING:
- out << "g_ptr_array_add (" << prefix << ", " << elem << ");" << endl;
- return;
- case t_base_type::TYPE_BOOL:
- case t_base_type::TYPE_I8:
- case t_base_type::TYPE_I16:
- case t_base_type::TYPE_I32:
- case t_base_type::TYPE_I64:
- case t_base_type::TYPE_DOUBLE:
- out << "g_array_append_vals (" << prefix << ", " << elem << ", 1);" << endl;
- return;
- default:
- throw "compiler error: no array info for type";
- }
+ if (ttype->is_void()) {
+ throw std::runtime_error("compiler error: list element type cannot be void");
+ } else if (is_numeric(ttype)) {
+ indent(out) << "g_array_append_vals (" << prefix << ", " << elem << ", 1);" << endl;
+ } else {
+ indent(out) << "g_ptr_array_add (" << prefix << ", " << elem << ");" << endl;
}
- out << "g_ptr_array_add (" << prefix << ", " << elem << ");" << endl;
}
string t_c_glib_generator::generate_free_func_from_type(t_type* ttype) {
@@ -4337,7 +4331,7 @@
} else if (ttype->is_enum()) {
return "NULL";
} else if (ttype->is_map() || ttype->is_set()) {
- return "(GDestroyNotify) g_hash_table_destroy";
+ return "(GDestroyNotify) thrift_safe_hash_table_destroy";
} else if (ttype->is_struct()) {
return "g_object_unref";
} else if (ttype->is_list()) {
@@ -4386,8 +4380,11 @@
throw "compiler error: cannot determine hash type";
break;
case t_base_type::TYPE_BOOL:
+ return "thrift_boolean_hash";
case t_base_type::TYPE_I8:
+ return "thrift_int8_hash";
case t_base_type::TYPE_I16:
+ return "thrift_int16_hash";
case t_base_type::TYPE_I32:
return "g_int_hash";
case t_base_type::TYPE_I64:
@@ -4421,8 +4418,11 @@
throw "compiler error: cannot determine hash type";
break;
case t_base_type::TYPE_BOOL:
+ return "thrift_boolean_equal";
case t_base_type::TYPE_I8:
+ return "thrift_int8_equal";
case t_base_type::TYPE_I16:
+ return "thrift_int16_equal";
case t_base_type::TYPE_I32:
return "g_int_equal";
case t_base_type::TYPE_I64:
@@ -4456,37 +4456,14 @@
}
string t_c_glib_generator::generate_new_array_from_type(t_type* ttype) {
- if (ttype->is_base_type()) {
- t_base_type::t_base tbase = ((t_base_type*)ttype)->get_base();
- switch (tbase) {
- case t_base_type::TYPE_VOID:
- throw "compiler error: cannot determine array type";
- break;
- case t_base_type::TYPE_BOOL:
- return "g_array_new (0, 1, sizeof (gboolean));";
- case t_base_type::TYPE_I8:
- return "g_array_new (0, 1, sizeof (gint8));";
- case t_base_type::TYPE_I16:
- return "g_array_new (0, 1, sizeof (gint16));";
- case t_base_type::TYPE_I32:
- return "g_array_new (0, 1, sizeof (gint32));";
- case t_base_type::TYPE_I64:
- return "g_array_new (0, 1, sizeof (gint64));";
- case t_base_type::TYPE_DOUBLE:
- return "g_array_new (0, 1, sizeof (gdouble));";
- case t_base_type::TYPE_STRING:
- return "g_ptr_array_new_with_free_func (g_free);";
- default:
- throw "compiler error: no array info for type";
- }
- } else if (ttype->is_enum()) {
- return "g_array_new (0, 1, sizeof (gint32));";
+ if (ttype->is_void()) {
+ throw std::runtime_error("compiler error: cannot determine array type");
+ } else if (is_numeric(ttype)) {
+ return "g_array_new (0, 1, sizeof (" + base_type_name(ttype) + "));";
} else {
string free_func = generate_free_func_from_type(ttype);
return "g_ptr_array_new_with_free_func (" + free_func + ");";
}
-
- return "g_ptr_array_new();";
}
/***************************************
diff --git a/lib/c_glib/src/thrift/c_glib/thrift.c b/lib/c_glib/src/thrift/c_glib/thrift.c
index 15f409f..8de869f 100644
--- a/lib/c_glib/src/thrift/c_glib/thrift.c
+++ b/lib/c_glib/src/thrift/c_glib/thrift.c
@@ -31,6 +31,67 @@
*list = g_list_append (*list, key);
}
+void thrift_safe_hash_table_destroy(GHashTable* hash_table)
+{
+ if (hash_table)
+ {
+ g_hash_table_destroy(hash_table);
+ }
+}
+
+guint thrift_boolean_hash(gconstpointer v)
+{
+ const gboolean* p = v;
+ return p && *p ? 1 : 0;
+}
+gboolean thrift_boolean_equal(gconstpointer a, gconstpointer b)
+{
+ if (a == b) {
+ return TRUE;
+ }
+ if (!a || !b) {
+ return FALSE;
+ }
+ const gboolean* pa = a;
+ const gboolean* pb = b;
+ return *pa == *pb;
+}
+
+guint thrift_int8_hash(gconstpointer v)
+{
+ const gint8* p = v;
+ return p ? *p : 0;
+}
+gboolean thrift_int8_equal(gconstpointer a, gconstpointer b)
+{
+ if (a == b) {
+ return TRUE;
+ }
+ if (!a || !b) {
+ return FALSE;
+ }
+ const gint8* pa = a;
+ const gint8* pb = b;
+ return *pa == *pb;
+}
+
+guint thrift_int16_hash(gconstpointer v)
+{
+ const gint16* p = v;
+ return p ? *p : 0;
+}
+gboolean thrift_int16_equal(gconstpointer a, gconstpointer b)
+{
+ if (a == b) {
+ return TRUE;
+ }
+ if (!a || !b) {
+ return FALSE;
+ }
+ const gint16* pa = a;
+ const gint16* pb = b;
+ return *pa == *pb;
+}
void
thrift_string_free (gpointer str)
diff --git a/lib/c_glib/src/thrift/c_glib/thrift.h b/lib/c_glib/src/thrift/c_glib/thrift.h
index 858ad86..94a6478 100644
--- a/lib/c_glib/src/thrift/c_glib/thrift.h
+++ b/lib/c_glib/src/thrift/c_glib/thrift.h
@@ -33,6 +33,17 @@
void thrift_hash_table_get_keys (gpointer key, gpointer value,
gpointer user_data);
+void thrift_safe_hash_table_destroy(GHashTable* hash_table);
+
+guint thrift_boolean_hash(gconstpointer v);
+gboolean thrift_boolean_equal(gconstpointer a, gconstpointer b);
+
+guint thrift_int8_hash(gconstpointer v);
+gboolean thrift_int8_equal(gconstpointer a, gconstpointer b);
+
+guint thrift_int16_hash(gconstpointer v);
+gboolean thrift_int16_equal(gconstpointer a, gconstpointer b);
+
void thrift_string_free (gpointer str);
#endif /* #ifndef _THRIFT_THRIFT_H */
diff --git a/lib/c_glib/test/CMakeLists.txt b/lib/c_glib/test/CMakeLists.txt
index 61dc490..48a30d0 100644
--- a/lib/c_glib/test/CMakeLists.txt
+++ b/lib/c_glib/test/CMakeLists.txt
@@ -28,6 +28,8 @@
# Create the thrift C test library
set(testgenc_SOURCES
gen-c_glib/t_test_debug_proto_test_types.c
+ gen-c_glib/t_test_enum_test_types.c
+ gen-c_glib/t_test_enum_test_service.c
gen-c_glib/t_test_empty_service.c
gen-c_glib/t_test_inherited.c
gen-c_glib/t_test_optional_required_test_types.c
@@ -38,6 +40,8 @@
gen-c_glib/t_test_thrift_test.c
gen-c_glib/t_test_thrift_test_types.c
gen-c_glib/t_test_debug_proto_test_types.h
+ gen-c_glib/t_test_enum_test_types.h
+ gen-c_glib/t_test_enum_test_service.h
gen-c_glib/t_test_empty_service.h
gen-c_glib/t_test_inherited.h
gen-c_glib/t_test_optional_required_test_types.h
@@ -146,6 +150,14 @@
)
add_custom_command(OUTPUT
+ gen-c_glib/t_test_enum_test_types.c
+ gen-c_glib/t_test_enum_test_types.h
+ gen-c_glib/t_test_enum_test_service.c
+ gen-c_glib/t_test_enum_test_service.h
+ COMMAND ${THRIFT_COMPILER} --gen c_glib ${PROJECT_SOURCE_DIR}/test/EnumTest.thrift
+)
+
+add_custom_command(OUTPUT
gen-c_glib/t_test_optional_required_test_types.c
gen-c_glib/t_test_optional_required_test_types.h
COMMAND ${THRIFT_COMPILER} --gen c_glib ${PROJECT_SOURCE_DIR}/test/OptionalRequiredTest.thrift
diff --git a/lib/c_glib/test/Makefile.am b/lib/c_glib/test/Makefile.am
index 555380c..4d35f2a 100755
--- a/lib/c_glib/test/Makefile.am
+++ b/lib/c_glib/test/Makefile.am
@@ -169,6 +169,8 @@
nodist_libtestgenc_la_SOURCES = \
gen-c_glib/t_test_container_test_types.c \
gen-c_glib/t_test_debug_proto_test_types.c \
+ gen-c_glib/t_test_enum_test_types.c \
+ gen-c_glib/t_test_enum_test_service.c \
gen-c_glib/t_test_empty_service.c \
gen-c_glib/t_test_inherited.c \
gen-c_glib/t_test_optional_required_test_types.c \
@@ -181,6 +183,8 @@
gen-c_glib/t_test_thrift_test_types.c \
gen-c_glib/t_test_container_test_types.h \
gen-c_glib/t_test_debug_proto_test_types.h \
+ gen-c_glib/t_test_enum_test_types.h \
+ gen-c_glib/t_test_enum_test_service.h \
gen-c_glib/t_test_empty_service.h \
gen-c_glib/t_test_inherited.h \
gen-c_glib/t_test_optional_required_test_types.h \
@@ -211,6 +215,9 @@
gen-c_glib/t_test_debug_proto_test_types.c gen-c_glib/t_test_debug_proto_test_types.h gen-c_glib/t_test_empty_service.c gen-c_glib/t_test_empty_service.h gen-c_glib/t_test_inherited.c gen-c_glib/t_test_inherited.h gen-c_glib/t_test_reverse_order_service.c gen-c_glib/t_test_reverse_order_service.h gen-c_glib/t_test_service_for_exception_with_a_map.c gen-c_glib/t_test_service_for_exception_with_a_map.h gen-c_glib/t_test_srv.c gen-c_glib/t_test_srv.h: ../../../test/DebugProtoTest.thrift
$(THRIFT) --gen c_glib $<
+gen-c_glib/t_test_enum_test_types.c gen-c_glib/t_test_enum_test_types.h gen-c_glib/t_test_enum_test_service.c gen-c_glib/t_test_enum_test_service.h : ../../../test/EnumTest.thrift
+ $(THRIFT) --gen c_glib $<
+
gen-c_glib/t_test_optional_required_test_types.c gen-c_glib/t_test_optional_required_test_types.h: ../../../test/OptionalRequiredTest.thrift
$(THRIFT) --gen c_glib $<
diff --git a/lib/c_glib/test/testoptionalrequired.c b/lib/c_glib/test/testoptionalrequired.c
index ae0c3d2..cfc96a2 100755
--- a/lib/c_glib/test/testoptionalrequired.c
+++ b/lib/c_glib/test/testoptionalrequired.c
@@ -187,6 +187,26 @@
g_object_unref (t3);
}
+static void
+test_non_set_binary (void)
+{
+ TTestBinaries *b1 = NULL;
+ TTestBinaries *b2 = NULL;
+ GError *error = NULL;
+
+ b1 = g_object_new (T_TEST_TYPE_BINARIES, NULL);
+ b2 = g_object_new (T_TEST_TYPE_BINARIES, NULL);
+
+ write_to_read (THRIFT_STRUCT (b1), THRIFT_STRUCT (b2), NULL, &error);
+ g_assert(!error);
+ write_to_read (THRIFT_STRUCT (b2), THRIFT_STRUCT (b1), NULL, &error);
+ g_assert(!error);
+ // OK. No segfault
+
+ g_object_unref (b1);
+ g_object_unref (b2);
+}
+
int
main(int argc, char *argv[])
{
@@ -202,6 +222,7 @@
g_test_add_func ("/testoptionalrequired/Tricky2", test_tricky2);
g_test_add_func ("/testoptionalrequired/Tricky3", test_tricky3);
g_test_add_func ("/testoptionalrequired/Tricky4", test_tricky4);
+ g_test_add_func ("/testoptionalrequired/Binary", test_non_set_binary);
return g_test_run ();
}
diff --git a/lib/c_glib/test/testserialization.c b/lib/c_glib/test/testserialization.c
index 0ece2ad..9fc6357 100644
--- a/lib/c_glib/test/testserialization.c
+++ b/lib/c_glib/test/testserialization.c
@@ -3,6 +3,53 @@
#include <thrift/c_glib/transport/thrift_memory_buffer.h>
#include <thrift/c_glib/transport/thrift_transport.h>
#include "gen-c_glib/t_test_debug_proto_test_types.h"
+#include "gen-c_glib/t_test_enum_test_types.h"
+
+static void enum_constants_read_write() {
+ GError* error = NULL;
+ ThriftTransport* transport
+ = THRIFT_TRANSPORT(g_object_new(THRIFT_TYPE_MEMORY_BUFFER, "buf_size", 1024, NULL));
+ ThriftProtocol* protocol
+ = THRIFT_PROTOCOL(g_object_new(THRIFT_TYPE_BINARY_PROTOCOL, "transport", transport, NULL));
+ TTestEnumTestStruct* src = T_TEST_ENUM_TEST;
+ TTestEnumTestStruct* dst = g_object_new(T_TEST_TYPE_ENUM_TEST_STRUCT, NULL);
+ TTestEnumTestStructClass* cls = T_TEST_ENUM_TEST_STRUCT_GET_CLASS(src);
+
+ int write_len = THRIFT_STRUCT_CLASS(cls)->write(THRIFT_STRUCT(src), protocol, &error);
+ g_assert(!error);
+ g_assert(write_len > 0);
+
+ int read_len = THRIFT_STRUCT_CLASS(cls)->read(THRIFT_STRUCT(dst), protocol, &error);
+ g_assert(!error);
+ g_assert_cmpint(write_len, ==, read_len);
+
+ g_object_unref(dst);
+ g_object_unref(protocol);
+ g_object_unref(transport);
+}
+
+static void struct_constants_read_write() {
+ GError* error = NULL;
+ ThriftTransport* transport
+ = THRIFT_TRANSPORT(g_object_new(THRIFT_TYPE_MEMORY_BUFFER, "buf_size", 4096, NULL));
+ ThriftProtocol* protocol
+ = THRIFT_PROTOCOL(g_object_new(THRIFT_TYPE_BINARY_PROTOCOL, "transport", transport, NULL));
+ TTestCompactProtoTestStruct* src = T_TEST_COMPACT_TEST;
+ TTestCompactProtoTestStruct* dst = g_object_new(T_TEST_TYPE_COMPACT_PROTO_TEST_STRUCT, NULL);
+ TTestCompactProtoTestStructClass* cls = T_TEST_COMPACT_PROTO_TEST_STRUCT_GET_CLASS(src);
+
+ int write_len = THRIFT_STRUCT_CLASS(cls)->write(THRIFT_STRUCT(src), protocol, &error);
+ g_assert(!error);
+ g_assert(write_len > 0);
+
+ int read_len = THRIFT_STRUCT_CLASS(cls)->read(THRIFT_STRUCT(dst), protocol, &error);
+ g_assert(!error);
+ g_assert_cmpint(write_len, ==, read_len);
+
+ g_object_unref(dst);
+ g_object_unref(protocol);
+ g_object_unref(transport);
+}
static void struct_read_write_length_should_equal() {
GError* error = NULL;
@@ -36,5 +83,7 @@
g_test_add_func("/testserialization/StructReadWriteLengthShouldEqual",
struct_read_write_length_should_equal);
+ g_test_add_func("/testserialization/StructConstants", struct_constants_read_write);
+ g_test_add_func("/testserialization/EnumConstants", enum_constants_read_write);
return g_test_run();
}
diff --git a/test/EnumTest.thrift b/test/EnumTest.thrift
index 6201923..17af408 100644
--- a/test/EnumTest.thrift
+++ b/test/EnumTest.thrift
@@ -21,6 +21,8 @@
* details.
*/
+namespace c_glib TTest
+
enum MyEnum1 {
ME1_0 = 0,
ME1_1 = 1,
@@ -70,3 +72,38 @@
3: MyEnum3 me3_d1 = MyEnum3.ME3_D1
}
+struct EnumTestStruct {
+ 1: MyEnum3 a_enum;
+ 2: list<MyEnum3> enum_list;
+ 3: set<MyEnum3> enum_set;
+ 4: map<MyEnum3, MyEnum3> enum_enum_map;
+ // collections as keys
+ 44: map<list<MyEnum3> (python.immutable = ""), MyEnum3> list_enum_map;
+ 45: map<set<MyEnum3> (python.immutable = ""), MyEnum3> set_enum_map;
+ 46: map<map<MyEnum3,MyEnum3> (python.immutable = ""), MyEnum3> map_enum_map;
+ // collections as values
+ 47: map<MyEnum3, map<MyEnum3, MyEnum3>> enum_map_map;
+ 48: map<MyEnum3, set<MyEnum3>> enum_set_map;
+ 49: map<MyEnum3, list<MyEnum3>> enum_list_map;
+}
+
+const EnumTestStruct ENUM_TEST = {
+ 'a_enum': MyEnum3.ME3_D1,
+ 'enum_list': [MyEnum3.ME3_D1, MyEnum3.ME3_0, MyEnum3.ME3_N2],
+ 'enum_set': [MyEnum3.ME3_D1, MyEnum3.ME3_N1],
+ 'enum_enum_map': {MyEnum3.ME3_D1: MyEnum3.ME3_0, MyEnum3.ME3_0: MyEnum3.ME3_D1},
+ 'list_enum_map': {[MyEnum3.ME3_D1, MyEnum3.ME3_0]: MyEnum3.ME3_0, [MyEnum3.ME3_D1]: MyEnum3.ME3_0, [MyEnum3.ME3_0]: MyEnum3.ME3_D1},
+ 'set_enum_map': {[MyEnum3.ME3_D1, MyEnum3.ME3_0]: MyEnum3.ME3_0, [MyEnum3.ME3_D1]: MyEnum3.ME3_0},
+ 'map_enum_map': {{MyEnum3.ME3_N1: MyEnum3.ME3_10}: MyEnum3.ME3_1},
+ 'enum_map_map': {MyEnum3.ME3_N1: {MyEnum3.ME3_D1: MyEnum3.ME3_D1}},
+ 'enum_set_map': {MyEnum3.ME3_N2: [MyEnum3.ME3_D1, MyEnum3.ME3_N1], MyEnum3.ME3_10: [MyEnum3.ME3_D1, MyEnum3.ME3_N1]},
+ 'enum_list_map': {MyEnum3.ME3_D1: [MyEnum3.ME3_10], MyEnum3.ME3_0: [MyEnum3.ME3_9, MyEnum3.ME3_10]},
+}
+
+service EnumTestService {
+ MyEnum3 testEnum(1: MyEnum3 enum1),
+ list<MyEnum3> testEnumList(1: list<MyEnum3> enum1),
+ set<MyEnum3> testEnumSet(1: set<MyEnum3> enum1),
+ map<MyEnum3, MyEnum3> testEnumMap(1: map<MyEnum3, MyEnum3> enum1),
+ EnumTestStruct testEnumStruct(1: EnumTestStruct enum1),
+}
diff --git a/test/OptionalRequiredTest.thrift b/test/OptionalRequiredTest.thrift
index dcdd0f2..a608898 100644
--- a/test/OptionalRequiredTest.thrift
+++ b/test/OptionalRequiredTest.thrift
@@ -80,3 +80,9 @@
5: required binary req_bin;
6: optional binary opt_bin;
}
+
+struct Binaries {
+ 4: binary bin;
+ 5: required binary req_bin;
+ 6: optional binary opt_bin;
+}
diff --git a/test/c_glib/src/thrift_test_handler.c b/test/c_glib/src/thrift_test_handler.c
index 69ddbc1..86b29dd 100644
--- a/test/c_glib/src/thrift_test_handler.c
+++ b/test/c_glib/src/thrift_test_handler.c
@@ -144,6 +144,7 @@
THRIFT_UNUSED_VAR (error);
printf ("testBinary()\n"); // TODO: hex output
+ g_byte_array_ref(thing);
*_return = thing;
return TRUE;