From 40456147da7fb14402e7aeaafa0c6ef775ec3a8c Mon Sep 17 00:00:00 2001 From: Maas-Maarten Zeeman Date: Mon, 3 Jan 2022 23:01:24 +0100 Subject: [PATCH 1/6] Clarify names used for raw connections and statements --- src/esqlite3.erl | 117 +++++++++++++++++++++++-------------------- src/esqlite3_nif.erl | 36 +++++++------ 2 files changed, 86 insertions(+), 67 deletions(-) diff --git a/src/esqlite3.erl b/src/esqlite3.erl index 81b247e..a1393ec 100644 --- a/src/esqlite3.erl +++ b/src/esqlite3.erl @@ -49,8 +49,17 @@ %% --type connection() :: {connection, reference(), term()}. --type statement() :: {statement, term(), connection()}. +-record(connection, { + raw_connection :: esqlite_nif:raw_connection() +}). + +-record(statement, { + raw_statement :: esqlite_nif:raw_statement(), + raw_connection :: esqlite_nif:raw_connection() +}). + +-type connection() :: #connection{}. +-type statement() :: #statement{}. -type sql() :: iodata(). %% erlang -> sqlite type conversions @@ -92,13 +101,13 @@ open(Filename) -> %% -spec open(string(), timeout()) -> {ok, connection()} | {error, _}. open(Filename, Timeout) -> - {ok, Connection} = esqlite3_nif:start(), + {ok, RawConnection} = esqlite3_nif:start(), Ref = make_ref(), - ok = esqlite3_nif:open(Connection, Ref, self(), Filename), - case receive_answer(Connection, Ref, Timeout) of + ok = esqlite3_nif:open(RawConnection, Ref, self(), Filename), + case receive_answer(RawConnection, Ref, Timeout) of ok -> - {ok, {connection, make_ref(), Connection}}; + {ok, #connection{raw_connection=RawConnection}}; {error, _Msg}=Error -> Error end. @@ -118,10 +127,10 @@ set_update_hook(Pid, Connection) -> %% @doc Same as set_update_hook, but with an additional timeout parameter. -spec set_update_hook(pid(), connection(), timeout()) -> ok | {error, term()}. -set_update_hook(Pid, {connection, _Ref, Connection}, Timeout) -> +set_update_hook(Pid, #connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:set_update_hook(Connection, Ref, self(), Pid), - receive_answer(Connection, Ref, Timeout). + ok = esqlite3_nif:set_update_hook(RawConnection, Ref, self(), Pid), + receive_answer(RawConnection, Ref, Timeout). %% @doc Execute a sql statement, returns a list with tuples. -spec q(sql(), connection()) -> list(tuple()) | {error, term()}. @@ -340,16 +349,16 @@ exec(Sql, Connection) -> exec(Sql, [], Connection, ?DEFAULT_TIMEOUT). -spec exec(sql(), list(cell_type()) | connection(), connection() | timeout()) -> ok | {error, _}. -exec(Sql, {connection, _,_}=Connection, Timeout) -> +exec(Sql, #connection{}=Connection, Timeout) -> exec(Sql, [], Connection, Timeout); -exec(Sql, Params, Connection) -> +exec(Sql, Params, #connection{}=Connection) -> exec(Sql, Params, Connection, ?DEFAULT_TIMEOUT). -spec exec(sql(), list(cell_type()), connection(), timeout()) -> ok | {error, _}. -exec(Sql, [], {connection, _Ref, Connection}, Timeout) -> +exec(Sql, [], #connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:exec(Connection, Ref, self(), Sql), - receive_answer(Connection, Ref, Timeout); + ok = esqlite3_nif:exec(RawConnection, Ref, self(), Sql), + receive_answer(RawConnection, Ref, Timeout); exec(Sql, Params, Connection, Timeout) -> {ok, Statement} = prepare(Sql, Connection, Timeout), bind(Statement, Params), @@ -362,10 +371,10 @@ changes(Connection) -> changes(Connection, ?DEFAULT_TIMEOUT). -spec changes(connection(), timeout()) -> non_neg_integer(). -changes({connection, _Ref, Connection}, Timeout) -> +changes(#connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:changes(Connection, Ref, self()), - receive_answer(Connection, Ref, Timeout). + ok = esqlite3_nif:changes(RawConnection, Ref, self()), + receive_answer(RawConnection, Ref, Timeout). %% @doc Insert records, returns the last rowid. %% @@ -375,10 +384,10 @@ insert(Sql, Connection) -> %% @doc Like insert/2, but with extra timeout parameter. -spec insert(sql(), connection(), timeout()) -> {ok, rowid()} | {error, _}. -insert(Sql, {connection, _Ref, Connection}, Timeout) -> +insert(Sql, #connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:insert(Connection, Ref, self(), Sql), - receive_answer(Connection, Ref, Timeout). + ok = esqlite3_nif:insert(RawConnection, Ref, self(), Sql), + receive_answer(RawConnection, Ref, Timeout). %% @doc Check if the connection is in auto-commit mode. %% See: [https://sqlite.org/c3ref/get_autocommit.html] for more details. @@ -389,10 +398,10 @@ get_autocommit(Connection) -> %% @doc Like autocommit/1, but with an extra timeout attribute. -spec get_autocommit(connection(), timeout()) -> true | false. -get_autocommit({connection, _Ref, Connection}, Timeout) -> +get_autocommit(#connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:get_autocommit(Connection, Ref, self()), - receive_answer(Connection, Ref, Timeout). + ok = esqlite3_nif:get_autocommit(RawConnection, Ref, self()), + receive_answer(RawConnection, Ref, Timeout). %% @doc Compile a SQL statement. Returns a cached compiled statement which can be used in %% queries. @@ -403,12 +412,14 @@ prepare(Sql, Connection) -> %% @doc Like prepare/2, but with an extra timeout value. -spec prepare(sql(), connection(), timeout()) -> {ok, statement()} | {error, _}. -prepare(Sql, {connection, _Ref, Connection}=C, Timeout) -> +prepare(Sql, #connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:prepare(Connection, Ref, self(), Sql), - case receive_answer(Connection, Ref, Timeout) of - {ok, Stmt} -> {ok, {statement, Stmt, C}}; - Else -> Else + ok = esqlite3_nif:prepare(RawConnection, Ref, self(), Sql), + case receive_answer(RawConnection, Ref, Timeout) of + {ok, Stmt} -> + {ok, #statement{raw_statement=Stmt, raw_connection=RawConnection}}; + Else -> + Else end. %% @doc Step @@ -420,10 +431,10 @@ step(Stmt) -> %% @doc %% -spec step(statement(), timeout()) -> tuple() | '$busy' | '$done'. -step({statement, Stmt, {connection, _, Conn}}, Timeout) -> +step(#statement{raw_statement=RawStatement, raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:multi_step(Conn, Stmt, 1, Ref, self()), - case receive_answer(Conn, Ref, Timeout) of + ok = esqlite3_nif:multi_step(RawConnection, RawStatement, 1, Ref, self()), + case receive_answer(RawConnection, Ref, Timeout) of {rows, [Row | []]} -> {row, Row}; {'$done', []} -> '$done'; {'$busy', []} -> '$busy'; @@ -437,18 +448,18 @@ step({statement, Stmt, {connection, _, Conn}}, Timeout) -> {'$busy', list(tuple())} | {'$done', list(tuple())} | {error, term()}. -multi_step({statement, Stmt, {connection, _, Conn}}, ChunkSize, Timeout) -> +multi_step(#statement{raw_statement=RawStatement, raw_connection=RawConnection}, ChunkSize, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:multi_step(Conn, Stmt, ChunkSize, Ref, self()), - receive_answer(Conn, Ref, Timeout). + ok = esqlite3_nif:multi_step(RawConnection, RawStatement, ChunkSize, Ref, self()), + receive_answer(RawConnection, Ref, Timeout). %% @doc Reset the prepared statement back to its initial state. %% -spec reset(statement()) -> ok | {error, _}. -reset({statement, Stmt, {connection, _, Conn}}) -> +reset(#statement{raw_statement=RawStatement, raw_connection=RawConnection}) -> Ref = make_ref(), - ok = esqlite3_nif:reset(Conn, Stmt, Ref, self()), - receive_answer(Conn, Ref, ?DEFAULT_TIMEOUT). + ok = esqlite3_nif:reset(RawConnection, RawStatement, Ref, self()), + receive_answer(RawConnection, Ref, ?DEFAULT_TIMEOUT). %% @doc Bind values to prepared statements %% @@ -458,10 +469,10 @@ bind(Stmt, Args) -> %% @doc Bind values to prepared statements -spec bind(statement(), list(cell_type()), timeout()) -> ok | {error, _}. -bind({statement, Stmt, {connection, _, Conn}}, Args, Timeout) -> +bind(#statement{raw_statement=RawStatement, raw_connection=RawConnection}, Args, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:bind(Conn, Stmt, Ref, self(), Args), - receive_answer(Conn, Ref, Timeout). + ok = esqlite3_nif:bind(RawConnection, RawStatement, Ref, self(), Args), + receive_answer(RawConnection, Ref, Timeout). %% @doc Return the column names of the prepared statement. %% @@ -470,22 +481,22 @@ column_names(Stmt) -> column_names(Stmt, ?DEFAULT_TIMEOUT). -spec column_names(statement(), timeout()) -> {atom()}. -column_names({statement, Stmt, {connection, _, Conn}}, Timeout) -> +column_names(#statement{raw_statement=RawStatement, raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:column_names(Conn, Stmt, Ref, self()), - receive_answer(Conn, Ref, Timeout). + ok = esqlite3_nif:column_names(RawConnection, RawStatement, Ref, self()), + receive_answer(RawConnection, Ref, Timeout). %% @doc Return the column types of the prepared statement. %% -spec column_types(statement()) -> {atom()}. -column_types(Stmt) -> - column_types(Stmt, ?DEFAULT_TIMEOUT). +column_types(Statement) -> + column_types(Statement, ?DEFAULT_TIMEOUT). -spec column_types(statement(), timeout()) -> {atom()}. -column_types({statement, Stmt, {connection, _, Conn}}, Timeout) -> +column_types(#statement{raw_statement=RawStatement, raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:column_types(Conn, Stmt, Ref, self()), - receive_answer(Conn, Ref, Timeout). + ok = esqlite3_nif:column_types(RawConnection, RawStatement, Ref, self()), + receive_answer(RawConnection, Ref, Timeout). %% @doc Close the database -spec close(connection()) -> ok | {error, _}. @@ -494,10 +505,10 @@ close(Connection) -> %% @doc Close the database -spec close(connection(), timeout()) -> ok | {error, _}. -close({connection, _Ref, Connection}, Timeout) -> +close(#connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:close(Connection, Ref, self()), - receive_answer(Connection, Ref, Timeout). + ok = esqlite3_nif:close(RawConnection, Ref, self()), + receive_answer(RawConnection, Ref, Timeout). %% @doc Flush any stale answers left in the mailbox of the current process. @@ -511,12 +522,12 @@ flush() -> %% Internal functions -receive_answer(Connection, Ref, Timeout) -> +receive_answer(RawConnection, Ref, Timeout) -> receive {esqlite3, Ref, Resp} -> Resp after Timeout -> - ok = esqlite3_nif:interrupt(Connection), + ok = esqlite3_nif:interrupt(RawConnection), throw({error, timeout, Ref}) end. diff --git a/src/esqlite3_nif.erl b/src/esqlite3_nif.erl index 3f2503a..d2592b9 100644 --- a/src/esqlite3_nif.erl +++ b/src/esqlite3_nif.erl @@ -39,6 +39,11 @@ close/3 ]). +-type raw_connection() :: reference(). +-type raw_statement() :: reference(). + +-export_type([raw_connection/0, raw_statement/0]). + -on_load(init/0). init() -> @@ -51,7 +56,7 @@ init() -> %% @doc Start a low level thread which will can handle sqlite3 calls. %% --spec start() -> {ok, esqlite:connection()} | {error, any()}. +-spec start() -> {ok, raw_connection()} | {error, _}. start() -> erlang:nif_error(nif_library_not_loaded). @@ -60,10 +65,11 @@ start() -> %% Sends an asynchronous open command over the connection and returns %% ok immediately. When the database is opened %% --spec open(esqlite:connection(), reference(), pid(), string()) -> ok | {error, any()}. +-spec open(raw_connection(), reference(), pid(), string()) -> ok | {error, _}. open(_Db, _Ref, _Dest, _Filename) -> erlang:nif_error(nif_library_not_loaded). +-spec set_update_hook(raw_connection(), reference(), pid(), pid()) -> ok | {error, _}. set_update_hook(_Db, _Ref, _Dest, _Pid) -> erlang:nif_error(nif_library_not_loaded). @@ -75,7 +81,7 @@ set_update_hook(_Db, _Ref, _Dest, _Pid) -> %% When the statement is executed Dest will receive message {Ref, answer()} %% with answer() integer | {error, reason()} %% --spec exec(esqlite:connection(), reference(), pid(), string()) -> ok | {error, any()}. +-spec exec(raw_connection(), reference(), pid(), string()) -> ok | {error, _}. exec(_Db, _Ref, _Dest, _Sql) -> erlang:nif_error(nif_library_not_loaded). @@ -83,70 +89,72 @@ exec(_Db, _Ref, _Dest, _Sql) -> %% %% When the statement is executed Dest will receive message {Ref, answer()} %% with answer() integer | {error, reason()} -%% +-spec changes(raw_connection(), reference(), pid()) -> ok | {error, _}. changes(_Db, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). %% @doc %% --spec prepare(esqlite:connection(), reference(), pid(), string()) -> ok | {error, any()}. +-spec prepare(raw_connection(), reference(), pid(), string()) -> ok | {error, _}. prepare(_Db, _Ref, _Dest, _Sql) -> erlang:nif_error(nif_library_not_loaded). %% @doc %% --spec multi_step(esqlite:connection(), esqlite:statement(), pos_integer(), reference(), pid()) -> ok | {error, any()}. +-spec multi_step(raw_connection(), raw_statement(), pos_integer(), reference(), pid()) -> ok | {error, _}. multi_step(_Db, _Stmt, _Chunk_Size, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). %% @doc %% --spec reset(esqlite:connection(), esqlite:statement(), reference(), pid()) -> ok | {error, any()}. +-spec reset(raw_connection(), raw_statement(), reference(), pid()) -> ok | {error, _}. reset(_Db, _Stmt, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). %% @doc %% --spec finalize(esqlite:connection(), esqlite:statement(), reference(), pid()) -> ok | {error, any()}. +-spec finalize(raw_connection(), raw_statement(), reference(), pid()) -> ok | {error, _}. finalize(_Db, _Stmt, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). %% @doc Bind parameters to a prepared statement. %% --spec bind(esqlite:connection(), esqlite:statement(), reference(), pid(), list(any())) -> ok | {error, any()}. +-spec bind(raw_connection(), raw_statement(), reference(), pid(), list(any())) -> ok | {error, _}. bind(_Db, _Stmt, _Ref, _Dest, _Args) -> erlang:nif_error(nif_library_not_loaded). %% @doc Retrieve the column names of the prepared statement %% --spec column_names(esqlite:connection(), esqlite:statement(), reference(), pid()) -> ok | {error, any()}. +-spec column_names(raw_connection(), raw_statement(), reference(), pid()) -> ok | {error, _}. column_names(_Db, _Stmt, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). %% @doc Retrieve the column types of the prepared statement %% --spec column_types(esqlite:connection(), esqlite:statement(), reference(), pid()) -> ok | {error, any()}. +-spec column_types(raw_connection(), raw_statement(), reference(), pid()) -> ok | {error, _}. column_types(_Db, _Stmt, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). %% @doc Interrupt all active queries. +-spec interrupt(raw_connection()) -> ok. interrupt(_Db) -> erlang:nif_error(nif_library_not_loaded). %% @doc Close the connection. %% --spec close(esqlite:connection(), reference(), pid()) -> ok | {error, any()}. +-spec close(raw_connection(), reference(), pid()) -> ok | {error, _}. close(_Db, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). %% @doc Insert record %% --spec insert(esqlite:connection(), reference(), pid(), esqlite:sql()) -> ok | {error, any()}. +-spec insert(raw_connection(), reference(), pid(), esqlite:sql()) -> ok | {error, _}. insert(_Db, _Ref, _Dest, _Sql) -> erlang:nif_error(nif_library_not_loaded). %% @doc Get automcommit %% --spec get_autocommit(esqlite:connection(), reference(), pid()) -> ok | {error, any()}. +-spec get_autocommit(raw_connection(), reference(), pid()) -> ok | {error, _}. get_autocommit(_Db, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). + From 49aff0cf9a09c8ce4ad311242ab660c06f69db71 Mon Sep 17 00:00:00 2001 From: Maas-Maarten Zeeman Date: Mon, 3 Jan 2022 23:33:37 +0100 Subject: [PATCH 2/6] Added extra check for closed database in update_hook --- c_src/esqlite3_nif.c | 193 ++++++++++++++++++++++--------------------- src/esqlite3.erl | 2 +- 2 files changed, 100 insertions(+), 95 deletions(-) diff --git a/c_src/esqlite3_nif.c b/c_src/esqlite3_nif.c index fd335b1..a3c2009 100644 --- a/c_src/esqlite3_nif.c +++ b/c_src/esqlite3_nif.c @@ -309,14 +309,19 @@ update_callback(void *arg, int sqlite_operation_type, char const *sqlite_databas } static ERL_NIF_TERM -do_set_update_hook(ErlNifEnv *env, esqlite_connection *db, const ERL_NIF_TERM arg) +do_set_update_hook(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) { - if(!enif_get_local_pid(env, arg, &db->notification_pid)) - return make_error_tuple(env, "invalid_pid"); + int rc; + + if(!enif_get_local_pid(env, arg, &conn->notification_pid)) { + return make_error_tuple(env, "invalid_pid"); + } - sqlite3_update_hook(db->db, NULL, NULL); - if(sqlite3_update_hook(db->db, update_callback, db) != SQLITE_OK) - return make_error_tuple(env, "sqlite3_update_hook_fail"); + if(!conn->db) { + return make_error_tuple(env, "closed"); + } + + sqlite3_update_hook(conn->db, update_callback, conn); return make_atom(env, "ok"); } @@ -707,34 +712,34 @@ evaluate_command(esqlite_command *cmd, esqlite_connection *conn) } switch(cmd->type) { - case cmd_open: - return do_open(cmd->env, conn, cmd->arg); - case cmd_update_hook_set: - return do_set_update_hook(cmd->env, conn, cmd->arg); - case cmd_exec: - return do_exec(cmd->env, conn, cmd->arg); - case cmd_changes: - return do_changes(cmd->env, conn, cmd->arg); - case cmd_prepare: - return do_prepare(cmd->env, conn, cmd->arg); - case cmd_multi_step: - return do_multi_step(cmd->env, conn->db, stmt->statement, cmd->arg); - case cmd_reset: - return do_reset(cmd->env, conn->db, stmt->statement); - case cmd_bind: - return do_bind(cmd->env, conn->db, stmt->statement, cmd->arg); - case cmd_column_names: - return do_column_names(cmd->env, stmt->statement); - case cmd_column_types: - return do_column_types(cmd->env, stmt->statement); - case cmd_close: - return do_close(cmd->env, conn, cmd->arg); - case cmd_insert: - return do_insert(cmd->env, conn, cmd->arg); - case cmd_get_autocommit: - return do_get_autocommit(cmd->env, conn); - default: - return make_error_tuple(cmd->env, "invalid_command"); + case cmd_open: + return do_open(cmd->env, conn, cmd->arg); + case cmd_update_hook_set: + return do_set_update_hook(cmd->env, conn, cmd->arg); + case cmd_exec: + return do_exec(cmd->env, conn, cmd->arg); + case cmd_changes: + return do_changes(cmd->env, conn, cmd->arg); + case cmd_prepare: + return do_prepare(cmd->env, conn, cmd->arg); + case cmd_multi_step: + return do_multi_step(cmd->env, conn->db, stmt->statement, cmd->arg); + case cmd_reset: + return do_reset(cmd->env, conn->db, stmt->statement); + case cmd_bind: + return do_bind(cmd->env, conn->db, stmt->statement, cmd->arg); + case cmd_column_names: + return do_column_names(cmd->env, stmt->statement); + case cmd_column_types: + return do_column_types(cmd->env, stmt->statement); + case cmd_close: + return do_close(cmd->env, conn, cmd->arg); + case cmd_insert: + return do_insert(cmd->env, conn, cmd->arg); + case cmd_get_autocommit: + return do_get_autocommit(cmd->env, conn); + default: + return make_error_tuple(cmd->env, "invalid_command"); } } @@ -760,17 +765,17 @@ esqlite_connection_run(void *arg) int continue_running = 1; while(continue_running) { - cmd = queue_pop(db->commands); + cmd = queue_pop(db->commands); - if(cmd->type == cmd_stop) { - continue_running = 0; + if(cmd->type == cmd_stop) { + continue_running = 0; } else if(cmd->type == cmd_notification) { enif_send(NULL, &db->notification_pid, cmd->env, cmd->arg); } else { - enif_send(NULL, &cmd->pid, cmd->env, make_answer(cmd, evaluate_command(cmd, db))); + enif_send(NULL, &cmd->pid, cmd->env, make_answer(cmd, evaluate_command(cmd, db))); } - command_destroy(cmd); + command_destroy(cmd); } return NULL; @@ -788,22 +793,22 @@ esqlite_start(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) /* Initialize the resource */ conn = enif_alloc_resource(esqlite_connection_type, sizeof(esqlite_connection)); if(!conn) - return make_error_tuple(env, "no_memory"); + return make_error_tuple(env, "no_memory"); conn->db = NULL; /* Create command queue */ conn->commands = queue_create(); if(!conn->commands) { - enif_release_resource(conn); - return make_error_tuple(env, "command_queue_create_failed"); + enif_release_resource(conn); + return make_error_tuple(env, "command_queue_create_failed"); } /* Start command processing thread */ conn->opts = enif_thread_opts_create("esqldb_thread_opts"); if(enif_thread_create("esqlite_connection", &conn->tid, esqlite_connection_run, conn, conn->opts) != 0) { - enif_release_resource(conn); - return make_error_tuple(env, "thread_create_failed"); + enif_release_resource(conn); + return make_error_tuple(env, "thread_create_failed"); } db_conn = enif_make_resource(env, conn); @@ -823,21 +828,21 @@ esqlite_open(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) ErlNifPid pid; if(argc != 4) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &db)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[1])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[2], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); if(!sqlite3_threadsafe()) - return make_error_tuple(env, "sqlite3 not thread safe."); + return make_error_tuple(env, "sqlite3 not thread safe."); /* Note, no check is made for the type of the argument */ cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); cmd->type = cmd_open; cmd->ref = enif_make_copy(cmd->env, argv[1]); @@ -855,17 +860,17 @@ set_update_hook(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) ErlNifPid pid; if(argc != 4) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &db)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[1])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[2], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); /* command */ cmd->type = cmd_update_hook_set; @@ -1007,17 +1012,17 @@ esqlite_prepare(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) ErlNifPid pid; if(argc != 4) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &conn)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[1])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[2], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); cmd->type = cmd_prepare; cmd->ref = enif_make_copy(cmd->env, argv[1]); @@ -1039,20 +1044,20 @@ esqlite_bind(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) ErlNifPid pid; if(argc != 5) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &conn)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[1], esqlite_statement_type, (void **) &stmt)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[2])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[3], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); cmd->type = cmd_bind; cmd->ref = enif_make_copy(cmd->env, argv[2]); @@ -1121,21 +1126,21 @@ esqlite_reset(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) ErlNifPid pid; if(argc != 4) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &conn)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[1], esqlite_statement_type, (void **) &stmt)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[2])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[3], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); if(!stmt->statement) - return make_error_tuple(env, "no_prepared_statement"); + return make_error_tuple(env, "no_prepared_statement"); cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); cmd->type = cmd_reset; cmd->ref = enif_make_copy(cmd->env, argv[2]); @@ -1157,21 +1162,21 @@ esqlite_column_names(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) ErlNifPid pid; if(argc != 4) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &conn)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[1], esqlite_statement_type, (void **) &stmt)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[2])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[3], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); if(!stmt->statement) - return make_error_tuple(env, "no_prepared_statement"); + return make_error_tuple(env, "no_prepared_statement"); cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); cmd->type = cmd_column_names; cmd->ref = enif_make_copy(cmd->env, argv[2]); @@ -1196,20 +1201,20 @@ esqlite_column_types(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) return enif_make_badarg(env); if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &conn)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_get_resource(env, argv[1], esqlite_statement_type, (void **) &stmt)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[2])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[3], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); if(!stmt->statement) - return make_error_tuple(env, "no_prepared_statement"); + return make_error_tuple(env, "no_prepared_statement"); cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); cmd->type = cmd_column_types; cmd->ref = enif_make_copy(cmd->env, argv[2]); @@ -1248,15 +1253,15 @@ esqlite_close(ErlNifEnv *env, int argc, const ERL_NIF_TERM argv[]) ErlNifPid pid; if(!enif_get_resource(env, argv[0], esqlite_connection_type, (void **) &conn)) - return enif_make_badarg(env); + return enif_make_badarg(env); if(!enif_is_ref(env, argv[1])) - return make_error_tuple(env, "invalid_ref"); + return make_error_tuple(env, "invalid_ref"); if(!enif_get_local_pid(env, argv[2], &pid)) - return make_error_tuple(env, "invalid_pid"); + return make_error_tuple(env, "invalid_pid"); cmd = command_create(); if(!cmd) - return make_error_tuple(env, "command_create_failed"); + return make_error_tuple(env, "command_create_failed"); cmd->type = cmd_close; cmd->ref = enif_make_copy(cmd->env, argv[1]); @@ -1274,15 +1279,15 @@ on_load(ErlNifEnv* env, void** priv, ERL_NIF_TERM info) ErlNifResourceType *rt; rt = enif_open_resource_type(env, "esqlite3_nif", "esqlite_connection_type", - destruct_esqlite_connection, ERL_NIF_RT_CREATE, NULL); + destruct_esqlite_connection, ERL_NIF_RT_CREATE, NULL); if(!rt) - return -1; + return -1; esqlite_connection_type = rt; rt = enif_open_resource_type(env, "esqlite3_nif", "esqlite_statement_type", - destruct_esqlite_statement, ERL_NIF_RT_CREATE, NULL); + destruct_esqlite_statement, ERL_NIF_RT_CREATE, NULL); if(!rt) - return -1; + return -1; esqlite_statement_type = rt; atom_esqlite3 = make_atom(env, "esqlite3"); diff --git a/src/esqlite3.erl b/src/esqlite3.erl index a1393ec..6663da0 100644 --- a/src/esqlite3.erl +++ b/src/esqlite3.erl @@ -447,7 +447,7 @@ step(#statement{raw_statement=RawStatement, raw_connection=RawConnection}, Timeo {rows, list(tuple())} | {'$busy', list(tuple())} | {'$done', list(tuple())} | - {error, term()}. + {error, _}. multi_step(#statement{raw_statement=RawStatement, raw_connection=RawConnection}, ChunkSize, Timeout) -> Ref = make_ref(), ok = esqlite3_nif:multi_step(RawConnection, RawStatement, ChunkSize, Ref, self()), From 6646fa1a0a84e24e891a0790b9a29759ec0f111e Mon Sep 17 00:00:00 2001 From: Maas-Maarten Zeeman Date: Mon, 3 Jan 2022 23:49:43 +0100 Subject: [PATCH 3/6] Added tests for closed database behaviour --- c_src/esqlite3_nif.c | 21 ++++++++++++++------- test/esqlite_test.erl | 12 ++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/c_src/esqlite3_nif.c b/c_src/esqlite3_nif.c index a3c2009..7fd0ac6 100644 --- a/c_src/esqlite3_nif.c +++ b/c_src/esqlite3_nif.c @@ -335,12 +335,11 @@ do_exec(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) int rc; ERL_NIF_TERM eos = enif_make_int(env, 0); - enif_inspect_iolist_as_binary(env, - enif_make_list2(env, arg, eos), &bin); + enif_inspect_iolist_as_binary(env, enif_make_list2(env, arg, eos), &bin); rc = sqlite3_exec(conn->db, (char *) bin.data, NULL, NULL, NULL); if(rc != SQLITE_OK) - return make_sqlite3_error_tuple(env, rc, conn->db); + return make_sqlite3_error_tuple(env, rc, conn->db); return make_atom(env, "ok"); } @@ -351,6 +350,10 @@ do_exec(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) static ERL_NIF_TERM do_changes(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) { + if(!conn->db) { + return make_error_tuple(env, "closed"); + } + int changes = sqlite3_changes(conn->db); ERL_NIF_TERM changes_term = enif_make_int64(env, changes); @@ -499,11 +502,15 @@ do_bind(ErlNifEnv *env, sqlite3 *db, sqlite3_stmt *stmt, const ERL_NIF_TERM arg) static ERL_NIF_TERM do_get_autocommit(ErlNifEnv *env, esqlite_connection *conn) { + if(!conn->db) { + return make_error_tuple(env, "closed"); + } + if(sqlite3_get_autocommit(conn->db) != 0) { return make_atom(env, "true"); - } else { - return make_atom(env, "false"); - } + } + + return make_atom(env, "false"); } static ERL_NIF_TERM @@ -694,7 +701,7 @@ do_close(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) rc = sqlite3_close_v2(conn->db); if(rc != SQLITE_OK) - return make_sqlite3_error_tuple(env, rc, conn->db); + return make_sqlite3_error_tuple(env, rc, conn->db); conn->db = NULL; return make_atom(env, "ok"); diff --git a/test/esqlite_test.erl b/test/esqlite_test.erl index d4f37bd..dc0281c 100644 --- a/test/esqlite_test.erl +++ b/test/esqlite_test.erl @@ -10,6 +10,18 @@ open_single_database_test() -> {ok, _C1} = esqlite3:open("test.db"), ok. +close_test() -> + {ok, C} = esqlite3:open(":memory:"), + ok = esqlite3:close(C), + + {error, closed} = esqlite3:set_update_hook(self(), C), + {error, closed} = esqlite3:changes(C), + {error, closed} = esqlite3:get_autocommit(C), + + {error, _} = esqlite3:exec("create table test(one, two, three)", C), + + ok. + open_multiple_same_databases_test() -> {ok, _C1} = esqlite3:open("test.db"), {ok, _C2} = esqlite3:open("test.db"), From 4e56d6cb9c47694e87e5946250177fe9088dfe03 Mon Sep 17 00:00:00 2001 From: Maas-Maarten Zeeman Date: Tue, 4 Jan 2022 10:22:59 +0100 Subject: [PATCH 4/6] Whitespace --- c_src/esqlite3_nif.c | 72 ++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/c_src/esqlite3_nif.c b/c_src/esqlite3_nif.c index 7fd0ac6..916a77a 100644 --- a/c_src/esqlite3_nif.c +++ b/c_src/esqlite3_nif.c @@ -520,8 +520,8 @@ make_binary(ErlNifEnv *env, const void *bytes, unsigned int size) ERL_NIF_TERM term; if(!enif_alloc_binary(size, &blob)) { - /* TODO: fix this */ - return make_atom(env, "error"); + /* TODO: fix this */ + return make_atom(env, "error"); } memcpy(blob.data, bytes, size); @@ -537,21 +537,21 @@ make_cell(ErlNifEnv *env, sqlite3_stmt *statement, unsigned int i) int type = sqlite3_column_type(statement, i); switch(type) { - case SQLITE_INTEGER: - return enif_make_int64(env, sqlite3_column_int64(statement, i)); - case SQLITE_FLOAT: - return enif_make_double(env, sqlite3_column_double(statement, i)); - case SQLITE_BLOB: - return enif_make_tuple2(env, make_atom(env, "blob"), - make_binary(env, sqlite3_column_blob(statement, i), - sqlite3_column_bytes(statement, i))); - case SQLITE_NULL: - return make_atom(env, "undefined"); - case SQLITE_TEXT: - return make_binary(env, sqlite3_column_text(statement, i), - sqlite3_column_bytes(statement, i)); - default: - return make_atom(env, "should_not_happen"); + case SQLITE_INTEGER: + return enif_make_int64(env, sqlite3_column_int64(statement, i)); + case SQLITE_FLOAT: + return enif_make_double(env, sqlite3_column_double(statement, i)); + case SQLITE_BLOB: + return enif_make_tuple2(env, make_atom(env, "blob"), + make_binary(env, sqlite3_column_blob(statement, i), + sqlite3_column_bytes(statement, i))); + case SQLITE_NULL: + return make_atom(env, "undefined"); + case SQLITE_TEXT: + return make_binary(env, sqlite3_column_text(statement, i), + sqlite3_column_bytes(statement, i)); + default: + return make_atom(env, "should_not_happen"); } } @@ -593,25 +593,25 @@ do_multi_step(ErlNifEnv *env, sqlite3 *db, sqlite3_stmt *stmt, const ERL_NIF_TER } switch(rc) { - case SQLITE_ROW: - status = make_atom(env, "rows"); - break; - case SQLITE_BUSY: - status = make_atom(env, "$busy"); - break; - case SQLITE_DONE: - /* - * Automatically reset the statement after a done so - * column_names will work after the statement is done. - * - * Not resetting the statement can lead to vm crashes. - */ - sqlite3_reset(stmt); - status = make_atom(env, "$done"); - break; - default: - /* We use prepare_v2, so any error code can be returned. */ - return make_sqlite3_error_tuple(env, rc, db); + case SQLITE_ROW: + status = make_atom(env, "rows"); + break; + case SQLITE_BUSY: + status = make_atom(env, "$busy"); + break; + case SQLITE_DONE: + /* + * Automatically reset the statement after a done so + * column_names will work after the statement is done. + * + * Not resetting the statement can lead to vm crashes. + */ + sqlite3_reset(stmt); + status = make_atom(env, "$done"); + break; + default: + /* We use prepare_v2, so any error code can be returned. */ + return make_sqlite3_error_tuple(env, rc, db); } enif_free(rowBuffer); From 1428e5396c497c303ee652ea1f2ea6bfa224e0c6 Mon Sep 17 00:00:00 2001 From: Maas-Maarten Zeeman Date: Tue, 4 Jan 2022 11:00:28 +0100 Subject: [PATCH 5/6] Fix return type problem --- rebar.config.script | 14 +++++++++++++- src/esqlite3.erl | 8 ++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/rebar.config.script b/rebar.config.script index 358e71c..47df1a2 100644 --- a/rebar.config.script +++ b/rebar.config.script @@ -24,6 +24,9 @@ CFlags = [ {require_min_otp_vsn, "21"}, + + {xref_checks, [undefined_function_calls]}, + {port_env, [ %% Default darwin ldflags causes loading of system sqlite. Removed -bundle flag. {"darwin", "DRV_LDFLAGS", @@ -46,5 +49,14 @@ CFlags = {provider_hooks, [{post, [{compile, {pc, compile}}, - {clean, {pc, clean}}]}]} + {clean, {pc, clean}}]}]}, + + {dialyzer, [ + {warnings, [ + unmatched_returns, + error_handling, + race_conditions + % underspecs + ]} + ]} ]. diff --git a/src/esqlite3.erl b/src/esqlite3.erl index 6663da0..9e091b6 100644 --- a/src/esqlite3.erl +++ b/src/esqlite3.erl @@ -361,8 +361,12 @@ exec(Sql, [], #connection{raw_connection=RawConnection}, Timeout) -> receive_answer(RawConnection, Ref, Timeout); exec(Sql, Params, Connection, Timeout) -> {ok, Statement} = prepare(Sql, Connection, Timeout), - bind(Statement, Params), - step(Statement, Timeout). + case bind(Statement, Params) of + ok -> + step(Statement, Timeout); + {error, _}=Error -> + Error + end. %% @doc Return the number of affected rows of last statement. From b8882581ac70a5dfeb2d58ce9678e47bbead9816 Mon Sep 17 00:00:00 2001 From: Maas-Maarten Zeeman Date: Tue, 4 Jan 2022 11:26:35 +0100 Subject: [PATCH 6/6] Fix merge --- c_src/esqlite3_nif.c | 13 +++++++++++-- src/esqlite3.erl | 8 ++++---- src/esqlite3_nif.erl | 2 +- test/esqlite_test.erl | 3 +++ 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/c_src/esqlite3_nif.c b/c_src/esqlite3_nif.c index 029fd98..56556a4 100644 --- a/c_src/esqlite3_nif.c +++ b/c_src/esqlite3_nif.c @@ -312,8 +312,6 @@ update_callback(void *arg, int sqlite_operation_type, char const *sqlite_databas static ERL_NIF_TERM do_set_update_hook(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) { - int rc; - if(!enif_get_local_pid(env, arg, &conn->notification_pid)) { return make_error_tuple(env, "invalid_pid"); } @@ -382,15 +380,24 @@ do_insert(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) return make_ok_tuple(env, last_rowid_term); } +/* + * Return the last inserted rowid + */ static ERL_NIF_TERM do_last_insert_rowid(ErlNifEnv *env, esqlite_connection *conn) { + if(!conn->db) { + return make_error_tuple(env, "closed"); + } + sqlite3_int64 last_rowid = sqlite3_last_insert_rowid(conn->db); ERL_NIF_TERM last_rowid_term = enif_make_int64(env, last_rowid); + return make_ok_tuple(env, last_rowid_term); } /* + * Compile a sql statement */ static ERL_NIF_TERM do_prepare(ErlNifEnv *env, esqlite_connection *conn, const ERL_NIF_TERM arg) @@ -750,6 +757,8 @@ evaluate_command(esqlite_command *cmd, esqlite_connection *conn) return do_column_types(cmd->env, stmt->statement); case cmd_close: return do_close(cmd->env, conn, cmd->arg); + case cmd_last_insert_rowid: + return do_last_insert_rowid(cmd->env, conn); case cmd_insert: return do_insert(cmd->env, conn, cmd->arg); case cmd_get_autocommit: diff --git a/src/esqlite3.erl b/src/esqlite3.erl index 52b2db3..f28e7d5 100644 --- a/src/esqlite3.erl +++ b/src/esqlite3.erl @@ -402,11 +402,11 @@ last_insert_rowid(Connection) -> %% @doc Get the last insert rowid. %% --spec last_insert_rowid(connection(), timeout()) -> {ok, integer()} | {error, _}. -last_insert_rowid({connection, _Ref, Connection}, Timeout) -> +-spec last_insert_rowid(connection(), timeout()) -> {ok, rowid()} | {error, _}. +last_insert_rowid(#connection{raw_connection=RawConnection}, Timeout) -> Ref = make_ref(), - ok = esqlite3_nif:last_insert_rowid(Connection, Ref, self()), - receive_answer(Connection, Ref, Timeout). + ok = esqlite3_nif:last_insert_rowid(RawConnection, Ref, self()), + receive_answer(RawConnection, Ref, Timeout). %% @doc Get autocommit %% @doc Check if the connection is in auto-commit mode. diff --git a/src/esqlite3_nif.erl b/src/esqlite3_nif.erl index ea8ea86..5276da9 100644 --- a/src/esqlite3_nif.erl +++ b/src/esqlite3_nif.erl @@ -155,7 +155,7 @@ insert(_Db, _Ref, _Dest, _Sql) -> %% @doc Get the last insert rowid. %% -%% @spec insert(connection(), Ref::reference(), Dest::pid()) -> {ok, integer()} | {error, message()} +-spec last_insert_rowid(raw_connection(), reference(), pid()) -> ok | {error, _}. last_insert_rowid(_Db, _Ref, _Dest) -> erlang:nif_error(nif_library_not_loaded). diff --git a/test/esqlite_test.erl b/test/esqlite_test.erl index f288588..5addee7 100644 --- a/test/esqlite_test.erl +++ b/test/esqlite_test.erl @@ -11,12 +11,15 @@ open_single_database_test() -> ok. close_test() -> + %% Open and close the database immediately {ok, C} = esqlite3:open(":memory:"), ok = esqlite3:close(C), + %% Check if functions still return sensible values. {error, closed} = esqlite3:set_update_hook(self(), C), {error, closed} = esqlite3:changes(C), {error, closed} = esqlite3:get_autocommit(C), + {error, closed} = esqlite3:last_insert_rowid(C), {error, _} = esqlite3:exec("create table test(one, two, three)", C),