From 99918c541a69d44f3f79a02ba0657d52d2b090ad Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 13 Oct 2019 09:14:57 +0300 Subject: [PATCH] Refactor initialization of range options Cherry-picked from Level/leveldown#681 --- binding.cc | 158 ++++++++++++++--------------------------------------- 1 file changed, 41 insertions(+), 117 deletions(-) diff --git a/binding.cc b/binding.cc index cbfc729e..4faa7a99 100644 --- a/binding.cc +++ b/binding.cc @@ -238,6 +238,25 @@ static size_t StringOrBufferLength (napi_env env, napi_value value) { return size; } +/** + * Takes a Buffer or string property 'name' from 'opts'. + * Returns null if the property does not exist or is zero-length. + */ +static std::string* RangeOption (napi_env env, napi_value opts, const char* name) { + if (HasProperty(env, opts, name)) { + napi_value value = GetProperty(env, opts, name); + + if (StringOrBufferLength(env, value) > 0) { + LD_STRING_OR_BUFFER_TO_COPY(env, value, to); + std::string* result = new std::string(toCh_, toSz_); + delete [] toCh_; + return result; + } + } + + return NULL; +} + /** * Calls a function. */ @@ -493,7 +512,7 @@ struct PriorityWorker : public BaseWorker { struct Iterator { Iterator (Database* database, uint32_t id, - leveldb::Slice* start, + std::string* start, std::string* end, bool reverse, bool keys, @@ -540,32 +559,14 @@ struct Iterator { ~Iterator () { assert(ended_); ReleaseTarget(); - if (start_ != NULL) { - // Special case for `start` option: it won't be - // freed up by any of the delete calls below. - if (!((lt_ != NULL && reverse_) - || (lte_ != NULL && reverse_) - || (gt_ != NULL && !reverse_) - || (gte_ != NULL && !reverse_))) { - delete [] start_->data(); - } - delete start_; - } - if (end_ != NULL) { - delete end_; - } - if (lt_ != NULL) { - delete lt_; - } - if (gt_ != NULL) { - delete gt_; - } - if (lte_ != NULL) { - delete lte_; - } - if (gte_ != NULL) { - delete gte_; - } + + if (start_ != NULL) delete start_; + if (end_ != NULL) delete end_; + if (lt_ != NULL) delete lt_; + if (gt_ != NULL) delete gt_; + if (lte_ != NULL) delete lte_; + if (gte_ != NULL) delete gte_; + delete options_; } @@ -730,7 +731,7 @@ struct Iterator { Database* database_; uint32_t id_; - leveldb::Slice* start_; + std::string* start_; std::string* end_; bool reverse_; bool keys_; @@ -1302,17 +1303,6 @@ static void FinalizeIterator (napi_env env, void* data, void* hint) { } } -#define CHECK_PROPERTY(name, code) \ - if (HasProperty(env, options, #name)) { \ - napi_value value = GetProperty(env, options, #name); \ - if (IsString(env, value) || IsBuffer(env, value)) { \ - if (StringOrBufferLength(env, value) > 0) { \ - LD_STRING_OR_BUFFER_TO_COPY(env, value, _##name); \ - code; \ - } \ - } \ - } \ - /** * Create an iterator. */ @@ -1331,84 +1321,18 @@ NAPI_METHOD(iterator_init) { uint32_t highWaterMark = Uint32Property(env, options, "highWaterMark", 16 * 1024); - // TODO simplify and refactor the hideous code below - - leveldb::Slice* start = NULL; - char *startStr = NULL; - CHECK_PROPERTY(start, { - start = new leveldb::Slice(_startCh_, _startSz_); - startStr = _startCh_; - }); - - std::string* end = NULL; - CHECK_PROPERTY(end, { - end = new std::string(_endCh_, _endSz_); - delete [] _endCh_; - }); - - std::string* lt = NULL; - CHECK_PROPERTY(lt, { - lt = new std::string(_ltCh_, _ltSz_); - delete [] _ltCh_; - if (reverse) { - if (startStr != NULL) { - delete [] startStr; - startStr = NULL; - } - if (start != NULL) { - delete start; - } - start = new leveldb::Slice(lt->data(), lt->size()); - } - }); - - std::string* lte = NULL; - CHECK_PROPERTY(lte, { - lte = new std::string(_lteCh_, _lteSz_); - delete [] _lteCh_; - if (reverse) { - if (startStr != NULL) { - delete [] startStr; - startStr = NULL; - } - if (start != NULL) { - delete start; - } - start = new leveldb::Slice(lte->data(), lte->size()); - } - }); - - std::string* gt = NULL; - CHECK_PROPERTY(gt, { - gt = new std::string(_gtCh_, _gtSz_); - delete [] _gtCh_; - if (!reverse) { - if (startStr != NULL) { - delete [] startStr; - startStr = NULL; - } - if (start != NULL) { - delete start; - } - start = new leveldb::Slice(gt->data(), gt->size()); - } - }); - - std::string* gte = NULL; - CHECK_PROPERTY(gte, { - gte = new std::string(_gteCh_, _gteSz_); - delete [] _gteCh_; - if (!reverse) { - if (startStr != NULL) { - delete [] startStr; - startStr = NULL; - } - if (start != NULL) { - delete start; - } - start = new leveldb::Slice(gte->data(), gte->size()); - } - }); + std::string* start = NULL; + std::string* end = RangeOption(env, options, "end"); + std::string* lt = RangeOption(env, options, "lt"); + std::string* lte = RangeOption(env, options, "lte"); + std::string* gt = RangeOption(env, options, "gt"); + std::string* gte = RangeOption(env, options, "gte"); + + if (!reverse && gte != NULL) start = new std::string(*gte); + else if (!reverse && gt != NULL) start = new std::string(*gt); + else if (reverse && lte != NULL) start = new std::string(*lte); + else if (reverse && lt != NULL) start = new std::string(*lt); + else start = RangeOption(env, options, "start"); uint32_t id = database->currentIteratorId_++; Iterator* iterator = new Iterator(database, id, start, end, reverse, keys,