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

Enhance logging #29

Closed
wants to merge 2 commits into from
Closed

Enhance logging #29

wants to merge 2 commits into from

Conversation

mnhauke
Copy link
Contributor

@mnhauke mnhauke commented Dec 30, 2019

FTP: log requested files, even if those could not be found
FTP: log requested files, even if those aren't accessable
(insufficient permissions, ...)
TFTP: Move loglevel for RRQ|WRQ|DATA from DBG to INFO

FTP: log requested files, even if those could not be found
FTP: log requested files, even if those aren't accessable
(insufficient permissions, ...)
TFTP: Move loglevel for RRQ|WRQ|DATA from DBG to INFO
@mnhauke mnhauke mentioned this pull request Dec 30, 2019
@troglobit
Copy link
Owner

Looks good to me. What are your thoughts on using LOG() instead of INFO() to get these log messages sent to syslogd by default, rather than having to start with uftpd -l info?

@troglobit
Copy link
Owner

Also, for future reference, if you have "Fix #28: ..." in the title of your commit message, or anywhere else for that matter, issue #28 would be automatically closed when I merge the PR :)

@mnhauke
Copy link
Contributor Author

mnhauke commented Dec 30, 2019

Looks good to me. What are your thoughts on using LOG() instead of INFO() to get these log messages sent to syslogd by default, rather than having to start with uftpd -l info?

For FTP it seems that almost nothing is logged with loglevel notice (default).
For TFTP only some errors are logged with loglevel notice.
So I guess loglevel info would still fit best.

I personally always use loglevel info since loglevel notice is a bit too less verbose.

@troglobit
Copy link
Owner

That was sort of my point. Changing INFO() to LOG() in your patch we'd get these messages by default. My reasoning is that you may not be alone in wanting these messages logged, by default.

@mnhauke
Copy link
Contributor Author

mnhauke commented Dec 30, 2019

Got it.
I have updated the PR.

troglobit added a commit that referenced this pull request Jan 1, 2020
Close #28, #29

Signed-off-by: Joachim Nilsson <[email protected]>
@troglobit
Copy link
Owner

Thank you! :)

I squashed your second commit 'blubb' into the first and added another change to the log level to further increase verbosity of common operations.

@troglobit troglobit closed this Jan 1, 2020
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