Skip to content

Add assertions and cleanup #582

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

Merged
merged 7 commits into from
Dec 5, 2018
Merged

Conversation

denizevrenci
Copy link
Contributor

Related to #579

@denizevrenci denizevrenci force-pushed the issue_579 branch 2 times, most recently from 3f23e19 to 5704bc2 Compare November 21, 2018 04:41
@@ -354,7 +354,7 @@ inline static void rotateLog(AtomicBuffer& logMetaDataBuffer, std::int32_t curre
LogBufferDescriptor::casActiveTermCount(logMetaDataBuffer, currentTermCount, nextTermCount);
}

inline static void initializeTailWithTermId(AtomicBuffer& logMetaDataBuffer, int partitionIndex, std::int32_t termId)
inline void initializeTailWithTermId(AtomicBuffer& logMetaDataBuffer, int partitionIndex, std::int32_t termId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the static declaration from methods? Would this not be a breaking API change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may break the ABI in case someone wants to compile object files with the new header and link with the client library that was compiled with the old headers. With a simple test case, I confirmed that two object files still link if one of them is inline static and the other is only inline. The advantage is that the resulting binary is smaller as the linker is now free to eliminate repeating inline symbols as they are not internally linked.

@denizevrenci denizevrenci force-pushed the issue_579 branch 2 times, most recently from e47382d to 418fbcf Compare December 4, 2018 05:55
@tmontgomery
Copy link
Contributor

Can this be brought up to date? Right now it has conflicts.

@denizevrenci
Copy link
Contributor Author

Rebased on master.

@tmontgomery tmontgomery merged commit 1a0c776 into aeron-io:master Dec 5, 2018
@denizevrenci denizevrenci deleted the issue_579 branch December 6, 2018 06:19
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.

3 participants