Merge lp:~ev/whoopsie/be-more-verbose into lp:whoopsie

[Evan]

This branch finally adds a log file (/var/log/whoopsie/whoopsie.log) to whoopsie. It is only written to when whoopsie is run as a daemon.

[James Hunt (jamesodhunt)]

* 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?

[Evan (ev)]

On 7 August 2013 09:59, James Hunt <email address hidden> wrote:
> Finally, what's the reason for not using syslog(3) out of interest?

A very fair point, and one I had not considered. I'll update the MP to
use that instead. Thanks for the review!

[lp:~ev/whoopsie/be-more-verbose]

567. By Evan on 2013-08-07

Use syslog instead of /var/log/whoopsie/whoopsie.log.

568. By Evan on 2013-08-07

Test logging (stdout) when running in the foreground.

569. By Evan on 2013-08-07

Move logging startup further forward to close the gaps. Include the PID in syslog logging.

570. By Evan on 2013-08-07

No longer needed.

571. By Evan on 2013-08-07

Also no longer needed.

572. By Evan on 2013-08-07

Test logging to syslog (in stderr mode) as well.

573. By Evan on 2013-08-08

Fix resetting stdout and stderr in the logging tests.

[Evan (ev)]

Okay, I believe this is ready for review again.

I've moved to using syslog for the backend logging and have expanded the unit tests to cover both foreground logging (stdout) and syslog logging (stderr via the LOG_PERROR option).

[James Hunt (jamesodhunt)]

LGTM. Only comment is that in some places your calls to log_msg() have multiple NLs specified which will look odd when logged to syslog. For example:

    if (response > 200)
        log_msg ("Server replied with:\n%s\n", s.p);

This will result in all except the last '\n's displaying in syslog as "#012" which probably isn't what you want. I guess the easiest solution is to just make 2 calls to log_msg() in those scenarios.

review: Approve

[Evan (ev)]

Thanks!

Good spot on the newlines. I'll fix those up in the merge.

=== modified file '.bzrignore'
--- .bzrignore	2012-05-27 19:39:06 +0000
+++ .bzrignore	2013-08-08 08:46:01 +0000
@@ -12,3 +12,4 @@
 src/tests/test_utils
 src/tests/test_monitor
 src/tests/test_identifier
+src/tests/test_logging
 
=== modified file 'Makefile'
--- Makefile	2013-02-26 10:36:50 +0000
+++ Makefile	2013-08-08 08:46:01 +0000
@@ -18,6 +18,7 @@
 		src/utils.c \
 		src/connectivity.c \
 		src/monitor.c \
+		src/logging.c \
 		lib/bson/bson.c \
 		lib/bson/encoding.c \
 		lib/bson/numbers.c
 
=== modified file 'debian/copyright'
--- debian/copyright	2012-06-18 08:32:13 +0000
+++ debian/copyright	2013-08-08 08:46:01 +0000
@@ -3,8 +3,8 @@
 Source: http://code.launchpad.net/whoopsie
 
 Files: src/*
-Copyright: 2011-2012 Canonical Ltd.
-           2011-2012 Evan Dandrea <[e-mail]>
+Copyright: 2011-2013 Canonical Ltd.
+           2011-2013 Evan Dandrea <[e-mail]>
 License: GPL-3
  This package is free software; you can redistribute it and/or modify
  it under the terms of the GNU General Public License as published by
 
=== modified file 'src/connectivity.c'
--- src/connectivity.c	2013-02-26 10:50:53 +0000
+++ src/connectivity.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
 
=== modified file 'src/connectivity.h'
--- src/connectivity.h	2013-01-28 15:38:09 +0000
+++ src/connectivity.h	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
 
=== added file 'src/globals.h'
--- src/globals.h	1970-01-01 00:00:00 +0000
+++ src/globals.h	2013-08-08 08:46:01 +0000
@@ -0,0 +1,25 @@
+/* whoopsie
+ * 
+ * Copyright © 2011-2013 Canonical Ltd.
+ * Author: Evan Dandrea <[e-mail]>
+ * 
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GLOBALS_H
+#define GLOBALS_H
+
+/* Options */
+extern int foreground;
+
+#endif /* GLOBALS_H */
 
