[nongnu] elpa/sqlite3 8ad45baacf 05/62: fixed memory leak; used sqlite3_close_v2

[ELPA Syncer]

branch: elpa/sqlite3
commit 8ad45baacff1a5725bade9b74375be5ef64eebf1
Author: Y. N. Lo <gordonynlo@yahoo.com>
Commit: Y. N. Lo <gordonynlo@yahoo.com>

    fixed memory leak; used sqlite3_close_v2
---
 README-in.md         | 51 +++++++++++++++++++++--------
 README.md            | 91 ++++++++++++++++++++++++++++++++++------------------
 sqlite3-api-module.c | 68 +++++++++++++++++++++++----------------
 3 files changed, 138 insertions(+), 72 deletions(-)

diff --git a/README-in.md b/README-in.md
index f66f5a1de4..b0a852f9f0 100644
--- a/README-in.md
+++ b/README-in.md
@@ -6,7 +6,6 @@ direct access to the core SQLite3 C API from Emacs Lisp.
 
 (setq dbh (sqlite3-open "person.sqlite3" sqlite-open-readwrite 
sqlite-open-create))
 (sqlite3-exec dbh "create table temp (name text, age integer)")
-(sqlite3-exec dbh "begin") ;; begin transaction
 (setq stmt (sqlite3-prepare dbh "insert into temp values (?,?)"))
 (cl-loop for i from 1 to 10 do
         (sqlite3-bind-multi stmt (format "name%d" i) i)
@@ -14,7 +13,6 @@ direct access to the core SQLite3 C API from Emacs Lisp.
         (sqlite3-step stmt)
         ;; call reset if you want to bind the SQL to a new set of variables
         (sqlite3-reset stmt) )
-(sqlite3-exec dbh "commit")
 (sqlite3-finalize stmt)
 
 (setq stmt (sqlite3-prepare dbh "select * from temp"))
@@ -24,10 +22,10 @@ direct access to the core SQLite3 C API from Emacs Lisp.
 (sqlite3-finalize stmt)
 (sqlite3-close dbh)
 ~~~
