Overhaul SQL error logging.

Use the primary sqlite3 interface for error logging, using a callback
function to report any errors as they happen.  This will allow us to
interface directly to the database if necessary rather than having to go
via the pkgindb_doquery() interface, and also provides useful error
messages rather than the very unhelpful "callback aborted".

Update the log output to include the date, format better for
readability, and open in append mode so we don't potentially lose useful
errors.

While here reorganise some functions into logical order and move the
remaining function declarations to pkgin.h

pkgin.h

@@ -308,7 +308,17 @@ extern char	*pkgin_errlog;
 extern char	*pkgin_sqllog;
 void		setup_pkgin_dbdir(void);
 uint8_t		have_privs(int);
+const char *	pdb_version(void);
+int		pdb_get_value(void *, int, char **, char **);
+int		pkgindb_doquery(const char *,
+		    int (*)(void *, int, char *[], char *[]), void *);
+int		pkgindb_dovaquery(const char *, ...);
 int		pkgindb_open(void);
+void		pkgindb_close(void);
+int		pkg_db_mtime(void);
+void		repo_record(char **);
+time_t		pkg_sum_mtime(char *);
+void		pkgindb_stats(void);
 
 /* preferred.c */
 void		load_preferred(void);

pkgindb.c

@@ -43,6 +43,17 @@ static const char *pragmaopts[] = {
 	NULL
 };
 
+/*
+ * Used to keep track of the current query so that it can be used in the
+ * error log callback for diagnostics.
+ *
+ * Most database access goes through pkgindb_doquery which will set curquery
+ * to the query it has been passed, and then resets it to NULL afterwards.
+ * Any callers that perform their own database access should use curquery to
+ * store their query so that errors are logged correctly.
+ */
+static const char *curquery = NULL;
+
 char *pkgin_dbdir;
 char *pkgin_sqldb;
 char *pkgin_cache;
@@ -125,6 +136,53 @@ pdb_get_value(void *param, int argc, char **argv, char **colname)
 	return PDB_ERR;
 }
 
+/*
+ * Record an error to the SQL log, optionally logging the query that caused
+ * the failure.  Any errors opening the log for writing are ignored so that
+ * regular users can issue queries.
+ */
+static void
+pkgindb_sqlerr_cb(void *arg, int errcode, const char *errmsg)
+{
+	FILE *fp;
+	struct tm tm;
+	time_t now;
+	char curtime[64];
+	char **query = (char **)arg;
+
+	now = time(NULL);
+	tm = *(localtime(&now));
+	strftime(curtime, sizeof(curtime), "%Y-%m-%d %H:%M:%S %Z", &tm);
+
+	if ((fp = fopen(pkgin_sqllog, "a")) != NULL) {
+		fprintf(fp, "SQL error at %s\n", curtime);
+		fprintf(fp, "   errmsg: %s\n", errmsg);
+		if (*query)
+			fprintf(fp, "    query: %s\n", *query);
+		fclose(fp);
+	}
+}
+
+int
+pkgindb_doquery(const char *query, int (*cb)(void *, int, char *[], char *[]),
+    void *param)
+{
+	int rv;
+
+	/*
+	 * Save the current query for the error logging callback, then reset
+	 * to NULL after the query has completed to avoid leaking into the
+	 * error log of callers that use the sqlite3_exec interface directly.
+	 */
+	curquery = query;
+
+	rv = sqlite3_exec(pdb, query, cb, param, NULL);
+
+	curquery = NULL;
+
+	return (rv == SQLITE_OK) ? PDB_OK : PDB_ERR;
+}
+
 int
 pkgindb_dovaquery(const char *fmt, ...)
 {
@@ -144,38 +202,6 @@ pkgindb_dovaquery(const char *fmt, ...)
 	return rv;
 }
 
-int
-pkgindb_doquery(const char *query, int (*cb)(void *, int, char *[], char *[]),
-    void *param)
-{
-	FILE *fp;
-	char *pdberr;
-
-	if (sqlite3_exec(pdb, query, cb, param, &pdberr) != SQLITE_OK) {
-		/*
-		 * Don't fail if we can't open the SQL log for writing, this
-		 * permits regular users to perform query operations.
-		 */
-		if ((fp = fopen(pkgin_sqllog, "w")) != NULL) {
-			if (pdberr != NULL)
-				fprintf(fp, "SQL error: %s\n", pdberr);
-			fprintf(fp, "SQL query: %s\n", query);
-			fclose(fp);
-		}
-		sqlite3_free(pdberr);
-
-		return PDB_ERR;
-	}
-
-	return PDB_OK;
-}
-
-void
-pkgindb_close()
-{
-	sqlite3_close(pdb);
-}
-
 /*
  * Configure the pkgin database.  Returns 0 if opening an existing compatible
  * database, or 1 if the database needs to be created or recreated (in the case
@@ -187,6 +213,11 @@ pkgindb_open(void)
 	int create, i, oflags;
 	char buf[128];
 
+	/*
+	 * Configure our sqlite error log callback function.
+	 */
+	sqlite3_config(SQLITE_CONFIG_LOG, pkgindb_sqlerr_cb, &curquery);
+
 	/*
 	 * Determine if we need to create a new database or can load an
 	 * existing one.  The SQLITE_OPEN_READWRITE flag does not require that
@@ -231,6 +262,12 @@ pkgindb_open(void)
 	return create;
 }
 
+void
+pkgindb_close()
+{
+	sqlite3_close(pdb);
+}
+
 int
 pkg_db_mtime()
 {

pkgindb.h

@@ -78,17 +78,6 @@ extern const char SHOW_ALL_CATEGORIES[];
 #define LOCAL_PKG "LOCAL_PKG"
 #define REMOTE_PKG "REMOTE_PKG"
 
-const char	*pdb_version(void);
-void		pkgindb_close(void);
-int		pkgindb_dovaquery(const char *, ...);
-int		pkgindb_doquery(const char *,
-		    int (*)(void *, int, char *[], char *[]), void *);
-int		pdb_get_value(void *, int, char **, char **);
-int		pkg_db_mtime(void);
-void		repo_record(char **);
-time_t		pkg_sum_mtime(char *);
-void            pkgindb_stats(void);
-
 #define PDB_OK 0
 #define PDB_ERR -1