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;
  }

8. 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