Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LongOperationInProgressException cannot be detected using the C API #170

Open
robinkrahl opened this issue Jan 14, 2020 · 2 comments · May be fixed by #177
Open

LongOperationInProgressException cannot be detected using the C API #170

robinkrahl opened this issue Jan 14, 2020 · 2 comments · May be fixed by #177
Milestone

Comments

@robinkrahl
Copy link
Contributor

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 robinkrahl changed the title LongOperationExceptionInProgress cannot be detected using the C API LongOperationInProgressException cannot be detected using the C API Jan 14, 2020
@szszszsz szszszsz added this to the v3.6 milestone Jan 15, 2020
@robinkrahl
Copy link
Contributor Author

robinkrahl commented Apr 2, 2020

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
Copy link
Member

szszszsz commented Apr 2, 2020

Yes, option 3 makes sense. Thank you!

robinkrahl added a commit to robinkrahl/libnitrokey that referenced this issue Apr 2, 2020
This patch changes LongOperationInProgressException to derive from
DeviceCommunicationException.  This makes it easier to use the C API as
the exception now has a unique numerical identifier.

Fixes Nitrokey#170.
@robinkrahl robinkrahl linked a pull request Apr 2, 2020 that will close this issue
@szszszsz szszszsz modified the milestones: v3.7, v3.8 Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants