The type of federate_id is used mixing uint16_t and int, and int32_t.

[Jakio815]

The use of federate_id's type is being mixed.

1. Definition of federate_id on the federate side. It is defined as int.

   ## util.c
   /**
   * The ID of this federate. For a non-federated execution, this will be -1.
   * For a federated execution, it will be assigned in the generated code.
   */
   int _lf_my_fed_id = -1;

2. The federate sends the federate_id on MSG_TYPE_FED_IDS. It is sending it as unit16_t.

   buffer[0] = MSG_TYPE_FED_IDS;
   // Next send the federate ID.
   if (_lf_my_fed_id > UINT16_MAX) {
     lf_print_error_and_exit("Too many federates! More than %d.", UINT16_MAX);
   }
   encode_uint16((uint16_t)_lf_my_fed_id, &buffer[1]);

3. The RTI receives the federate_id. Receives as uint16_t.

   // Received federate ID.
   fed_id = extract_uint16(buffer + 1);

4. The function which received the federate_id, returns it as int32_t.

   int32_t receive_and_check_fed_id_message(){
   ...
   return (int32_t)fed_id;
   }

5. RTI mixes up using it.

   // The first message from the federate should contain its ID and the federation ID.
   int32_t fed_id = receive_and_check_fed_id_message(&socket_id, (struct sockaddr_in*)&client_fd);
   if (fed_id >= 0 && socket_id >= 0 && receive_connection_information(&socket_id, (uint16_t)fed_id) &&
       receive_udp_message_and_set_up_clock_sync(&socket_id, (uint16_t)fed_id)) {

6. Later on clock synchronization. Federate sends MSG_TYPE_CLOCK_SYNC_T3 with its federate_id as int32_t. However the buffer size is defined as 1 +sizeof(int) which can be critical for 16-bit systems.

   // Reply will have the federate ID as a payload.
   unsigned char reply_buffer[1 + sizeof(int)];
   reply_buffer[0] = MSG_TYPE_CLOCK_SYNC_T3;
   encode_int32(_lf_my_fed_id, &(reply_buffer[1]));
   
   // Write the reply to the socket.
   LF_PRINT_DEBUG("Sending T3 message to RTI.");
   if (write_to_socket(socket, 1 + sizeof(int), reply_buffer)) {
   lf_print_error("Clock sync: Failed to send T3 message to RTI.");
   return -1;
   }

7. RTI receives this message, and compares the federate_id. It extracts the received federate_id as int32_t. However, it compares with the fed->enclave.id, which is uint16_t

   ## rti_remote.c
         if (buffer[0] == MSG_TYPE_CLOCK_SYNC_T3) {
           int32_t fed_id_2 = extract_int32(&(buffer[1]));
           // Check that this message came from the correct federate.
           if (fed_id_2 != fed->enclave.id) {

   ## rti_common.h
   typedef struct scheduling_node_t {
   uint16_t id; 
   ...
   }

So, is this intended? Or is it just a bug. It seems it didn't really matter because we are not using federates more than 2^16. However, it seems that the part int _lf_my_fed_id = -1; that defines the federate_id as int seems dangerous, when considering using 16-bit systems. Sending MSG_TYPE_CLOCK_SYNC_T3 on 16-bit systems will only send 1+2 bytes, and the RTI will attempt to read 1 + 4bytes, which will block on the read().

[edwardalee]

It does look like the clock sync code should be sending and receiving uint16_t. This should be a small fix. Do you want to issue a PR?

The use of an int32_t for recording the ID, I think, is so that -1 can be used to indicate an error or unknown. I don't see why this would create problems. Can you clarify?

I don't understand the question about 16-bit systems. Even a 16-bit system should respect these data types.

[Jakio815]

Oh, now I see why it's recording it as int32_t.

The point I was concerned is on this part.

// util.c
/**
 * The ID of this federate. For a non-federated execution, this will be -1.
 * For a federated execution, it will be assigned in the generated code.
 */
int _lf_my_fed_id = -1;

It's just saving it as an int type not uint16_t or int32_t. In this case, wouldn't the int type be saved in int16_t in 16 byte systems?

[edwardalee]

Oh, possibly, yes. This would be better declared as an int32_t.

[Jakio815]

I made a PR on this issue. #453