[
https://issues.apache.org/jira/browse/THRIFT-1266?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Simon South updated THRIFT-1266:
--------------------------------
Attachment:
thrift-1266-c_glib-variable-collision-serializing-nested-maps.patch
In the "better late than never" category:
I've added a patch that fixes this problem. It modifies
{{t_c_glib_generator::generate_serialize_container}} so variables representing
hash-table keys and values are named with a unique suffix, eliminating the
possibility of a collision. (This is the same solution used in
{{generate_const_initializer}}.)
No specific test case, but the server half of the integration test suite for
c_glib will trigger this bug without the patch applied.
With this fix, the relevant parts of the code generated for Cassandra 1.1.1 now
look like this:
{noformat}
GPtrArray * val75 = NULL;
g_hash_table_foreach ((GHashTable *) val73, thrift_hash_table_get_keys,
&key_list);
...
val75 = (GPtrArray *) g_hash_table_lookup (((GHashTable *) val73), (gpointer)
key74);
{noformat}
> generated C code for iterating over nested maps is wrong
> --------------------------------------------------------
>
> Key: THRIFT-1266
> URL: https://issues.apache.org/jira/browse/THRIFT-1266
> Project: Thrift
> Issue Type: Bug
> Components: C glib - Compiler
> Affects Versions: 0.8
> Environment: Revision 1158683 of
> http://svn.apache.org/repos/asf/thrift/trunk.
> Reporter: Aleksandrs Saveljevs
> Assignee: Simon South
> Priority: Critical
> Labels: c_glib, cassandra
> Attachments: batch_mutate.c,
> thrift-1266-c_glib-variable-collision-serializing-nested-maps.patch
>
>
> 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:
> {code}/**
> 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),
> {code}
> If we now look at the generated code, we will notice the following fragment:
> {code}GPtrArray * value;
> g_hash_table_foreach ((GHashTable *) value, thrift_hash_table_get_keys,
> &key_list); /* LINE A */
> {code}
> 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:
> {code}
> value = (GPtrArray *) g_hash_table_lookup (((GHashTable *) value),
> (gpointer) key); /* LINE B */
> {code}
> 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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
