DMTF / spdm-emu

BSD 3-Clause "New" or "Revised" License
32 stars 44 forks source link

return status of spdm_load_negotiated_state() is not checked #23

Open rw8896 opened 3 years ago

rw8896 commented 3 years ago

1) Take responder for example (same for the requester), https://github.com/DMTF/spdm-emu/blob/ecabfed2837b1dfc8f4af96f9c829cdf6b12a03c/spdm_emu/spdm_responder_emu/spdm_responder.c#L139

Current code didn't check return status of spdm_load_negotiated_state but logically spdm_load_negotiated_state could fail with some values left in spdm_context.

Maybe we could call spdm_load_negotiated_state immediately after spdm_init_context and re-init the context in case of load failures.

    spdm_init_context(spdm_context);
    if (m_load_state_file_name != NULL) {
        if (spdm_load_negotiated_state(spdm_context, FALSE) != RETURN_SUCCESS) {
            /* re-init spdm context */
            spdm_init_context(spdm_context);
        }
    }
    spdm_register_device_io_func(spdm_context, spdm_device_send_message,
                     spdm_device_receive_message);

2) spdm_clear_negotiated_state has nothing to do with spdm_context. Suggest to take out the input parameter. return_status spdm_clear_negotiated_state(IN void *spdm_context)

jyao1 commented 2 years ago
  1. Agree.
  2. It may be used for future. I think we need keep it for consistency reason. (in case the function need get some parameter from spdm_context.