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

[C] Add client shared library #836

Merged
merged 3 commits into from Feb 4, 2020
Merged

[C] Add client shared library #836

merged 3 commits into from Feb 4, 2020

Conversation

kdoherty2
Copy link

No description provided.

@tmontgomery
Copy link
Contributor

What does the TLS error handling do? The API at this point does not use this style of error handling. So, why is it added?

Also, the naming conflicts with C native driver naming making bringing in an embedded native driver problematic.

Thirdly, what is the motivation for this PR? The client is quite small. What does making it a shared object help with?

Fourth, why export all symbols? This is less than ideal on Windows. Why do this?

Copy link
Author

@kdoherty2 kdoherty2 left a comment

Choose a reason for hiding this comment

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

re: the motivation
The customer application uses aeron through an intermediary non-latency critical layer which links dynamically

@skurkchan
Copy link

re: TLS error handling
I can remove DLLMain then. Is that OK ?
Or keep the function and remove error handling ?
Thanks.

@tmontgomery
Copy link
Contributor

The DllMain and associated error functions are not needed in the client. And as I point out, could conflict if the C driver is used with the client.

@tmontgomery tmontgomery merged commit 9ff7837 into aeron-io:master Feb 4, 2020
@mjpt777
Copy link
Contributor

mjpt777 commented Nov 5, 2020

How where the set of classes decided that CLIENT_EXPORT got added to?

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.

4 participants