Update Hiredis to use unique sds symbol names.#7661
Merged
oranagra merged 3 commits intoredis:unstablefrom Aug 16, 2020
Merged
Conversation
00272d669 Rename sds calls so they don't conflict in Redis. git-subtree-dir: deps/hiredis git-subtree-split: 00272d669b11e96b8311d9bfe167c117f8887dd6
…unique-sds-symbols
oranagra
approved these changes
Aug 16, 2020
Member
|
@michael-grunder thank you. |
JackieXie168
pushed a commit
to JackieXie168/redis
that referenced
this pull request
Aug 21, 2020
…-symbols This resolves an issue with sentinel that was created when hiredis was recently updated. this was due to sds symbol names clashing, since hiredis now includes different implementation of sdsrange than the one in redis. The state of things is that redis-benchamrk and redis-cli include only hiredis sds implementation. redis-cli even operates (calls sdscatlen) on sds that's allocated by hiredis. Sentinel however has both implementations of the sds library in it (now each with it's own unique symbols).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit updates Hiredis to use totally unique symbol names for its bundled copy of the SDS library.
See #7609 for a good discussion of why we need to do this as well as all of the different options considered.
I did my best to verify (manually and with some scripting) that Redis sentinel never directly interacts with Hiredis' sds strings, and that I didn't miss any functions to rename.
Tests seem good but let me know if you'd like any changes to the PR.
I plan on removing SDS from hiredis at which point we can clean up this fix.