Skip to content

Added Polaris_AuthenticateTo() to allow use of an alternate API server.#44

Merged
adamshapiro0 merged 1 commit intomasterfrom
auth
Sep 17, 2021
Merged

Added Polaris_AuthenticateTo() to allow use of an alternate API server.#44
adamshapiro0 merged 1 commit intomasterfrom
auth

Conversation

@adamshapiro0
Copy link
Copy Markdown
Contributor

No description provided.

@adamshapiro0 adamshapiro0 self-assigned this Sep 17, 2021
Comment thread c/src/point_one/polaris/polaris.c
Comment thread c/src/point_one/polaris/polaris.c
Copy link
Copy Markdown
Member

@anathan anathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works. I would have perhaps considered making the API URL part of the context instead to avoid the "three strings" argument in authenticate to, but this functionally works.

@adamshapiro0
Copy link
Copy Markdown
Contributor Author

adamshapiro0 commented Sep 17, 2021

I thought about that a bit. To do it, we would still need to create a setter to modify the context since ideally we don't want anyone to mess with it directly in case it changes in the future. Even then, it would either store const char*, which would only work if the caller guaranteed the string didn't go out of scope ever, or have to store a char array, which increases memory footprint. With const char*, if they user used a temp string (e.g., wanted to use sprintf() or something) and it went out of scope, future calls to Authenticate() to handle reconnects would crash.

I figured this was the clearest and safest solution, and it's consistent with the existing ConnectTo() call.

@adamshapiro0 adamshapiro0 merged commit 226d786 into master Sep 17, 2021
@adamshapiro0 adamshapiro0 deleted the auth branch September 17, 2021 15:26
adamshapiro0 added a commit that referenced this pull request Sep 17, 2021
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.

2 participants