THRIFT-3260 multiple warnings in c_glib tutorial
Client: c_glib
Simon South <ssouth@simonsouth.com>
This closes #573
Here are additional changes that should really and truly resolve all the warnings generated when building the c_glib tutorial:
Compiler:
- Do not output a trailing comma in exception-enum definitions.
- Move variable declarations to avoid mixing declarations and code in generated code.
- Improve the readability of affected code blocks (and rely on indent_up and indent_down for indentation).
Library
- Use only C-style comments in headers included by clients.
Tutorial
- Move THRIFT_UNUSED_VAR calls to avoid mixing declarations and code.
diff --git a/compiler/cpp/src/generate/t_c_glib_generator.cc b/compiler/cpp/src/generate/t_c_glib_generator.cc
index f8431cd..4fd6412 100644
--- a/compiler/cpp/src/generate/t_c_glib_generator.cc
+++ b/compiler/cpp/src/generate/t_c_glib_generator.cc
@@ -485,11 +485,20 @@
generate_object(tstruct);
- f_types_ << "/* exception */" << endl << "typedef enum" << endl << "{" << endl << " "
- << this->nspace_uc << name_uc << "_ERROR_CODE," << endl << "} " << this->nspace << name
- << "Error;" << endl << endl << "GQuark " << this->nspace_lc << name_lc
- << "_error_quark (void);" << endl << "#define " << this->nspace_uc << name_uc
- << "_ERROR (" << this->nspace_lc << name_lc << "_error_quark())" << endl << endl << endl;
+ f_types_ << "/* exception */" << endl
+ << "typedef enum" << endl
+ << "{" << endl;
+ indent_up();
+ f_types_ << indent() << this->nspace_uc << name_uc << "_ERROR_CODE" << endl;
+ indent_down();
+ f_types_ << "} " << this->nspace << name << "Error;" << endl
+ << endl
+ << "GQuark " << this->nspace_lc << name_lc
+ << "_error_quark (void);" << endl
+ << "#define " << this->nspace_uc << name_uc << "_ERROR ("
+ << this->nspace_lc << name_lc << "_error_quark())" << endl
+ << endl
+ << endl;
f_types_impl_ << "/* define the GError domain for exceptions */" << endl << "#define "
<< this->nspace_uc << name_uc << "_ERROR_DOMAIN \"" << this->nspace_lc << name_lc
@@ -1476,42 +1485,74 @@
indent(f_service_) << function_signature(&recv_function) << endl;
scope_up(f_service_);
- f_service_ << endl << indent() << "gint32 rseqid;" << endl << indent()
- << "gchar * fname = NULL;" << endl << indent() << "ThriftMessageType mtype;"
- << endl << indent() << "ThriftProtocol * protocol = " << this->nspace_uc
- << base_service_name_uc << "_CLIENT (iface)->input_protocol;" << endl << endl
+ f_service_ << indent() << "gint32 rseqid;" << endl
+ << indent() << "gchar * fname = NULL;" << endl
+ << indent() << "ThriftMessageType mtype;" << endl
+ << indent() << "ThriftProtocol * protocol = "
+ << this->nspace_uc << base_service_name_uc
+ << "_CLIENT (iface)->input_protocol;" << endl
+ << indent() << "ThriftApplicationException *xception;" << endl
+ << endl
<< indent() << "if (thrift_protocol_read_message_begin "
- << "(protocol, &fname, &mtype, &rseqid, error) < 0)" << endl << indent() << "{"
- << endl << indent() << " if (fname) g_free (fname);" << endl << indent()
- << " return FALSE;" << endl << indent() << "}" << endl << endl << indent()
- << "if (mtype == T_EXCEPTION) {" << endl << indent()
- << " if (fname) g_free (fname);" << endl << indent()
- << " ThriftApplicationException *xception = g_object_new "
- "(THRIFT_TYPE_APPLICATION_EXCEPTION, NULL);" << endl << indent()
- << " thrift_struct_read (THRIFT_STRUCT (xception), protocol, NULL);" << endl
- << indent() << " thrift_protocol_read_message_end (protocol, NULL);" << endl
- << indent() << " thrift_transport_read_end (protocol->transport, NULL);" << endl
- << indent() << " g_set_error (error, THRIFT_APPLICATION_EXCEPTION_ERROR, "
- "xception->type, \"application error: %s\", xception->message);"
- << endl << indent() << " g_object_unref (xception);" << endl << indent()
- << " return FALSE;" << endl << indent() << "} else if (mtype != T_REPLY) {"
- << endl << indent() << " if (fname) g_free (fname);" << endl << indent()
- << " thrift_protocol_skip (protocol, T_STRUCT, NULL);" << endl << indent()
- << " thrift_protocol_read_message_end (protocol, NULL);" << endl << indent()
- << " thrift_transport_read_end (protocol->transport, NULL);" << endl << indent()
- << " g_set_error (error, THRIFT_APPLICATION_EXCEPTION_ERROR, "
- "THRIFT_APPLICATION_EXCEPTION_ERROR_INVALID_MESSAGE_TYPE, \"invalid message "
- "type %d, expected T_REPLY\", mtype);" << endl << indent() << " return FALSE;"
- << endl << indent() << "} else if (strncmp (fname, \"" << name << "\", "
- << name.length() << ") != 0) {" << endl << indent()
- << " thrift_protocol_skip (protocol, T_STRUCT, NULL);" << endl << indent()
- << " thrift_protocol_read_message_end (protocol, error);" << endl << indent()
- << " thrift_transport_read_end (protocol->transport, error);" << endl << indent()
- << " g_set_error (error, THRIFT_APPLICATION_EXCEPTION_ERROR, "
- "THRIFT_APPLICATION_EXCEPTION_ERROR_WRONG_METHOD_NAME, \"wrong method name %s, "
- "expected " << name << "\", fname);" << endl << indent()
- << " if (fname) g_free (fname);" << endl << indent() << " return FALSE;" << endl
- << indent() << "}" << endl << indent() << "if (fname) g_free (fname);" << endl
+ "(protocol, &fname, &mtype, &rseqid, error) < 0) {" << endl;
+ indent_up();
+ f_service_ << indent() << "if (fname) g_free (fname);" << endl
+ << indent() << "return FALSE;" << endl;
+ indent_down();
+ f_service_ << indent() << "}" << endl
+ << endl
+ << indent() << "if (mtype == T_EXCEPTION) {" << endl;
+ indent_up();
+ f_service_ << indent() << "if (fname) g_free (fname);" << endl
+ << indent() << "xception = g_object_new "
+ "(THRIFT_TYPE_APPLICATION_EXCEPTION, NULL);" << endl
+ << indent() << "thrift_struct_read (THRIFT_STRUCT (xception), "
+ "protocol, NULL);" << endl
+ << indent() << "thrift_protocol_read_message_end "
+ "(protocol, NULL);" << endl
+ << indent() << "thrift_transport_read_end "
+ "(protocol->transport, NULL);" << endl
+ << indent() << "g_set_error (error, "
+ "THRIFT_APPLICATION_EXCEPTION_ERROR,xception->type, "
+ "\"application error: %s\", xception->message);" << endl
+ << indent() << "g_object_unref (xception);" << endl
+ << indent() << "return FALSE;" << endl;
+ indent_down();
+ f_service_ << indent() << "} else if (mtype != T_REPLY) {" << endl;
+ indent_up();
+ f_service_ << indent() << "if (fname) g_free (fname);" << endl
+ << indent() << "thrift_protocol_skip (protocol, T_STRUCT, "
+ "NULL);" << endl
+ << indent() << "thrift_protocol_read_message_end (protocol, "
+ "NULL);" << endl
+ << indent() << "thrift_transport_read_end ("
+ "protocol->transport, NULL);" << endl
+ << indent() << "g_set_error (error, "
+ "THRIFT_APPLICATION_EXCEPTION_ERROR, "
+ "THRIFT_APPLICATION_EXCEPTION_ERROR_INVALID_MESSAGE_TYPE, "
+ "\"invalid message type %d, expected T_REPLY\", mtype);"
+ << endl
+ << indent() << "return FALSE;" << endl;
+ indent_down();
+ f_service_ << indent() << "} else if (strncmp (fname, \"" << name
+ << "\", " << name.length() << ") != 0) {" << endl;
+ indent_up();
+ f_service_ << indent() << "thrift_protocol_skip (protocol, T_STRUCT, "
+ "NULL);" << endl
+ << indent() << "thrift_protocol_read_message_end (protocol,"
+ "error);" << endl
+ << indent() << "thrift_transport_read_end ("
+ "protocol->transport, error);" << endl
+ << indent() << "g_set_error (error, "
+ "THRIFT_APPLICATION_EXCEPTION_ERROR, "
+ "THRIFT_APPLICATION_EXCEPTION_ERROR_WRONG_METHOD_NAME, "
+ "\"wrong method name %s, expected " << name
+ << "\", fname);" << endl
+ << indent() << "if (fname) g_free (fname);" << endl
+ << indent() << "return FALSE;" << endl;
+ indent_down();
+ f_service_ << indent() << "}" << endl
+ << indent() << "if (fname) g_free (fname);" << endl
<< endl;
t_struct* xs = (*f_iter)->get_xceptions();
@@ -2075,9 +2116,10 @@
<< "ThriftProtocol *output_protocol," << endl << args_indent << "GError **error)"
<< endl;
scope_up(f_service_);
- f_service_ << indent() << "gboolean result = TRUE;" << endl << indent()
- << "ThriftTransport * transport;" << endl << indent()
- << args_class_name + " * args =" << endl;
+ f_service_ << indent() << "gboolean result = TRUE;" << endl
+ << indent() << "ThriftTransport * transport;" << endl
+ << indent() << "ThriftApplicationException *xception;" << endl
+ << indent() << args_class_name + " * args =" << endl;
indent_up();
f_service_ << indent() << "g_object_new (" << args_class_type << ", NULL);" << endl << endl;
indent_down();
@@ -2269,7 +2311,7 @@
<< (*function_iter)->get_name() << " implementation returned FALSE \"" << endl
<< indent() << string(11, ' ') << "\"but did not set an error\");" << endl << endl;
indent_down();
- f_service_ << indent() << "ThriftApplicationException *xception =" << endl;
+ f_service_ << indent() << "xception =" << endl;
indent_up();
f_service_ << indent() << "g_object_new (THRIFT_TYPE_APPLICATION_EXCEPTION," << endl;
args_indent = indent() + string(14, ' ');
diff --git a/lib/c_glib/src/thrift/c_glib/thrift.h b/lib/c_glib/src/thrift/c_glib/thrift.h
index 236b46f..858ad86 100644
--- a/lib/c_glib/src/thrift/c_glib/thrift.h
+++ b/lib/c_glib/src/thrift/c_glib/thrift.h
@@ -35,4 +35,4 @@
gpointer user_data);
void thrift_string_free (gpointer str);
-#endif // #ifndef _THRIFT_THRIFT_H
+#endif /* #ifndef _THRIFT_THRIFT_H */
diff --git a/tutorial/c_glib/c_glib_server.c b/tutorial/c_glib/c_glib_server.c
index 3aa99ec..1b7f6c4 100644
--- a/tutorial/c_glib/c_glib_server.c
+++ b/tutorial/c_glib/c_glib_server.c
@@ -173,8 +173,6 @@
InvalidOperation **ouch,
GError **error)
{
- THRIFT_UNUSED_VAR (error);
-
TutorialCalculatorHandler *self;
gint *log_key;
@@ -186,6 +184,8 @@
Operation op;
gboolean result = TRUE;
+ THRIFT_UNUSED_VAR (error);
+
g_return_val_if_fail (IS_TUTORIAL_CALCULATOR_HANDLER (iface),
FALSE);
self = TUTORIAL_CALCULATOR_HANDLER (iface);
@@ -300,14 +300,14 @@
const gint32 key32,
GError **error)
{
- THRIFT_UNUSED_VAR (error);
-
gint key = (gint)key32;
TutorialCalculatorHandler *self;
SharedStruct *log_struct;
gint log_key;
gchar *log_value;
+ THRIFT_UNUSED_VAR (error);
+
g_return_val_if_fail (IS_TUTORIAL_CALCULATOR_HANDLER (iface),
FALSE);
self = TUTORIAL_CALCULATOR_HANDLER (iface);