[THRIFT-1266] generated C code for iterating over nested maps is wrong

• Type: Bug
• Status: Closed
• Priority: Critical
• Resolution: Fixed
• Affects Version/s: 0.8
• Fix Version/s: 0.9.2
• Component/s: C glib - Compiler
• Labels: c_glib cassandra

Environment

Revision 1158683 of http://svn.apache.org/repos/asf/thrift/trunk.

• Assignee: Simon South
• Reporter: Aleksandrs Saveljevs
• Created: 17/Aug/11 13:38
• Updated: 05/Nov/14 04:48
• Resolved: 03/Sep/14 22:18

Description

We are working with Cassandra API in C generated by Thrift and have noticed a bug in the generated code for cassandra_client_send_batch_mutate().

Full code that Thrift generates for this function is attached, but here is the specification for Cassandra's batch_mutate method:

/**
  Mutate many columns or super columns for many row keys. See also: Mutation.

  mutation_map maps key to column family to a list of Mutation objects to take place at that scope.
**/
void batch_mutate(1:required map<binary, map<string, list<Mutation>>> mutation_map,
                  2:required ConsistencyLevel consistency_level=ConsistencyLevel.ONE)
     throws (1:InvalidRequestException ire, 2:UnavailableException ue, 3:TimedOutException te),

If we now look at the generated code, we will notice the following fragment:

GPtrArray * value;
g_hash_table_foreach ((GHashTable *)  value, thrift_hash_table_get_keys, &key_list); /* LINE A */

We can see that in line A it uses the variable "value" as GHashTable, even though the GHashTable "value" was shadowed by GPtrArray "value" a line before.

Similarly, we can see another fragment below that one, where one instance of variable "value" shadows another instance:

value = (GPtrArray *) g_hash_table_lookup (((GHashTable *)  value), (gpointer) key); /* LINE B */

We have worked around the bug in our particular case by renaming one of the "value" variables to "value2" (see "svn di -c 21176 svn://svn.zabbix.com/branches/dev/ZBXNEXT-844/src/libs/zbxcassa/cassandra.c@21176" for a diff), but it would be nice to fix it in Thrift, too.

Issue Links

• is required by: THRIFT-2690 c_glib: Incorrect behaviour when serializing a map with typedef'd keys