=== added file 'src/logging.c'
--- src/logging.c	1970-01-01 00:00:00 +0000
+++ src/logging.c	2013-08-08 08:46:01 +0000
@@ -0,0 +1,59 @@
+/* whoopsie
+ * 
+ * Copyright © 2011-2013 Canonical Ltd.
+ * Author: Evan Dandrea <[e-mail]>
+ * 
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define _BSD_SOURCE
+
+#include <stdio.h>
+#include <syslog.h>
+#include <stdarg.h>
+
+#include "globals.h"
+
+static int logging_started = 0;
+int logging_flags = LOG_PID;
+
+void
+log_msg (const char* fmt, ...)
+{
+    va_list argp;
+    va_start (argp, fmt);
+
+    if (foreground || !logging_started) {
+        vfprintf (stdout, fmt, argp);
+        fflush (stdout);
+    } else {
+        vsyslog(LOG_INFO, fmt, argp);
+    }
+
+    va_end (argp);
+}
+
+void
+open_log (void)
+{
+    logging_started = 1;
+    openlog ("whoopsie", logging_flags, LOG_DAEMON);
+    log_msg ("whoopsie " VERSION " starting up.\n");
+}
+
+void
+close_log (void)
+{
+    if (!foreground && logging_started)
+        closelog ();
+}
 
=== added file 'src/logging.h'
--- src/logging.h	1970-01-01 00:00:00 +0000
+++ src/logging.h	2013-08-08 08:46:01 +0000
@@ -0,0 +1,27 @@
+/* whoopsie
+ * 
+ * Copyright © 2011-2013 Canonical Ltd.
+ * Author: Evan Dandrea <[e-mail]>
+ * 
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef LOGGING_H
+#define LOGGING_H
+
+void log_msg (const char* fmt, ...);
+void open_log (void);
+void close_log (void);
+void initialise_logging (void);
+
+#endif /* LOGGING_H */
 
=== modified file 'src/monitor.c'
--- src/monitor.c	2013-02-08 11:58:46 +0000
+++ src/monitor.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
 
=== modified file 'src/monitor.h'
--- src/monitor.h	2013-01-29 17:23:14 +0000
+++ src/monitor.h	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
 
=== modified file 'src/tests/Makefile'
--- src/tests/Makefile	2013-02-26 11:02:35 +0000
+++ src/tests/Makefile	2013-08-08 08:46:01 +0000
@@ -7,6 +7,7 @@
 
 test_parse_report_SOURCES=test_parse_report.c \
 							../whoopsie.c \
+							../logging.c \
 							../utils.c \
 							../identifier.c \
 							../../lib/bson/bson.c \
@@ -31,11 +32,16 @@
 test_identifier_EXECUTABLE=test_identifier
 test_identifier_OBJECTS=$(test_identifier_SOURCES:.c=.test.o)
 
+test_logging_SOURCES=test_logging.c \
+					../logging.c
+test_logging_EXECUTABLE=test_logging
+test_logging_OBJECTS=$(test_logging_SOURCES:.c=.test.o)
+
 .PHONY: all clean check
 
 all: check
 
-check: $(test_parse_report_EXECUTABLE) $(test_utils_EXECUTABLE) $(test_monitor_EXECUTABLE) $(test_identifier_EXECUTABLE)
+check: $(test_parse_report_EXECUTABLE) $(test_utils_EXECUTABLE) $(test_monitor_EXECUTABLE) $(test_identifier_EXECUTABLE) $(test_logging_EXECUTABLE)
 	$(foreach p, $^, ./$p -k;)
 
 $(test_parse_report_EXECUTABLE): $(test_parse_report_OBJECTS)
@@ -46,6 +52,8 @@
 	$(CC) -std=c99 $^ $(LIBS) -o $@
 $(test_identifier_EXECUTABLE): $(test_identifier_OBJECTS)
 	$(CC) -std=c99 $^ $(LIBS) -o $@
+$(test_logging_EXECUTABLE): $(test_logging_OBJECTS)
+	$(CC) -std=c99 $^ $(LIBS) -o $@
 
 test_parse_report_coverage: $(test_parse_report_OBJECTS:.test.o=.coverage.o)
 	$(CC) -std=c99 $^ $(LIBS) -lgcov -o $@
