Nitrokey / libnitrokey

Communicate with Nitrokey devices in a clean and easy manner
https://nitrokey.com/
GNU Lesser General Public License v3.0
65 stars 34 forks source link

LongOperationInProgressException cannot be detected using the C API #170

Open robinkrahl opened 4 years ago

robinkrahl commented 4 years ago

The C API translates the C++ exceptions to integer values (see NK_C_API.cc:get_with_status). For instances of CommandFailedException, the last_command_status field is used for the conversion. This field is typically set to a value of the stick10::command_status enum. But LongOperationInProgressException, which is a subclass of CommandFailedException, uses stick10::device_status::busy (= 1) instead. This overlaps with the wrong_CRC variant of the command_status enum, making it impossible to distinguish the two errors as a user of the C API.

Possible solutions:

  1. Add a new value to the stick10::command_status enum and use it for LongOperationInProgressException.
  2. Add a special case for the LongOperationInProgressException to the get_with_status function and return a unique ID for it. (I don’t really like this option because it makes the error handling code more complicated.)
  3. Let LongOperationInProgressException inherit from DeviceCommunicationException instead of CommandFailedException and define a new ID.
robinkrahl commented 4 years ago

If you let me know which option you prefer I can create a pull request with an implementation. I’d personally prefer option 3.

szszszsz commented 4 years ago

Yes, option 3 makes sense. Thank you!