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

Fix enums in pxd files because macro creates enums that are _1_ base… #308

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BertKleewein
Copy link
Member

…d instead of 0 based.

I haven't witnessed any bad behavior because of this except for confusing logs, and maybe some unnecessary state transitions.

I found this while investigating a CBS auth failure that turned out to be a problem with our service. I had some momentary confusion because, at one point, the C code called on_cbs_open_complete passing CBS_OPEN_OK, which the C code has defined as 1. Then, inside cbs.pyx, the code assumed that 1 was CBS_OPEN_ERROR, so it set self.state to AUTH_STATE_FAILURE.

Fortunately, our service finishes the put_token quickly enough that the CBS operation can complete successfully, so no harm was actually done.

I have mixed feelings about this change. On the one hand, the code is very clearly incorrect, but, on the other hand, the code appears to work just fine with the incorrect values. I can't say if this change would break code that was tuned to work around these invalid values. I can say that our limited testing (focused on sending IoTHub cloud-to-device messages) passes with this change.

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 this pull request may close these issues.

1 participant