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-api: turn const global into define #150

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

VDanielEdwards
Copy link
Member

@VDanielEdwards VDanielEdwards commented Nov 20, 2024

Subject

When linking two separately compiled object files, both using the C-API, the linker informed me about a multiply defined symbol. This turned out to be the const global SilKit_LinDataLengthUnknown.

I did grep through the C headers and haven't found another one of those.

Neither the .dll nor the .so export this symbol so no trouble there.

Issue: SILKIT-1654

Developer checklist (address before review)

  • Changelog.md updated
  • Prepared update for depending repositories
  • Documentation updated (public API changes only)
  • API docstrings updated (public API changes only)
  • Rebase → commit history clean
  • Squash and merge → proper PR title

@VDanielEdwards VDanielEdwards self-assigned this Nov 20, 2024
Copy link
Collaborator

@MariusBgm MariusBgm left a comment

Choose a reason for hiding this comment

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

looks good, as this seems to only affect a non-visible symbol.
I just tested your changes on ubuntu and Windows with a modified SilKitDemoLin (added a use of the symbol) executable. it doesn't complain about missing symbols.

SilKit/include/silkit/capi/Lin.h Show resolved Hide resolved
@VDanielEdwards VDanielEdwards merged commit 8eb649d into main Nov 27, 2024
13 checks passed
@VDanielEdwards VDanielEdwards deleted the dev_fix_const_global_in_lin_c_header branch November 27, 2024 08:57
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