Skip to content

Fixed an infinite loop in "Transcoder::encode"#34

Merged
ams-tschoening merged 3 commits intomasterfrom
logcxx_398_transcoder_encode_losschar
Aug 9, 2020
Merged

Fixed an infinite loop in "Transcoder::encode"#34
ams-tschoening merged 3 commits intomasterfrom
logcxx_398_transcoder_encode_losschar

Conversation

@ams-tschoening
Copy link
Copy Markdown
Contributor

Transcoder::encode(const LogString&, std::wstring&)" by appending LOSSCHAR. The most likely alternative would be to throw on any invalid input instead, but that's not the case right now and LOSSCHAR used in other places as well.

https://issues.apache.org/jira/browse/LOGCXX-398

ams-tschoening and others added 2 commits August 8, 2020 10:49
…wstring&)" by appending LOSSCHAR. The most likely alternative would be to throw on any invalid input instead, but that's not the case right now and LOSSCHAR used in other places as well.

https://issues.apache.org/jira/browse/LOGCXX-398
@rm5248
Copy link
Copy Markdown
Contributor

rm5248 commented Aug 8, 2020

Looks good to me - I went and added a test case as well to validate that it works correctly, if you want to take a look.

Copy link
Copy Markdown
Contributor Author

@ams-tschoening ams-tschoening left a comment

Choose a reason for hiding this comment

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

This introduced spaces instead of tabs, which I have changed now. The current code style uses tabs, if that should be changed, this needs to be done for the whole codebase and the files .astylerc and .editorconfig need to be adopted. While I prefer tabs in most cases, one should make sure to use some editor honoring .editorconfig.

@ams-tschoening ams-tschoening merged commit b2190f1 into master Aug 9, 2020
@ams-tschoening ams-tschoening deleted the logcxx_398_transcoder_encode_losschar branch August 9, 2020 09:56
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