@@ -55,7 +63,9 @@
 	$(CC) -std=c99 $^ $(LIBS) -lgcov -o $@
 test_identifier_coverage: $(test_identifier_OBJECTS:.test.o=.coverage.o)
 	$(CC) -std=c99 $^ $(LIBS) -lgcov -o $@
-coverage: test_parse_report_coverage test_utils_coverage test_monitor_coverage test_identifier_coverage
+test_logging_coverage: $(test_logging_OBJECTS:.test.o=.coverage.o)
+	$(CC) -std=c99 $^ $(LIBS) -lgcov -o $@
+coverage: test_parse_report_coverage test_utils_coverage test_monitor_coverage test_identifier_coverage test_logging_coverage
 	$(foreach p, $^, ./$p -k;)
 	$(foreach p, $(wildcard ../*.c), gcov $p -o $(p:.c=.coverage.o);)
 
@@ -68,9 +78,11 @@
 		$(test_monitor_EXECUTABLE) \
 		$(test_identifier_OBJECTS) \
 		$(test_identifier_EXECUTABLE) \
+		$(test_logging_EXECUTABLE) \
 		test_parse_report_coverage \
 		test_utils_coverage \
 		test_identifier_coverage \
+		test_logging_coverage \
 		coverage
 	find ../.. \( -name '*.coverage.o' -o \
 		-name '*.gcda' -o \
 
=== modified file 'src/tests/test_identifier.c'
--- src/tests/test_identifier.c	2013-02-20 18:36:11 +0000
+++ src/tests/test_identifier.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
@@ -166,7 +166,10 @@
 int
 main (int argc, char** argv)
 {
+#if GLIB_MAJOR_VERSION <= 2 && GLIB_MINOR_VERSION < 35
+    /* Deprecated in glib 2.35/2.36. */
     g_type_init ();
+#endif
     g_test_init (&argc, &argv, NULL);
 
     /* This wont work when running under fakeroot. */
 
=== added file 'src/tests/test_logging.c'
--- src/tests/test_logging.c	1970-01-01 00:00:00 +0000
+++ src/tests/test_logging.c	2013-08-08 08:46:01 +0000
@@ -0,0 +1,177 @@
+/* whoopsie
+ * 
+ * Copyright © 2011-2013 Canonical Ltd.
+ * Author: Evan Dandrea <[e-mail]>
+ * 
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define _XOPEN_SOURCE 500
+
+#include <unistd.h>
+#include <glib.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include <syslog.h>
+
+#include "../src/globals.h"
+#include "../src/logging.h"
+
+int foreground;
+
+void
+test_logging (void)
+{
+    char template[] = "/tmp/XXXXXX";
+
+    char buf[512] = {0};
+    FILE* fp = NULL;
+    const char *expected = "whoopsie " VERSION " starting up.\nHello.\n";
+
+    foreground = 1;
+
+    int fd;
+    fpos_t pos;
+
+    /* http://c-faq.com/stdio/undofreopen.html */
+    fflush (stdout);
+    fgetpos (stdout, &pos);
+    fd = dup (fileno (stdout));
+
+    tmpnam (template);
+    if (freopen (template, "w", stdout) == NULL) {
+        g_printerr ("Could not open temporary stdout.\n");
+        g_test_fail ();
+        goto out;
+    }
+
+    /* Test opening, writing to, and closing the log. */
+    open_log ();
+    log_msg ("Hello.\n");
+    close_log ();
+
+    /* Did we write a log file? */
+    if (access (template, F_OK) < 0) {
+        g_printerr ("We did not write anything to stdout.\n");
+        g_test_fail ();
+    }
+
+    /* Now ensure we've logged exactly what was expected. */
+    if ((fp = fopen (template, "r")) == NULL) {
+        g_printerr ("Could not open temporary stdout for reading.\n");
+        g_test_fail ();
+    }
+
+    fread (buf, 512, 1, fp);
+    if (strcmp (buf, expected) != 0) {
+        g_printerr ("We did not write what was expected to stdout:\n%s", buf);
+        g_test_fail ();
+    }
+
+    /* Clean up */
+    if (fclose (fp))
+        g_printerr ("Could not close the file pointer.\n");
+
+    if (unlink (template) < 0) {
+        g_printerr ("Could not remove temporary stdout.\n");
+        g_test_fail ();
+    }
+out:
+    dup2 (fd, fileno (stdout));
+    close (fd);
+    clearerr (stdout);
+    fsetpos (stdout, &pos);
+}
+
+void
+test_logging_syslog (void)
+{
+    char template[] = "/tmp/XXXXXX";
+
+    char buf[512] = {0};
+    FILE* fp = NULL;
+
+    extern int logging_flags;
+    const char *expected = "whoopsie: whoopsie " VERSION " starting up.\nwhoopsie: Hello.\n";
+
+    logging_flags = LOG_PERROR;
+    foreground = 0;
+
+    int fd;
+    fpos_t pos;
+
+    /* http://c-faq.com/stdio/undofreopen.html */
+    fflush (stderr);
+    fgetpos (stderr, &pos);
+    fd = dup (fileno (stderr));
+
+    tmpnam (template);
+    if (freopen (template, "w", stderr) == NULL) {
+        g_print ("Could not open temporary stderr.\n");
+        g_test_fail ();
+        goto out;
+    }
+
+    open_log ();
+    log_msg ("Hello.\n");
+    fflush (stderr);
+    close_log ();
+
+    /* Did we write a log file? */
+    if (access (template, F_OK) < 0) {
+        g_print ("We did not write anything to stderr.\n");
+        g_test_fail ();
+    }
+
+    /* Now ensure we've logged exactly what was expected. */
+    if ((fp = fopen (template, "r")) == NULL) {
+        g_print ("Could not open temporary stderr for reading.\n");
+        g_test_fail ();
+    }
+
+    fread (buf, 512, 1, fp);
+    if (strcmp (buf, expected) != 0) {
+        g_print ("We did not write what was expected to stderr:\n%s", buf);
+        g_test_fail ();
+    }
+
+    /* Clean up */
+    if (fclose (fp))
+        g_print ("Could not close the file pointer.\n");
+
+    if (unlink (template) < 0) {
+        g_print ("Could not remove temporary stderr.\n");
+        g_test_fail ();
+    }
+out:
+    dup2 (fd, fileno (stderr));
+    close (fd);
+    clearerr (stderr);
+    fsetpos (stderr, &pos);
+}
+
+int
+main (int argc, char** argv)
+{
+#if GLIB_MAJOR_VERSION <= 2 && GLIB_MINOR_VERSION < 35
+    /* Deprecated in glib 2.35/2.36. */
+    g_type_init ();
+#endif
+    g_test_init (&argc, &argv, NULL);
+    g_test_add_func ("/whoopsie/logging",
+                     test_logging);
+    g_test_add_func ("/whoopsie/logging-syslog",
+                     test_logging_syslog);
+    return g_test_run ();
+}
 
=== modified file 'src/tests/test_monitor.c'
--- src/tests/test_monitor.c	2013-02-06 17:22:07 +0000
+++ src/tests/test_monitor.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
@@ -215,7 +215,10 @@
 int
 main (int argc, char** argv)
 {
+#if GLIB_MAJOR_VERSION <= 2 && GLIB_MINOR_VERSION < 35
+    /* Deprecated in glib 2.35/2.36. */
     g_type_init ();
+#endif
     g_test_init (&argc, &argv, NULL);
     g_test_add_func ("/whoopsie/callback-never-triggered",
                      test_callback_never_triggered);
 
=== modified file 'src/tests/test_parse_report.c'
--- src/tests/test_parse_report.c	2013-01-31 14:39:18 +0000
+++ src/tests/test_parse_report.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
@@ -381,7 +381,10 @@
 int
 main (int argc, char** argv)
 {
+#if GLIB_MAJOR_VERSION <= 2 && GLIB_MINOR_VERSION < 35
+    /* Deprecated in glib 2.35/2.36. */
     g_type_init ();
+#endif
     g_test_init (&argc, &argv, NULL);
 
     g_test_add_func ("/whoopsie/parse-report", test_parse_report);
 
=== modified file 'src/tests/test_utils.c'
--- src/tests/test_utils.c	2013-01-31 14:39:18 +0000
+++ src/tests/test_utils.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
@@ -213,7 +213,10 @@
 int
 main (int argc, char** argv)
 {
+#if GLIB_MAJOR_VERSION <= 2 && GLIB_MINOR_VERSION < 35
+    /* Deprecated in glib 2.35/2.36. */
     g_type_init ();
+#endif
     g_test_init (&argc, &argv, NULL);
     g_test_add_func ("/whoopsie/change-file-extension", test_change_file_extension);
     g_test_add_func ("/whoopsie/already-handled-report", test_already_handled_report);
 
=== modified file 'src/utils.c'
--- src/utils.c	2013-01-31 14:39:18 +0000
+++ src/utils.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
 
=== modified file 'src/utils.h'
--- src/utils.h	2012-03-27 13:38:10 +0000
+++ src/utils.h	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
 
=== modified file 'src/whoopsie.c'
--- src/whoopsie.c	2013-07-26 15:16:39 +0000
+++ src/whoopsie.c	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify
@@ -46,6 +46,8 @@
 #include "connectivity.h"
 #include "monitor.h"
 #include "identifier.h"
+#include "logging.h"
+#include "globals.h"
 
 /* The length of time to wait before processing outstanding crashes, in seconds
  */
@@ -58,6 +60,9 @@
 /* The URL of the crash database. */
 static char* crash_db_url = NULL;
 
+/* Username we will run under */
+static const char* username = "whoopsie";
+
 /*  The database identifier. Either:
  *  - The system UUID, taken from the DMI tables and SHA-512 hashed
  *  - The MAC address of the first non-loopback device, SHA-512 hashed */
@@ -66,9 +71,6 @@
 /* The URL for sending the initial crash report */
 static char* crash_db_submit_url = NULL;
 
-/* Username we will run under */
-static const char username[] = "whoopsie";
-
 /* The file path and descriptor for our instance lock */
 static const char* lock_path = "/var/lock/whoopsie/lock";
 static int lock_fd = 0;
@@ -77,10 +79,9 @@
 static const char* report_dir = "/var/crash";
 
 /* Options */
+int foreground = 0;
 
 #ifndef TEST
-static gboolean foreground = FALSE;
-
 static GOptionEntry option_entries[] = {
     { "foreground", 'f', 0, G_OPTION_ARG_NONE, &foreground,
       "Run in the foreground", NULL },
@@ -264,12 +265,12 @@
     /* TODO use curl_share for DNS caching. */
     /* Repeated calls to curl_global_init will have no effect. */
     if (curl_global_init (CURL_GLOBAL_SSL)) {
-        g_print ("Unable to initialize curl.\n\n");
+        log_msg ("Unable to initialize curl.\n\n");
         exit (EXIT_FAILURE);
     }
 
     if ((curl = curl_easy_init ()) == NULL) {
-        printf ("Couldn't init curl.\n");
+        log_msg ("Couldn't init curl.\n");
         return FALSE;
     }
     curl_easy_setopt (curl, CURLOPT_POST, 1);
@@ -288,9 +289,9 @@
     curl_slist_free_all(list);
     curl_easy_getinfo (curl, CURLINFO_RESPONSE_CODE, &response_code);
 
-    printf ("Sent; server replied with: %s\n",
+    log_msg ("Sent; server replied with: %s\n",
         curl_easy_strerror (result_code));
-    printf ("Response code: %ld\n", response_code);
+    log_msg ("Response code: %ld\n", response_code);
     curl_easy_cleanup (curl);
 
     if (result_code != CURLE_OK)
@@ -515,7 +516,7 @@
     /* TODO use CURLOPT_READFUNCTION to transparently compress data with
      * Snappy. */
     if ((curl = curl_easy_init ()) == NULL) {
-        printf ("Couldn't init curl.\n");
+        log_msg ("Couldn't init curl.\n");
         g_free (crash_db_core_url);
         return FALSE;
     }
@@ -536,9 +537,9 @@
 
     curl_easy_getinfo (curl, CURLINFO_RESPONSE_CODE, &response_code);
 
-    printf ("Sent; server replied with: %s\n",
+    log_msg ("Sent; server replied with: %s\n",
         curl_easy_strerror (result_code));
-    printf ("Response code: %ld\n", response_code);
+    log_msg ("Response code: %ld\n", response_code);
     curl_easy_cleanup (curl);
     g_free (crash_db_core_url);
     destroy_response_string (&s);
@@ -568,11 +569,11 @@
                     /* We do not retry the upload. Once is a big enough hit to
                      * their Internet connection, and we can always count on
                      * the next person in line to send it. */
-                    printf ("Upload of the core dump failed.\n");
+                    log_msg ("Upload of the core dump failed.\n");
             } else
-                printf ("Asked for a core dump that we don't have.\n");
+                log_msg ("Asked for a core dump that we don't have.\n");
         } else
-            printf ("Got command: %s\n", command);
+            log_msg ("Got command: %s\n", command);
     }
 }
 
@@ -588,28 +589,28 @@
     bson b[1];
     int response = 0;
 
-    g_print ("Parsing %s.\n", crash_file);
+    log_msg ("Parsing %s.\n", crash_file);
     report = parse_report (crash_file, FALSE, &error);
     if (!report) {
         if (error) {
-            g_print ("Unable to parse report (%s): %s\n", crash_file,
+            log_msg ("Unable to parse report (%s): %s\n", crash_file,
                        error->message);
             g_error_free (error);
         } else {
-            g_print ("Unable to parse report (%s)\n", crash_file);
+            log_msg ("Unable to parse report (%s)\n", crash_file);
         }
         /* Do not keep trying to parse and upload this */
         return TRUE;
     }
 
     if (!bsonify (report, b, &message_data, &message_len)) {
-        g_print ("Unable to bsonify report (%s)\n", crash_file);
+        log_msg ("Unable to bsonify report (%s)\n", crash_file);
         if (bson_data (b))
             bson_destroy (b);
         /* Do not keep trying to parse and upload this */
         success = TRUE;
     } else {
-        g_print ("Uploading %s.\n", crash_file);
+        log_msg ("Uploading %s.\n", crash_file);
         init_response_string (&s);
         response = upload_report (message_data, message_len, &s);
         if (bson_data (b))
@@ -625,7 +626,7 @@
             success = FALSE;
 
         if (response > 200)
-            g_print ("Server replied with:\n%s\n", s.p);
+            log_msg ("Server replied with:\n%s\n", s.p);
 
         if (response == 200 && s.length > 0)
             handle_response (report, s.p);
@@ -672,7 +673,7 @@
             continue;
         } else if (online_state && parse_and_upload_report (crash_file)) {
             if (!mark_handled (crash_file))
-                g_print ("Unable to mark report as seen (%s)\n", crash_file);
+                log_msg ("Unable to mark report as seen (%s)\n", crash_file);
 
         } 
 
@@ -691,7 +692,7 @@
     struct rlimit rl = {0};
 
     if (getrlimit (RLIMIT_NOFILE, &rl) < 0) {
-        g_print ("Could not get resource limits.\n");
+        log_msg ("Could not get resource limits.\n");
         exit (EXIT_FAILURE);
     }
 
@@ -713,7 +714,7 @@
     if ((open ("/dev/null", O_RDWR) != 0) ||
         (dup (0) != 1) ||
         (dup (0) != 2)) {
-        g_print ("Could not redirect file descriptors to /dev/null.\n");
+        log_msg ("Could not redirect file descriptors to /dev/null.\n");
         exit (EXIT_FAILURE);
     }
 }
@@ -730,21 +731,21 @@
         /* use system directory */
         if (mkdir ("/var/lock/whoopsie", 0755) < 0) {
             if (errno != EEXIST) {
-                g_print ("Could not create lock directory.\n");
+                log_msg ("Could not create lock directory.\n");
             }
         }
     }
 
-    g_print("Using lock path: %s\n", lock_path);
+    log_msg ("Using lock path: %s\n", lock_path);
 
     lock_fd = open (lock_path, O_CREAT | O_RDWR, 0600);
     rc = flock (lock_fd, LOCK_EX | LOCK_NB);
     if (rc) {
         if (EWOULDBLOCK == errno) {
-            g_print ("Another instance is already running.\n");
+            log_msg ("Another instance is already running.\n");
             exit (1);
         } else {
-            g_print ("Could not create lock file: %s\n", strerror (errno));
+            log_msg ("Could not create lock file: %s\n", strerror (errno));
         }
     }
 }
@@ -806,7 +807,7 @@
 void
 network_changed (gboolean available)
 {
-    g_print (available ? "online\n" : "offline\n");
+    log_msg (available ? "online\n" : "offline\n");
     if (!available) {
         online_state = FALSE;
         return;
@@ -825,12 +826,12 @@
 check_online_then_upload (const char* crash_file) {
 
     if (!online_state) {
-        g_print ("Not online; processing later (%s).\n", crash_file);
+        log_msg ("Not online; processing later (%s).\n", crash_file);
         return FALSE;
     }
 
     if (!parse_and_upload_report (crash_file)) {
-        g_print ("Could not upload; processing later (%s).\n", crash_file);
+        log_msg ("Could not upload; processing later (%s).\n", crash_file);
         return FALSE;
     }
 
@@ -838,31 +839,32 @@
 }
 
 void
-create_directory (void)
+create_crash_directory (void)
 {
     struct passwd *pw = NULL;
 
     if (mkdir (report_dir, 0755) < 0) {
         if (errno != EEXIST) {
-            g_print ("Could not create non-existent directory to monitor (%d): %s.\n", errno, report_dir);
+            log_msg ("Could not create non-existent report_directory to monitor (%d): %s.\n", errno, report_dir);
             exit (EXIT_FAILURE);
         }
     } else {
         /* Only change the permissions if we've just created it */
         if (!(pw = getpwnam (username))) {
-            g_print ("Could not find user, %s.\n", username);
+            log_msg ("Could not find user, %s.\n", username);
             exit (EXIT_FAILURE);
         }
         if (chown (report_dir, -1, pw->pw_gid) < 0) {
-            g_print ("Could not change ownership of %s.\n", report_dir);
+            log_msg ("Could not change ownership of %s.\n", report_dir);
             exit (EXIT_FAILURE);
         }
         if (chmod (report_dir, 03777) < 0) {
-            g_print ("Could not change permissions on %s.\n", report_dir);
+            log_msg ("Could not change permissions on %s.\n", report_dir);
             exit (EXIT_FAILURE);
         }
     }
 }
+
 #ifndef TEST
 static GMainLoop* loop = NULL;
 
@@ -875,7 +877,7 @@
     context = g_option_context_new (NULL);
     g_option_context_add_main_entries (context, option_entries, NULL);
     if (!g_option_context_parse (context, argc, argv, &err)) {
-        g_print ("whoopsie: %s\n", err->message);
+        log_msg ("whoopsie: %s\n", err->message);
         g_error_free (err);
         exit (EXIT_FAILURE);
     }
@@ -916,10 +918,13 @@
     parse_arguments (&argc, &argv);
 
     if ((crash_db_url = get_crash_db_url ()) == NULL) {
-        g_print ("Could not get crash database location.\n");
+        log_msg ("Could not get crash database location.\n");
         exit (EXIT_FAILURE);
     }
 
+    if (!foreground)
+        open_log ();
+
     /* environment might change report directory and identifier */
     env = g_getenv ("APPORT_REPORT_DIR");
     if (env != NULL)
@@ -932,7 +937,7 @@
         whoopsie_identifier_generate (&whoopsie_identifier, &err);
 
     if (err) {
-        g_print ("%s\n", err->message);
+        log_msg ("%s\n", err->message);
         g_error_free (err);
         err = NULL;
         crash_db_submit_url = strdup (crash_db_url);
@@ -942,11 +947,11 @@
                                                whoopsie_identifier);
     }
 
-    create_directory ();
+    create_crash_directory ();
 
     drop_privileges (&err);
     if (err) {
-        g_print ("%s\n", err->message);
+        log_msg ("%s\n", err->message);
         g_error_free (err);
         exit (EXIT_FAILURE);
     }
@@ -975,6 +980,8 @@
     unmonitor_directory (monitor, check_online_then_upload);
     unmonitor_connectivity ();
 
+    close_log ();
+
     g_unlink (lock_path);
     close (lock_fd);
     curl_global_cleanup ();
 
=== modified file 'src/whoopsie.h'
--- src/whoopsie.h	2013-07-26 15:16:39 +0000
+++ src/whoopsie.h	2013-08-08 08:46:01 +0000
@@ -1,6 +1,6 @@
 /* whoopsie
  * 
- * Copyright © 2011-2012 Canonical Ltd.
+ * Copyright © 2011-2013 Canonical Ltd.
  * Author: Evan Dandrea <[e-mail]>
  * 
  * This program is free software: you can redistribute it and/or modify