Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-1266

generated C code for iterating over nested maps is wrong

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    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.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            simonsouth Simon South
            asaveljevs Aleksandrs Saveljevs
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment