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

Fixed manifest_dump issues when printing keys and values containing null characters #8378

Closed

Conversation

bjlemaire
Copy link
Contributor

Changed fprintf function to fputc in ApplyVersionEdit, and replaced null characters with whitespaces.
Added unit test in ldb_test.py - verifies that manifest_dump --verbose output is correct when keys and values containing null characters are inserted.

@pdillinger
Copy link
Contributor

You can see CircleCI failed on your test. (The regex does seem so specific that I was afraid it would be fragile.)

…ll characters directly, which wasnt working properly because of the python subprocess call.
@pdillinger
Copy link
Contributor

The latest CircleCI failures are unrelated to this change

Copy link
Contributor

@pdillinger pdillinger left a comment

Choose a reason for hiding this comment

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

The test is slightly obscure but gets the job done. 👍

db/version_edit_handler.cc Outdated Show resolved Hide resolved
tools/ldb_test.py Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@bjlemaire has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@bjlemaire merged this pull request in d61a449.

facebook-github-bot pushed a commit that referenced this pull request May 28, 2024
Summary:
**Context/Summary:**
the flag --json of manifest_dump in ldb tool has no effect
The bug may  be introduced by pr #8378

Pull Request resolved: #12703

Reviewed By: cbi42

Differential Revision: D57848094

Pulled By: ajkr

fbshipit-source-id: 3d1ce65528bf4ce9c53593a7208406ab90e8994b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants