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

Inconsistent int signedness for printf

Attach filesAttach ScreenshotAdd voteVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    Description

      The Protocol's Type Message ENUM is inconsistent with "printf" for c_glib and might be in some cases for C++.
      The issue can be seen at compiling with the flag " -Wformat-signedness".

      Looks like all the printf use "%d" signed-i32 - in order to keep it consistent. It be better to have enum of type int32_t or unsigned with printf to be changed to "%u" - or further more reduce it to uint8_t (unless Message Type needs to be reserved for growing in future)

       

      The printf cases can be found at https://github.com/apache/thrift/search?q="invalid+message+type+%25d%2C"&type=code

      The corresponding ENUMs are the:

      C++:
      https://github.com/apache/thrift/blob/66d897667c451ef6560d89b979b7001c57a3eda6/lib/cpp/src/thrift/protocol/TEnum.h#L57

      enum TMessageType {
        T_CALL       = 1,
        T_REPLY      = 2,
        T_EXCEPTION  = 3,
        T_ONEWAY     = 4
      };
      

      c_glib: 
      https://github.com/apache/thrift/blob/66d897667c451ef6560d89b979b7001c57a3eda6/lib/c_glib/src/thrift/c_glib/protocol/thrift_protocol.h#L63

      typedef enum {
        T_CALL      = 1,
        T_REPLY     = 2,
        T_EXCEPTION = 3,
        T_ONEWAY    = 4
      } ThriftMessageType;
      

       

       

      I'm willing to make a pull-request, Just I would like first to know if there are any preferences/considerations on the subject.

      Attachments

        Activity

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

          People

            Unassigned Unassigned
            kashirin.alex Kashirin Alex

            Dates

              Created:
              Updated:

              Slack

                Issue deployment