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 Clang failing TestKey() test from bad pointer arithmetic. #7

Merged
merged 1 commit into from
Mar 24, 2016

Conversation

mikepb
Copy link
Contributor

@mikepb mikepb commented Mar 23, 2016

Clang fails this test on make check:

trie-test.cc:71: TestKey(): 115: Assertion `r_key.ptr() == NULL' failed.

This patch uses intptr_t for pointer arithmetic to enable clang support.

@s-yata s-yata merged commit 77701a3 into s-yata:master Mar 24, 2016
@s-yata s-yata self-assigned this Mar 24, 2016
@s-yata
Copy link
Owner

s-yata commented Mar 24, 2016

Thank you. I've merged this pull request.

The behavior of a pointer arithmetic that causes an overflow is undefined.
Hmm, uintptr_t is better than intptr_t?

@mikepb
Copy link
Contributor Author

mikepb commented Mar 24, 2016

I was thinking that intptr_t would be more "proper" because the code is using -1 to mean empty. That would sidestep underflow issues entirely.

Though now that you bring it up, using undefined behavior for pointer arithmetic seems ripe for hard-to-find bugs. I was surprised that the initializer had the intended behavior too.

s-yata added a commit that referenced this pull request Mar 24, 2016
@s-yata
Copy link
Owner

s-yata commented Mar 24, 2016

Thank you for your comment.
There is not a reason enough to use -1 to mean empty.

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.

None yet

2 participants