Code review comment for lp:~ev/whoopsie/be-more-verbose

Revision history for this message
James Hunt (jamesodhunt) wrote :

* main():
  - There are 2 calls to log_msg() before the call to initialise_logging(). I can see that this actually works
    since log_msg() checks log_file_p, but it looks a little alarming initially :)
* initialise_logging():
  - Print errno values for error conditions?
* log_msg():
  - You might want that fflush() to apply in both scenario since stdout is buffered by default.
* open_log():
  - I'd make the file perms 0640 for security reasons.
  - Might be worth adding in the pid whoopsie is running under to the startup message.
  - I'd add an "assert (! log_file_p)" to the start of the function.

Might be worth having open_log() call initialise_logging() itself (and then use a static variable in open_log() to determine if the logging subsystem has already been initialised) since it's slightly confusing seeing two calls in main that both seem to do something similar.

Finally, what's the reason for not using syslog(3) out of interest?

« Back to merge proposal