-
Notifications
You must be signed in to change notification settings - Fork 123
loopd: nil-pointer bug when showing subsystems #560
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
Conversation
| SetupLoggers(logWriter, shutdownInterceptor) | ||
|
|
||
| // Special show command to list supported subsystems and exit. | ||
| if config.DebugLevel == "show" { |
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.
Is this fix effective because if err := Validate(&config) precedes now?
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.
hmm let me check
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.
yeah so debuglevel validation seems not part and not necessary to validate so it should be fixed, but maybe we clean this code part up a bit, maybe just get rid of the global pointer variable which led to the bug in the first place and do a clean initialization ?
172c076 to
0cbaeaa
Compare
hieblmi
left a comment
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.
Rebased this PR and tested, as @ziggie1984 mentioned the validation doesn't check the debuglevel, so moving the code is fine. LGTM 💾
✅ tAck
0cbaeaa to
a853b17
Compare
dcd4367 to
5db270d
Compare
bhandras
left a comment
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.
LGTM 🎉
Just a short drive-bug fix: loopd crashes when you wanna show the logging subsystems with
loopd --debuglevel showThis fix just copies the rogue code to another place, but I think a better fix would be to get rid of the global pointer and maybe just return the logger when setting it up ?
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes