-
Notifications
You must be signed in to change notification settings - Fork 291
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
Add LOGGER_ASSERT
for checking fatal error conditions.
#1230
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 1 approvals obtained (waiting on @iphydf)
toxcore/logger.c, line 107 at r1 (raw file):
log = &logger_stderr; #else fprintf(stderr, "NULL logger not permitted\n");
Since this is a direct fprintf statement, punctuation would be appropriate, no?
toxcore/tox.c, line 1135 at r1 (raw file):
set_message
I get that the case is here for documentation purposes but it's still superfluous
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @iphydf)
toxcore/tox.c, line 1135 at r1 (raw file):
Previously, hugbubby wrote…
set_message
I get that the case is here for documentation purposes but it's still superfluous
(meant to point to case -5: and not set_message)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @hugbubby)
toxcore/logger.h, line 103 at r1 (raw file):
Previously, hugbubby wrote…
Capitalize the A here
Done.
toxcore/logger.c, line 107 at r1 (raw file):
Previously, hugbubby wrote…
Since this is a direct fprintf statement, punctuation would be appropriate, no?
Done.
toxcore/tox.c, line 1135 at r1 (raw file):
Previously, hugbubby wrote…
(meant to point to case -5: and not set_message)
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r2.
Reviewable status: 1 change requests, 0 of 1 approvals obtained
Wouldn't know what Travis is complaining about, but I approved it because it LGTM |
These are not compiled out under `NDEBUG` and should be provably correct.
174585f
to
aa5c782
Compare
These are not compiled out under
NDEBUG
and should be provably correct.This change is