-While this module provides only 14 functions (vs 200+ in the C API), it should 
satisfy most
+While this module provides only 14 functions (vs [200+ in the C 
API](https://sqlite.org/c3ref/funclist.html)), it should satisfy most
 users' needs.
 
-This is an alpha release and it might crash your Emacs. Save your work before 
you try it out!
+This is an alpha release so it might crash your Emacs. Save your work before 
you try it out!
 
 <<TOC>>
 ## Requirements
@@ -35,7 +33,7 @@ This is an alpha release and it might crash your Emacs. Save 
your work before yo
 - sqlite3 library and header file
 - A C99 compiler
 
-It's been tested on macOS Sierra) and CentOS 7.
+It's been tested on macOS (Sierra) and CentOS 7.
 ## Installation
 ~~~sh
 $ git co https://github.com/pekingduck/emacs-sqlite3-api
@@ -85,7 +83,7 @@ Close the database file.
 ~~~
 Compile the supplied SQL statement and return a statement handle.
 
-This function calls 
[`sqlite3_prepare_v2()`](https://www.sqlite.org/c3ref/prepare.html) internally 
and raises 'sql-error (such as invalid SQL statement).
+This function calls 
[`sqlite3_prepare_v2()`](https://www.sqlite.org/c3ref/prepare.html) internally 
and raises 'sql-error in case of error.
 ### sqlite3-finalize
 ~~~el
 (sqlite3-finalize statement-handle)
@@ -141,6 +139,8 @@ The callback function, if supplied, is invoked for *each 
row* and should accept
 To signal an error condition inside the callback, return `nil`. 
 `sqlite3_exec()` will stop the execution and raise 'db-error.
 
+Raises db-error in case of error.
+
 An example of a callback:
 ~~~el
 (defun print-row (ncols data names)
@@ -159,13 +159,6 @@ More examples:
 
 ;; Retrieve the metadata of columns in a table
 (sqlite3-exec dbh "pragma table_info(table)" #'print-row)
-
-;; Transaction support
-(sqlite3-exec dbh "begin")
-....
-(sqlite3-exec dbh "commit")
-...
-(sqlite3-exec dbh "rollback")
 ~~~
 ### sqlite3-bind-*
 ~~~el
@@ -236,6 +229,33 @@ Example:
 convenience. It retrieves the current row as a 
 list without having to deal with sqlite3-column-* explicitly.
 
+## Transaction Support
+Use `sqlite3-exec` to start, commit and rollback a transaction:
+~~~el
+(sqlite3-exec dbh "begin")
+(sqlite3-exec dbh "commit")
+(sqlite3-exec dbh "rollback")
+~~~
+See Error Handling below on how to use the 
[`condition-case`](https://www.gnu.org/software/emacs/manual/html_node/elisp/Handling-Errors.html)
 form to handle rollback.
+## Error Handling
+Currently two error symbols are defined in `sqlite3-api.el`:
+1. `sql-error` is raised by `sqlite3-prepare`
+2. `db-error` is raised by `sqlite3-open` and `sqlite3-exec`
+
+~~~el
+(condition-case db-err
+    (progn
+      (sqlite3-exec dbh "begin")
+      (sqlite3-exec dbh "update temp set a = 1 where b = 2")
+      (sqlite3-exec dbh "commit"))
+  (db-error
+   (message "%s (%s [%d])" (car db-err) (cadr db-err) (caddr db-err))
+   (sqlite3-exec dbh "rollback")))
+~~~
+`db-err` is a list containing the error symbol (`db-error` or `sql-error`), an 
error message and finally an error code returned from the 
+corresponding SQLite
+C API.
+
 ## A Note on Garbage Collection
 Since Emacs's garbage collection is non-deterministic, it would be 
 a good idea 
@@ -249,6 +269,11 @@ For integers > 61 bits you can retrieve them as text as a 
workaround.
 ## License
 The code is licensed under the [GNU GPL 
v3](https://www.gnu.org/licenses/gpl-3.0.html).
 
+## Changelog
+*2017-08-29*
+- Fixed a memory leak in `sql_api_exec()`
+- Changed `sqlite3_close()` to `sqlite3_close_v2()` in `sqlite_api_close()`
+- Better error handling: Error code is returned along with error message
 ## Useful Links for Writing Dynamic Modules
 - https://phst.github.io/emacs-modules
 - http://nullprogram.com/blog/2016/11/05/
diff --git a/README.md b/README.md
index 98dc45b3c8..2bc9d7c22f 100644
--- a/README.md
+++ b/README.md
@@ -1,12 +1,11 @@
-# SQLite3 Native API for Emacs 25+
-`sqlite-napi` is a dynamic module for GNU Emacs that provides 
-direct access to the core SQLite3 C API.
+# SQLite3 API for Emacs 25+
+`sqlite3-api` is a dynamic module for GNU Emacs 25+ that provides 
+direct access to the core SQLite3 C API from Emacs Lisp.
 ~~~el
-(require 'sqlite3-napi)
+(require 'sqlite3-api)
 
 (setq dbh (sqlite3-open "person.sqlite3" sqlite-open-readwrite 
sqlite-open-create))
 (sqlite3-exec dbh "create table temp (name text, age integer)")
-(sqlite3-exec dbh "begin") ;; begin transaction
 (setq stmt (sqlite3-prepare dbh "insert into temp values (?,?)"))
 (cl-loop for i from 1 to 10 do
         (sqlite3-bind-multi stmt (format "name%d" i) i)
@@ -14,7 +13,6 @@ direct access to the core SQLite3 C API.
         (sqlite3-step stmt)
         ;; call reset if you want to bind the SQL to a new set of variables
         (sqlite3-reset stmt) )
-(sqlite3-exec dbh "commit")
 (sqlite3-finalize stmt)
 
 (setq stmt (sqlite3-prepare dbh "select * from temp"))
@@ -24,11 +22,10 @@ direct access to the core SQLite3 C API.
 (sqlite3-finalize stmt)
 (sqlite3-close dbh)
 ~~~
-While this module provides only 14 functions (vs 200+ in the C API), it should 
satisfy most
+While this module provides only 14 functions (vs [200+ in the C 
API](https://sqlite.org/c3ref/funclist.html)), it should satisfy most
 users' needs.
 
-This is alpha software and might crash your Emacs. Save your work before
-trying it out.
+This is an alpha release so it might crash your Emacs. Save your work before 
you try it out!
 
 ## Table of Contents
 * [Requirements](#1)
@@ -48,23 +45,26 @@ trying it out.
     * [sqlite3-bind-multi](#3-12)
     * [sqlite3-column-*](#3-13)
     * [sqlite3-fetch](#3-14)
-* [A Note on Garbage Collection](#4)
-* [Known Problems](#5)
-* [License](#6)
-* [Useful Links for Writing Dynamic Modules](#7)
+* [Transaction Support](#4)
+* [Error Handling](#5)
+* [A Note on Garbage Collection](#6)
+* [Known Problems](#7)
+* [License](#8)
+* [Changelog](#9)
+* [Useful Links for Writing Dynamic Modules](#10)
 
 ## <a name="1"/> Requirements
-- Emacs 25.1 or above, compiled with module support (`--with-modules`)
+- Emacs 25.1 or above, compiled with module support (`./configure 
--with-modules`)
 - sqlite3 library and header file
-- A C compiler
+- A C99 compiler
 
-It's been tested on macOS and Linux (CentOS).
+It's been tested on macOS (Sierra) and CentOS 7.
 ## <a name="2"/> Installation
 ~~~sh
-$ git co https://github.com/pekingduck/emacs-sqlite3-napi
-$ cd emacs-sqlite3-napi
+$ git co https://github.com/pekingduck/emacs-sqlite3-api
+$ cd emacs-sqlite3-api
 $ make
-$ cp sqlite3-napi.el sqlite3-napi-constants.el sqlite3-napi-module.so 
/your/elisp/load-path/
+$ cp sqlite3-api.el sqlite3-api-constants.el sqlite3-api-module.so 
/your/elisp/load-path/
 ~~~
 A copy of `emacs-module.h` is included in this repo so Emacs source tree
 is not needed to build the module.
@@ -108,7 +108,7 @@ Close the database file.
 ~~~
 Compile the supplied SQL statement and return a statement handle.
 
-This function calls 
[`sqlite3_prepare_v2()`](https://www.sqlite.org/c3ref/prepare.html) internally 
and raises 'sql-error (such as invalid SQL statement).
+This function calls 
[`sqlite3_prepare_v2()`](https://www.sqlite.org/c3ref/prepare.html) internally 
and raises 'sql-error in case of error.
 ### <a name="3-4"/> sqlite3-finalize
 ~~~el
 (sqlite3-finalize statement-handle)
@@ -164,6 +164,8 @@ The callback function, if supplied, is invoked for *each 
row* and should accept
 To signal an error condition inside the callback, return `nil`. 
 `sqlite3_exec()` will stop the execution and raise 'db-error.
 
+Raises db-error in case of error.
+
 An example of a callback:
 ~~~el
 (defun print-row (ncols data names)
@@ -182,13 +184,6 @@ More examples:
 
 ;; Retrieve the metadata of columns in a table
 (sqlite3-exec dbh "pragma table_info(table)" #'print-row)
-
-;; Transaction support
-(sqlite3-exec dbh "begin")
-....
-(sqlite3-exec dbh "commit")
-...
-(sqlite3-exec dbh "rollback")
 ~~~
 ### <a name="3-11"/> sqlite3-bind-*
 ~~~el
@@ -259,19 +254,51 @@ Example:
 convenience. It retrieves the current row as a 
 list without having to deal with sqlite3-column-* explicitly.
 
-## <a name="4"/> A Note on Garbage Collection
+## <a name="4"/> Transaction Support
+Use `sqlite3-exec` to start, commit and rollback a transaction:
+~~~el
+(sqlite3-exec dbh "begin")
+(sqlite3-exec dbh "commit")
+(sqlite3-exec dbh "rollback")
+~~~
+See Error Handling below on how to use the 
[`condition-case`](https://www.gnu.org/software/emacs/manual/html_node/elisp/Handling-Errors.html)
 form to handle rollback.
+## <a name="5"/> Error Handling
+Currently two error symbols are defined in `sqlite3-api.el`:
+1. `sql-error` is raised by `sqlite3-prepare`
+2. `db-error` is raised by `sqlite3-open` and `sqlite3-exec`
+
+~~~el
+(condition-case db-err
+    (progn
+      (sqlite3-exec dbh "begin")
+      (sqlite3-exec dbh "update temp set a = 1 where b = 2")
+      (sqlite3-exec dbh "commit"))
+  (db-error
+   (message "%s (%s [%d])" (car db-err) (cadr db-err) (caddr db-err))
+   (sqlite3-exec dbh "rollback")))
+~~~
+`db-err` is a list containing the error symbol (`db-error` or `sql-error`), an 
error message and finally an error code returned from the 
+corresponding SQLite
+C API.
+
+## <a name="6"/> A Note on Garbage Collection
 Since Emacs's garbage collection is non-deterministic, it would be 
 a good idea 
 to manually free database/statement handles once they are not needed.
 
-## <a name="5"/> Known Problems
+## <a name="7"/> Known Problems
 - SQLite3 supports 64 bit integers but Emacs integers are only 61 bits.
 For integers > 61 bits you can retrieve them as text as a workaround.
-- BLOB/ TEXT fields with embedded NULLs are not supported.
+- BLOB/TEXT columns with embedded NULLs are not supported.
 
-## <a name="6"/> License
+## <a name="8"/> License
 The code is licensed under the [GNU GPL 
v3](https://www.gnu.org/licenses/gpl-3.0.html).
 
-## <a name="7"/> Useful Links for Writing Dynamic Modules
+## <a name="9"/> Changelog
+*2017-08-29*
+- Fixed a memory leak in `sql_api_exec()`
+- Changed `sqlite3_close()` to `sqlite3_close_v2()` in `sqlite_api_close()`
+- Better error handling: Error code is returned along with error message
+## <a name="10"/> Useful Links for Writing Dynamic Modules
 - https://phst.github.io/emacs-modules
 - http://nullprogram.com/blog/2016/11/05/
diff --git a/sqlite3-api-module.c b/sqlite3-api-module.c
index 546e5965b1..887e35a460 100644
--- a/sqlite3-api-module.c
+++ b/sqlite3-api-module.c
@@ -40,6 +40,8 @@ int plugin_is_GPL_compatible;
 #define ERROR(env, ...) message(env, SQLITE3_LOG_LEVEL_ERROR, __VA_ARGS__)
 #define INFO(env, ...) message(env, SQLITE3_LOG_LEVEL_INFO, __VA_ARGS__)
 
+#define FREE(p) if ((p) != 0) free(p);
+
 #define SQLITE3_MAX_LOG_BUF 1000
 
 static int SQLITE3_LOG_LEVEL_DEBUG = 0;
@@ -48,20 +50,22 @@ static int SQLITE3_LOG_LEVEL_WARN = 2;
 static int SQLITE3_LOG_LEVEL_ERROR = 3;
 static int sqlite3_api_log_level;
 
-int symbol_value_as_int(emacs_env *env,
-                        emacs_value sym,
-                        int defaul) {
+#if 0
+static int symbol_value_as_int(emacs_env *env,
+                               emacs_value sym,
+                               int deft) {
   emacs_value v = env->funcall(env, SYM(env, "symbol-value"), 1, &sym);
   if (IS_INTEGER(env, v))
     return env->extract_integer(env, v);
-  return defaul;
+  return deft;
 }
+#endif
 
 /* Equivalent to (list a b c) in elisp
    n is the number of arguments
    elts, an array of emacs_valuem, are elements of the list
  */
-emacs_value make_list(emacs_env *env, int n, emacs_value *elts) {
+static emacs_value make_list(emacs_env *env, int n, emacs_value *elts) {
   return env->funcall(env, SYM(env, "list"), n, elts);
 }
 
@@ -117,11 +121,22 @@ static void message(emacs_env *env, int log_level, const 
char *fmt, ...) {
   fprintf(stderr, "\n");
 }
 
-/* Equivalent to (signal error data) in elisp */
-void signal_error(emacs_env *env, const char *symbol, const char *msg) {
+/* Equivalent to (signal symbol '(msg code)) in elisp */
+void signal_error(
+    emacs_env *env,
+    const char *symbol,
+    const char *msg,
+    int code) {
   emacs_value signal = SYM(env, symbol);
-  emacs_value errmsg = env->make_string(env, msg, strlen(msg));
-  env->non_local_exit_signal(env, signal, make_list(env, 1, &errmsg));
+  emacs_value argv[2] = {
+    env->make_string(env, msg, strlen(msg)),
+    env->make_integer(env, code)
+  };
+
+  env->non_local_exit_signal(
+      env,
+      signal,
+      make_list(env, 2, argv));
 }
 
 /* Extract and copy string contents from function parameters */
@@ -132,7 +147,7 @@ int extract_string_arg(emacs_env *env, emacs_value arg, 
char **str) {
 
   *str = malloc(size);
   if (!env->copy_string_contents(env, arg, *str, &size)) {
-    free(*str);
+    FREE(*str);
     *str = 0;
     return 1;
   }
@@ -161,7 +176,7 @@ static void sqlite3_dbh_gc(void *ptr) {
 
   if (ptr) {
     INFO(0, "%s: non-null dbh", __func__);
-    sqlite3_close((sqlite3 *)ptr);
+    sqlite3_close_v2((sqlite3 *)ptr);
   }
 }
 
@@ -300,7 +315,7 @@ static emacs_value sqlite3_api_bind_text(
 
   DEBUG(env, "%s: [%s] to col %d", __func__, txt, col);
   int rv = sqlite3_bind_text(stmt, col, txt, -1, SQLITE_TRANSIENT);
-  free(txt);
+  FREE(txt);
   return env->make_integer(env, rv);
 }
 
@@ -334,7 +349,7 @@ static emacs_value sqlite3_api_bind_multi(
     } else if (IS_STRING(env, args[i])) {
       extract_string_arg(env, args[i], &txt);
       rv = sqlite3_bind_text(stmt, i, txt, -1, SQLITE_TRANSIENT);
-      free(txt);
+      FREE(txt);
       NON_LOCAL_EXIT_CHECK(env);
     } else if (args[i] == SYM(env, "nil")) {
       rv = sqlite3_bind_null(stmt, i);
@@ -588,7 +603,7 @@ static emacs_value sqlite3_api_fetch(
   }
 
   emacs_value res = make_list(env, ncols, elts);
-  free(elts);
+  FREE(elts);
   return res;
 }
 
@@ -620,12 +635,9 @@ static emacs_value sqlite3_api_prepare(
   int rv = sqlite3_prepare_v2(dbh, sql_txt, -1, &stmt, &tail);
   INFO(env, "%s: statement prepared (rv=%d)", __func__,  rv);
 
-  free(sql_txt);
+  FREE(sql_txt);
   if (rv != SQLITE_OK) {
-    char buf[SQLITE3_MAX_LOG_BUF];
-    snprintf(buf, SQLITE3_MAX_LOG_BUF,
-             "prepare(): sqlite3_prepare_v2() returned %d", rv);
-    signal_error(env, "sql-error", buf);
+    signal_error(env, "sql-error", "sqlite3_prepare_v2() failed", rv);
     return SYM(env, "nil");
   }
   return env->make_user_ptr(env, sqlite3_stmt_gc, stmt);
@@ -659,8 +671,8 @@ struct func_env {
 /* this #define is only used in exec_callback() */
 #define NON_LOCAL_EXIT_CHECK_AND_CLEANUP \
   if (env->non_local_exit_check(env) != emacs_funcall_exit_return) {    \
-  free(data_args); \
-  free(col_args); \
+  FREE(data_args); \
+  FREE(col_args);  \
   return 1; \
 }
 
@@ -697,8 +709,8 @@ static int exec_callback(void *data, int ncols,
   NON_LOCAL_EXIT_CHECK_AND_CLEANUP;
 
   emacs_value v = env->funcall(env, fe->callback, 3, args);
-  free(data_args);
-  free(col_args);
+  FREE(data_args);
+  FREE(col_args);
 
   if (env->is_not_nil(env, v))
     return 0;
@@ -721,6 +733,7 @@ static emacs_value sqlite3_api_exec(
 
   char *sql_txt;
   if (extract_string_arg(env, args[1], &sql_txt)) {
+      FREE(sql_txt);
     return SYM(env, "nil");
   }
 
@@ -732,9 +745,10 @@ static emacs_value sqlite3_api_exec(
   } else {
     rv = sqlite3_exec(dbh, sql_txt, 0, 0, &errmsg);
   }
+  FREE(sql_txt);
 
   if (rv != SQLITE_OK) {
-    signal_error(env, "db-error", errmsg);
+    signal_error(env, "db-error", errmsg, rv);
     if (errmsg)
       sqlite3_free(errmsg);
     return SYM(env, "nil");
@@ -782,7 +796,7 @@ static emacs_value sqlite3_api_close(
   NON_LOCAL_EXIT_CHECK(env);
 
   INFO(env, "%s: entered", __func__);
-  sqlite3_close(dbh);
+  sqlite3_close_v2(dbh);
   env->set_user_ptr(env, args[0], 0);
   return SYM(env, "nil");
 }
@@ -860,12 +874,12 @@ static emacs_value sqlite3_api_open(
   sqlite3 *dbh = 0;
   int rv = sqlite3_open_v2(db_file, &dbh, flags, 0);
   INFO(env, "%s: file=%s, flags=%d, rv=%d", __func__, db_file, flags, rv);
-  free(db_file);
+  FREE(db_file);
 
   if (rv != SQLITE_OK) {
     if (dbh)
       sqlite3_free(dbh);
-    signal_error(env, "db-error", "failed to open DB file");
+    signal_error(env, "db-error", "sqlite_open_v2() failed", rv);
     return SYM(env, "nil");
   }