[jira] [Comment Edited] (THRIFT-1266) generated C code for iterating over nested maps is wrong

["Aleksandrs Saveljevs (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-1266?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13292639#comment-13292639 ] 

Aleksandrs Saveljevs edited comment on THRIFT-1266 at 6/11/12 4:37 AM:
-----------------------------------------------------------------------

Unfortunately, neither a reduced test case, nor a patch would be possible, because none of us are experts on Thrift. We are simply Cassandra users and, as such, the bug should have been reported by Cassandra developers.

However, I have just tried Thrift 0.8.0 on Cassandra 1.1.1. It generates the same code for batch_mutate() method, so it seems the problem has not been fixed yet and is easily reproducible.

In order to reproduce the problem using Cassandra, you can do the following:
(1) download Cassandra source from http://cassandra.apache.org/download/ ;
(2) from Cassandra source root, run Thrift using "thrift --gen c_glib interface/cassandra.thrift";
(3) open "gen-c_glib/cassandra.c" and observe that "value" variable is incorrectly shadowed inside cassandra_client_send_batch_mutate().
                
      was (Author: asaveljevs):
    Unfortunately, neither a reduced test case, nor a patch would be possible, because none of us are experts on Thrift. We are simply Cassandra users and, as such, the bug should have been reported by Cassandra developers.

However, I have just tried Thrift 0.8.0 on Cassandra 1.1.1. It generates the same code for batch_mutate() method, so it seems the problem has not been fixed yet and is easily reproducible.

In order to reproduce the problem using Cassandra, you can do the following:
(1) download Cassandra source from http://cassandra.apache.org/download/ ;
(2) from Cassandra source root, run Thrift using "thrift --gen c_glib interface/cassandra.thrift";
(3) open "gen-c_glib/cassandra.c" and observe that "value" variable is shadowed inside cassandra_client_send_batch_mutate().
                  
> 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
>            Priority: Critical
>              Labels: c_glib, cassandra
>         Attachments: batch_mutate.c
>
>
> 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 is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira