Skip to content

Commit d94d4e0

Browse files
indutny-signalindutny-signal
committed
Feedback
1 parent a461c5f commit d94d4e0

File tree

3 files changed

+91
-89
lines changed

3 files changed

+91
-89
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
"build:docs": "typedoc lib/index.ts --includeVersion",
4242
"install": "node-gyp-build",
4343
"prebuildify": "prebuildify --strip --napi",
44-
"test": "vitest --coverage",
44+
"test": "vitest --coverage --pool threads",
4545
"format": "run-p --print-label format:c format:js",
4646
"format:c": "xcrun clang-format --style=chromium -Werror --verbose -i src/*.cc",
4747
"format:js": "prettier --cache --write .",

‎src/addon.cc‎

Lines changed: 56 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414

1515
class SignalTokenizerModule {
1616
public:
17-
static void Destroy(void* p_ctx) {}
17+
static void Destroy(void* p_ctx) {
18+
delete static_cast<SignalTokenizerModule*>(p_ctx);
19+
}
1820

19-
inline fts5_tokenizer* GetAPIObject() { return &api_object; }
21+
static fts5_tokenizer api_object;
2022

2123
private:
2224
static int Create(void* p_ctx, char const**, int, Fts5Tokenizer** pp_out) {
@@ -26,7 +28,6 @@ class SignalTokenizerModule {
2628
}
2729

2830
static void Delete(Fts5Tokenizer* tokenizer) {}
29-
static fts5_tokenizer api_object;
3031
};
3132

3233
fts5_tokenizer SignalTokenizerModule::api_object = {
@@ -72,6 +73,27 @@ static Napi::Value SignalTokenize(const Napi::CallbackInfo& info) {
7273
return result;
7374
}
7475

76+
// Utils
77+
78+
Napi::Error FormatError(Napi::Env env, const char* format, ...) {
79+
va_list args;
80+
81+
va_start(args, format);
82+
83+
// Get buffer size
84+
auto size = vsnprintf(nullptr, 0, format, args);
85+
86+
// Allocate and fill the string
87+
auto buf = new char[size + 1];
88+
vsnprintf(buf, size + 1, format, args);
89+
90+
va_end(args);
91+
92+
auto err = Napi::Error::New(env, buf);
93+
delete[] buf;
94+
return err;
95+
}
96+
7597
// Database
7698

7799
Napi::Object Database::Init(Napi::Env env, Napi::Object exports) {
@@ -126,10 +148,8 @@ Napi::Value Database::Open(const Napi::CallbackInfo& info) {
126148
sqlite3* handle = nullptr;
127149
int r = sqlite3_open_v2(path_utf8.c_str(), &handle, flags, nullptr);
128150
if (r != SQLITE_OK) {
129-
char buf[1024];
130-
131-
snprintf(buf, sizeof(buf), "sqlite open error: %s", sqlite3_errstr(r));
132-
NAPI_THROW(Napi::Error::New(env, buf), Napi::Value());
151+
NAPI_THROW(FormatError(env, "sqlite open error: %s", sqlite3_errstr(r)),
152+
Napi::Value());
133153
}
134154

135155
auto db = new Database(env, handle);
@@ -145,8 +165,12 @@ Napi::Value Database::Open(const Napi::CallbackInfo& info) {
145165
return Napi::Value();
146166
}
147167
SignalTokenizerModule* icu = new SignalTokenizerModule();
148-
fts5->xCreateTokenizer(fts5, "signal_tokenizer", icu, icu->GetAPIObject(),
149-
&SignalTokenizerModule::Destroy);
168+
r = fts5->xCreateTokenizer(fts5, "signal_tokenizer", icu, &icu->api_object,
169+
&SignalTokenizerModule::Destroy);
170+
if (r != SQLITE_OK) {
171+
delete icu;
172+
return db->ThrowSqliteError(env, r);
173+
}
150174

151175
return db->self_ref_.Value();
152176
}
@@ -204,20 +228,18 @@ Napi::Value Database::Exec(const Napi::CallbackInfo& info) {
204228
}
205229

206230
Napi::Value Database::ThrowSqliteError(Napi::Env env, int error) {
207-
char buf[1024];
208-
209231
assert(handle_ != nullptr);
210232
const char* msg = sqlite3_errmsg(handle_);
211233
int offset = sqlite3_error_offset(handle_);
212234
int extended = sqlite3_extended_errcode(handle_);
213235
if (offset == -1) {
214-
snprintf(buf, sizeof(buf), "sqlite error(%d): %s", extended, msg);
236+
NAPI_THROW(FormatError(env, "sqlite error(%d): %s", extended, msg),
237+
Napi::Value());
215238
} else {
216-
snprintf(buf, sizeof(buf), "sqlite error(%d): %s, offset: %d", extended,
217-
msg, offset);
239+
NAPI_THROW(FormatError(env, "sqlite error(%d): %s, offset: %d", extended,
240+
msg, offset),
241+
Napi::Value());
218242
}
219-
220-
NAPI_THROW(Napi::Error::New(env, buf), Napi::Value());
221243
}
222244

223245
fts5_api* Database::GetFTS5API(Napi::Env env) {
@@ -249,7 +271,9 @@ std::list<Statement*>::const_iterator Database::TrackStatement(
249271
self_ref_.Ref();
250272

251273
statements_.emplace_back(stmt);
252-
return --statements_.end();
274+
auto end = statements_.end();
275+
end--;
276+
return end;
253277
}
254278

255279
void Database::UntrackStatement(std::list<Statement*>::const_iterator iter) {
@@ -492,8 +516,6 @@ Napi::Value Statement::Step(const Napi::CallbackInfo& info) {
492516
}
493517

494518
bool Statement::BindParams(Napi::Env env, Napi::Value params) {
495-
char buf[1024];
496-
497519
int key_count = sqlite3_bind_parameter_count(handle_);
498520

499521
if (params.IsNull()) {
@@ -505,29 +527,29 @@ bool Statement::BindParams(Napi::Env env, Napi::Value params) {
505527
return true;
506528
}
507529

508-
snprintf(buf, sizeof(buf), "Expected %d parameters, got 0", key_count);
509-
NAPI_THROW(Napi::Error::New(env, buf), false);
530+
NAPI_THROW(FormatError(env, "Expected %d parameters, got 0", key_count),
531+
false);
510532
} else if (params.IsArray()) {
511533
auto list = params.As<Napi::Array>();
512534
auto list_len = static_cast<int>(list.Length());
513535
if (list_len != key_count) {
514-
snprintf(buf, sizeof(buf), "Expected %d parameters, got %d", key_count,
515-
list_len);
516-
NAPI_THROW(Napi::Error::New(env, buf), false);
536+
NAPI_THROW(FormatError(env, "Expected %d parameters, got %d", key_count,
537+
list_len),
538+
false);
517539
}
518540

519541
for (int i = 1; i <= list_len; i++) {
520542
auto name = sqlite3_bind_parameter_name(handle_, i);
521543
if (name != nullptr) {
522-
snprintf(buf, sizeof(buf), "Unexpected named param %s at %d", name, i);
523-
NAPI_THROW(Napi::Error::New(env, buf), false);
544+
NAPI_THROW(FormatError(env, "Unexpected named param %s at %d", name, i),
545+
false);
524546
}
525547

526548
auto error = BindParam(env, i, list[i - 1]);
527549
if (error != nullptr) {
528-
snprintf(buf, sizeof(buf), "Failed to bind param %d, error %s", i,
529-
error);
530-
NAPI_THROW(Napi::Error::New(env, buf), false);
550+
NAPI_THROW(
551+
FormatError(env, "Failed to bind param %d, error %s", i, error),
552+
false);
531553
}
532554
}
533555
} else {
@@ -536,18 +558,18 @@ bool Statement::BindParams(Napi::Env env, Napi::Value params) {
536558
for (int i = 1; i <= key_count; i++) {
537559
auto name = sqlite3_bind_parameter_name(handle_, i);
538560
if (name == nullptr) {
539-
snprintf(buf, sizeof(buf), "Unexpected anonymous param at %d", i);
540-
NAPI_THROW(Napi::Error::New(env, buf), false);
561+
NAPI_THROW(FormatError(env, "Unexpected anonymous param at %d", i),
562+
false);
541563
}
542564

543565
// Skip "$"
544566
name = name + 1;
545567
auto value = obj[name];
546568
auto error = BindParam(env, i, value);
547569
if (error != nullptr) {
548-
snprintf(buf, sizeof(buf), "Failed to bind param %s, error %s", name,
549-
error);
550-
NAPI_THROW(Napi::Error::New(env, buf), false);
570+
NAPI_THROW(
571+
FormatError(env, "Failed to bind param %s, error %s", name, error),
572+
false);
551573
}
552574
}
553575
}

‎src/addon.h‎

Lines changed: 34 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -79,61 +79,41 @@ class Statement {
7979
// statements.
8080
//
8181
// If return value `false` - only whitespace and comments remain.
82-
static inline bool HasTail(const char* tail) {
83-
for (; *tail != 0; tail++) {
84-
switch (*tail) {
85-
// Various whitespace
86-
case '\t': // 0x09
87-
case '\r': // 0x0a
88-
case '\v': // 0x0b
89-
case '\f': // 0x0c
90-
case '\n': // 0x0d
91-
case ' ':
92-
93-
// Statement separator
94-
case ';':
95-
break;
96-
97-
// Line comment
98-
case '-':
99-
tail++;
100-
if (*tail != '-') {
101-
return true;
102-
}
103-
104-
tail = strchr(tail, '\n');
105-
if (tail == nullptr) {
106-
return false;
107-
}
108-
109-
break;
110-
82+
//
83+
// Note: we use `rfind(..., 0)` for effectively a prefix check.
84+
static inline bool HasTail(std::string const& tail) {
85+
std::string p(tail);
86+
while (!p.empty()) {
87+
auto ch = p.front();
88+
// Various whitespace or statement separator
89+
if (std::isspace(ch) || ch == ';') {
90+
p = p.substr(1);
91+
92+
} else if (p.rfind("--", 0) == 0) {
93+
// Line comment: "--"
94+
p = p.substr(2);
95+
96+
// Skip until the end of the line
97+
auto end = p.find("\n");
98+
if (end == p.npos) {
99+
return false;
100+
}
101+
102+
p = p.substr(end + 1);
103+
104+
} else if (p.rfind("/*", 0) == 0) {
111105
// Block comment
112-
case '/':
113-
tail++;
114-
if (*tail != '*') {
115-
return true;
116-
}
117-
118-
// Look for closing '*/'
119-
while (true) {
120-
tail = strchr(tail, '*');
121-
if (tail == nullptr) {
122-
return false;
123-
}
124-
125-
tail++;
126-
const char next = *tail;
127-
if (next == 0) {
128-
return false;
129-
}
130-
if (next == '/') {
131-
tail++;
132-
break;
133-
}
134-
}
135-
default:
136-
return true;
106+
p = p.substr(2);
107+
108+
auto end = p.find("*/");
109+
if (end == p.npos) {
110+
return false;
111+
}
112+
113+
p = p.substr(end + 2);
114+
} else {
115+
// Not whitespace or comments
116+
return true;
137117
}
138118
}
139119
return false;

0 commit comments

Comments
 (0)