From 5b4d2e1ac9378efcc9cf75ac7dd389ed58e9573a Mon Sep 17 00:00:00 2001 From: JonasBa Date: Tue, 20 Aug 2024 09:08:06 -0400 Subject: [PATCH 1/5] src: remove node namespace from sqlite.cc src: remove unnecessary node namespace src: remove string view changes --- src/node_sqlite.cc | 67 +++++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index fbef321dcab16c..5e1c8db33195da 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -6,6 +6,7 @@ #include "node.h" #include "node_errors.h" #include "node_mem-inl.h" +#include "path.h" #include "sqlite3.h" #include "util-inl.h" @@ -47,7 +48,7 @@ using v8::Value; #define THROW_AND_RETURN_ON_BAD_STATE(env, condition, msg) \ do { \ if ((condition)) { \ - node::THROW_ERR_INVALID_STATE((env), (msg)); \ + THROW_ERR_INVALID_STATE((env), (msg)); \ return; \ } \ } while (0) @@ -94,7 +95,7 @@ DatabaseSync::DatabaseSync(Environment* env, bool open) : BaseObject(env, object) { MakeWeak(); - node::Utf8Value utf8_location(env->isolate(), location); + Utf8Value utf8_location(env->isolate(), location); location_ = utf8_location.ToString(); connection_ = nullptr; @@ -117,7 +118,7 @@ void DatabaseSync::MemoryInfo(MemoryTracker* tracker) const { bool DatabaseSync::Open() { if (IsOpen()) { - node::THROW_ERR_INVALID_STATE(env(), "database is already open"); + THROW_ERR_INVALID_STATE(env(), "database is already open"); return false; } @@ -160,8 +161,8 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { } if (!args[0]->IsString()) { - node::THROW_ERR_INVALID_ARG_TYPE(env->isolate(), - "The \"path\" argument must be a string."); + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"path\" argument must be a string."); return; } @@ -169,8 +170,8 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { if (args.Length() > 1) { if (!args[1]->IsObject()) { - node::THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), "The \"options\" argument must be an object."); + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"options\" argument must be an object."); return; } @@ -182,7 +183,7 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { } if (!open_v->IsUndefined()) { if (!open_v->IsBoolean()) { - node::THROW_ERR_INVALID_ARG_TYPE( + THROW_ERR_INVALID_ARG_TYPE( env->isolate(), "The \"options.open\" argument must be a boolean."); return; } @@ -217,12 +218,12 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); if (!args[0]->IsString()) { - node::THROW_ERR_INVALID_ARG_TYPE(env->isolate(), - "The \"sql\" argument must be a string."); + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"sql\" argument must be a string."); return; } - node::Utf8Value sql(env->isolate(), args[0].As()); + auto sql = Utf8Value(env->isolate(), args[0].As()); sqlite3_stmt* s = nullptr; int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, 0); CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void()); @@ -238,12 +239,12 @@ void DatabaseSync::Exec(const FunctionCallbackInfo& args) { THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); if (!args[0]->IsString()) { - node::THROW_ERR_INVALID_ARG_TYPE(env->isolate(), - "The \"sql\" argument must be a string."); + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"sql\" argument must be a string."); return; } - node::Utf8Value sql(env->isolate(), args[0].As()); + auto sql = Utf8Value(env->isolate(), args[0].As()); int r = sqlite3_exec(db->connection_, *sql, nullptr, nullptr, nullptr); CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void()); } @@ -311,7 +312,7 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { if (insertion.second == false) { auto existing_full_name = (*insertion.first).second; if (full_name != existing_full_name) { - node::THROW_ERR_INVALID_STATE( + THROW_ERR_INVALID_STATE( env(), "Cannot create bare named parameter '%s' because of " "conflicting names '%s' and '%s'.", @@ -331,7 +332,7 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { return false; } - node::Utf8Value utf8_key(env()->isolate(), key); + auto utf8_key = Utf8Value(env()->isolate(), key); int r = sqlite3_bind_parameter_index(statement_, *utf8_key); if (r == 0) { if (allow_bare_named_params_) { @@ -343,7 +344,7 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { } if (r == 0) { - node::THROW_ERR_INVALID_STATE( + THROW_ERR_INVALID_STATE( env(), "Unknown named parameter '%s'", *utf8_key); return false; } @@ -387,7 +388,7 @@ bool StatementSync::BindValue(const Local& value, const int index) { double val = value.As()->Value(); r = sqlite3_bind_double(statement_, index, val); } else if (value->IsString()) { - node::Utf8Value val(env()->isolate(), value.As()); + auto val = Utf8Value(env()->isolate(), value.As()); r = sqlite3_bind_text( statement_, index, *val, val.length(), SQLITE_TRANSIENT); } else if (value->IsNull()) { @@ -400,13 +401,12 @@ bool StatementSync::BindValue(const Local& value, const int index) { bool lossless; int64_t as_int = value.As()->Int64Value(&lossless); if (!lossless) { - node::THROW_ERR_INVALID_ARG_VALUE(env(), - "BigInt value is too large to bind."); + THROW_ERR_INVALID_ARG_VALUE(env(), "BigInt value is too large to bind."); return false; } r = sqlite3_bind_int64(statement_, index, as_int); } else { - node::THROW_ERR_INVALID_ARG_TYPE( + THROW_ERR_INVALID_ARG_TYPE( env()->isolate(), "Provided value cannot be bound to SQLite parameter %d.", index); @@ -432,7 +432,7 @@ MaybeLocal StatementSync::ColumnToValue(const int column) { "represented as a JavaScript number: %" PRId64, column, value); - return MaybeLocal(); + return {}; } } case SQLITE_FLOAT: @@ -441,7 +441,11 @@ MaybeLocal StatementSync::ColumnToValue(const int column) { case SQLITE_TEXT: { const char* value = reinterpret_cast( sqlite3_column_text(statement_, column)); - return String::NewFromUtf8(env()->isolate(), value).As(); + Local val; + if (!String::NewFromUtf8(env()->isolate(), value).ToLocal(&val)) { + return {}; + } + return val; } case SQLITE_NULL: return Null(env()->isolate()); @@ -463,12 +467,15 @@ MaybeLocal StatementSync::ColumnToValue(const int column) { MaybeLocal StatementSync::ColumnNameToName(const int column) { const char* col_name = sqlite3_column_name(statement_, column); if (col_name == nullptr) { - node::THROW_ERR_INVALID_STATE( - env(), "Cannot get name of column %d", column); - return MaybeLocal(); + THROW_ERR_INVALID_STATE(env(), "Cannot get name of column %d", column); + return {}; } - return String::NewFromUtf8(env()->isolate(), col_name).As(); + Local key; + if (!String::NewFromUtf8(env()->isolate(), col_name).ToLocal(&key)) { + return {}; + } + return key; } void StatementSync::MemoryInfo(MemoryTracker* tracker) const {} @@ -657,7 +664,7 @@ void StatementSync::SetAllowBareNamedParameters( env, stmt->IsFinalized(), "statement has been finalized"); if (!args[0]->IsBoolean()) { - node::THROW_ERR_INVALID_ARG_TYPE( + THROW_ERR_INVALID_ARG_TYPE( env->isolate(), "The \"allowBareNamedParameters\" argument must be a boolean."); return; @@ -674,7 +681,7 @@ void StatementSync::SetReadBigInts(const FunctionCallbackInfo& args) { env, stmt->IsFinalized(), "statement has been finalized"); if (!args[0]->IsBoolean()) { - node::THROW_ERR_INVALID_ARG_TYPE( + THROW_ERR_INVALID_ARG_TYPE( env->isolate(), "The \"readBigInts\" argument must be a boolean."); return; } @@ -683,7 +690,7 @@ void StatementSync::SetReadBigInts(const FunctionCallbackInfo& args) { } void IllegalConstructor(const FunctionCallbackInfo& args) { - node::THROW_ERR_ILLEGAL_CONSTRUCTOR(Environment::GetCurrent(args)); + THROW_ERR_ILLEGAL_CONSTRUCTOR(Environment::GetCurrent(args)); } Local StatementSync::GetConstructorTemplate( From 4ded9ab4454e9669950ed7b7617240582c28e511 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 22 Aug 2024 11:26:56 -0400 Subject: [PATCH 2/5] src: use string view instead of local This reverts commit 1340a81744b5a934b4ea0423bc01d424fa152127. src: use direct initialzation and const string view signature --- src/node_sqlite.cc | 14 +++++++++----- src/node_sqlite.h | 3 ++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 5e1c8db33195da..2a02012e096472 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -9,8 +9,10 @@ #include "path.h" #include "sqlite3.h" #include "util-inl.h" +#include "util.h" #include +#include namespace node { namespace sqlite { @@ -91,12 +93,11 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) { DatabaseSync::DatabaseSync(Environment* env, Local object, - Local location, + const std::string_view location, bool open) : BaseObject(env, object) { MakeWeak(); - Utf8Value utf8_location(env->isolate(), location); - location_ = utf8_location.ToString(); + location_ = std::string(location); connection_ = nullptr; if (open) { @@ -191,7 +192,10 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { } } - new DatabaseSync(env, args.This(), args[0].As(), open); + BufferValue location(env->isolate(), args[0]); + CHECK_NOT_NULL(*location); + ToNamespacedPath(env, &location); + new DatabaseSync(env, args.This(), location.ToStringView(), open); } void DatabaseSync::Open(const FunctionCallbackInfo& args) { @@ -388,7 +392,7 @@ bool StatementSync::BindValue(const Local& value, const int index) { double val = value.As()->Value(); r = sqlite3_bind_double(statement_, index, val); } else if (value->IsString()) { - auto val = Utf8Value(env()->isolate(), value.As()); + Utf8Value val(env()->isolate(), value.As()); r = sqlite3_bind_text( statement_, index, *val, val.length(), SQLITE_TRANSIENT); } else if (value->IsNull()) { diff --git a/src/node_sqlite.h b/src/node_sqlite.h index f62b2e991cb95a..8cca5f58e77066 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -9,6 +9,7 @@ #include "util.h" #include +#include #include namespace node { @@ -20,7 +21,7 @@ class DatabaseSync : public BaseObject { public: DatabaseSync(Environment* env, v8::Local object, - v8::Local location, + std::string_view location, bool open); void MemoryInfo(MemoryTracker* tracker) const override; static void New(const v8::FunctionCallbackInfo& args); From a773f1152bc8e9d5ec9726c053cb6092ce903460 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Fri, 23 Aug 2024 20:53:59 -0400 Subject: [PATCH 3/5] fixup! src: remove node namespace from sqlite.cc --- src/node_sqlite.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 2a02012e096472..7ebfe7e29628b3 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -9,7 +9,6 @@ #include "path.h" #include "sqlite3.h" #include "util-inl.h" -#include "util.h" #include #include From 94803b17f5bd0ebd95743bdb8140c087122fabf4 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 26 Sep 2024 20:56:00 -0400 Subject: [PATCH 4/5] fixup! src: remove node namespace from sqlite.cc --- src/node_sqlite.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 7ebfe7e29628b3..f86c275f54b64d 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -9,6 +9,7 @@ #include "path.h" #include "sqlite3.h" #include "util-inl.h" +#include "util.h" #include #include @@ -226,7 +227,7 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { return; } - auto sql = Utf8Value(env->isolate(), args[0].As()); + Utf8Value sql(env->isolate(), args[0].As()); sqlite3_stmt* s = nullptr; int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, 0); CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void()); @@ -247,7 +248,7 @@ void DatabaseSync::Exec(const FunctionCallbackInfo& args) { return; } - auto sql = Utf8Value(env->isolate(), args[0].As()); + Utf8Value sql(env->isolate(), args[0].As()); int r = sqlite3_exec(db->connection_, *sql, nullptr, nullptr, nullptr); CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void()); } @@ -335,7 +336,7 @@ bool StatementSync::BindParams(const FunctionCallbackInfo& args) { return false; } - auto utf8_key = Utf8Value(env()->isolate(), key); + Utf8Value utf8_key(env()->isolate(), key); int r = sqlite3_bind_parameter_index(statement_, *utf8_key); if (r == 0) { if (allow_bare_named_params_) { From 18e5be3e6f6753c0496dd3454640f414ba2e9f5a Mon Sep 17 00:00:00 2001 From: JonasBa Date: Fri, 27 Sep 2024 11:44:39 -0400 Subject: [PATCH 5/5] fixup! src: remove node namespace from sqlite.cc --- src/node_sqlite.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index f86c275f54b64d..4d7d566af8fe0c 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -9,7 +9,6 @@ #include "path.h" #include "sqlite3.h" #include "util-inl.h" -#include "util.h" #include #include