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

Why a "%d" specifier next to another "%d" specifier would crash the decompressor #41

Closed
ghost opened this issue Oct 18, 2020 · 3 comments
Labels

Comments

@ghost
Copy link

ghost commented Oct 18, 2020

Make modifications to "sample/main.cc" as shown in the figure below,
image
Then compile and run the program,

$ cd sample
$ make
$ ./sampleApplication
$ ./decompressor decompress compressedLog

image
What I expected to output is "My number is 987654321123456789", but the actual output is "My number is 987654321987654321". It outputs the integer 987654321 twice but doesn't output the integer 123456789.
On top of that, the decompressor aborted.

adding a comma between the two "%d" specifiers could solve the problem,
image

I'm confusing and wondering whether this is a bug or not.

@syang0
Copy link
Member

syang0 commented Oct 19, 2020

Hey zcq1223,

Thanks for the clear examples! I was able to reproduce the issue, and I do believe it is in fact a bug; NanoLog should support logging %d%d as two integers.

I briefly looked at the stack trace, and it appears to be crashing at Log.cc:1405, and I suspect it's due to an alignment issue when reading back the integer arguments.

In any case, I should be able to track down the bug and fix it in a couple of days, so please use the comma alternative for now. Thanks for reporting!

Best Regards,
Stephen Yang

@syang0 syang0 added the bug label Oct 19, 2020
syang0 added a commit that referenced this issue Oct 19, 2020
Fixed an issue with the decompressor whereby specifiers without
spaces between them (e.g. "%d%d") would be incorrectly parsed,
causing the decompressor to crash upon decompression.
@syang0
Copy link
Member

syang0 commented Oct 19, 2020

Hey zcq1223,

The issue should be resolved now with the latest commit (b1b9ee0). Please check it out and let me know if the issue persists.

Best Regards,
Stephen Yang

P.S. If you're curious, the fix just involved adding one line to Log.cc; the rest of the commit adds unit tests.

@ghost
Copy link
Author

ghost commented Oct 21, 2020

Great, I checked it out and found the issue was resolved.

@ghost ghost closed this as completed Oct 21, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant