From 5e3634fc979827421c5241fed5bbbaed2dc1e929 Mon Sep 17 00:00:00 2001 From: Sylvain Abadie Date: Tue, 26 Jul 2022 11:01:14 -0700 Subject: [PATCH] :wrench: code review --- cpp/packer.cpp | 54 +++++++++++++++++------------------- cpp/packer.h | 7 ++--- cpp/react-native-leveldb.cpp | 12 +++----- 3 files changed, 33 insertions(+), 40 deletions(-) diff --git a/cpp/packer.cpp b/cpp/packer.cpp index 350b935..f4a7260 100644 --- a/cpp/packer.cpp +++ b/cpp/packer.cpp @@ -1,8 +1,10 @@ #include "packer.h" -using namespace::facebook; +namespace Packer { -void Packer::pack(const jsi::Value& value, jsi::Runtime& runtime, mpack_writer_t* writer) { +jsi::String unpackString(jsi::Runtime& runtime, mpack_reader_t* reader, size_t strLength); + +void pack(const jsi::Value& value, jsi::Runtime& runtime, mpack_writer_t* writer) { if(value.isString()) { mpack_write_cstr(writer, value.getString(runtime).utf8(runtime).c_str()); @@ -26,6 +28,11 @@ void Packer::pack(const jsi::Value& value, jsi::Runtime& runtime, mpack_writer_t mpack_write_bytes(writer, "u", 1); mpack_finish_bin(writer); + } else if(value.isSymbol()) { + + mpack_writer_flag_error(writer, mpack_error_data); + throw jsi::JSError(runtime, "pack/ symbols are not supported"); + } else if(value.isObject()) { auto obj = value.getObject(runtime); @@ -72,31 +79,7 @@ void Packer::pack(const jsi::Value& value, jsi::Runtime& runtime, mpack_writer_t } } -jsi::String unpackString(jsi::Runtime& runtime, mpack_reader_t* reader, size_t strLength) { - if (mpack_should_read_bytes_inplace(reader, strLength)) { - const char* data = mpack_read_bytes_inplace(reader, strLength); - - if (mpack_reader_error(reader) != mpack_ok) - throw jsi::JSError(runtime, "unpackKey/ failed to read in-place"); - - mpack_done_str(reader); - return jsi::String::createFromUtf8(runtime,(uint8_t *) data, strLength); - } else { - char* data = (char *) malloc(strLength); - mpack_read_bytes(reader, data, strLength); - if (mpack_reader_error(reader) != mpack_ok) { - free(data); - throw jsi::JSError(runtime, "unpackKey/ failed to read with malloc"); - } - auto key = jsi::String::createFromUtf8(runtime,(uint8_t *) data, strLength); - free(data); - mpack_done_str(reader); - return key; - } -} - - -jsi::Value Packer::unpackElement(jsi::Runtime& runtime, mpack_reader_t* reader, int depth) { +jsi::Value unpackElement(jsi::Runtime& runtime, mpack_reader_t* reader, int depth) { if (depth >= 32) { // critical check! mpack_reader_flag_error(reader, mpack_error_too_big); throw jsi::JSError(runtime, "unpackElement/ maximum depth reached"); @@ -142,10 +125,10 @@ jsi::Value Packer::unpackElement(jsi::Runtime& runtime, mpack_reader_t* reader, size_t count = mpack_tag_array_count(&tag); jsi::Array array = jsi::Array(runtime, count); for(size_t i = 0; i< count; i++) { + array.setValueAtIndex(runtime, i, unpackElement(runtime, reader, depth + 1)); if (mpack_reader_error(reader) != mpack_ok) { throw jsi::JSError(runtime, "unpackElement/ failed to read element in array"); } - array.setValueAtIndex(runtime, i, unpackElement(runtime, reader, depth + 1)); } mpack_done_array(reader); return array; @@ -178,3 +161,18 @@ jsi::Value Packer::unpackElement(jsi::Runtime& runtime, mpack_reader_t* reader, } } +jsi::String unpackString(jsi::Runtime& runtime, mpack_reader_t* reader, size_t strLength) { + if (mpack_should_read_bytes_inplace(reader, strLength)) { + const char* data = mpack_read_bytes_inplace(reader, strLength); + + if (mpack_reader_error(reader) != mpack_ok) + throw jsi::JSError(runtime, "unpackKey/ failed to read in-place"); + + mpack_done_str(reader); + return jsi::String::createFromUtf8(runtime,(uint8_t *) data, strLength); + } else { + // We don't stream data to the reader so we should always be able to read bytes in place. + throw jsi::JSError(runtime, "unpackString/ unable to read bytes in place"); + } +} +} diff --git a/cpp/packer.h b/cpp/packer.h index b671637..7eff72f 100644 --- a/cpp/packer.h +++ b/cpp/packer.h @@ -6,10 +6,9 @@ #import "mpack.h" -using namespace::facebook; -class Packer { -public: +using namespace facebook; +namespace Packer { jsi::Value unpackElement(jsi::Runtime& runtime, mpack_reader_t* reader, int depth); void pack(const jsi::Value& value, jsi::Runtime& runtime, mpack_writer_t* writer); -}; +} #endif /* packer_h */ diff --git a/cpp/react-native-leveldb.cpp b/cpp/react-native-leveldb.cpp index 121c759..2c903ac 100644 --- a/cpp/react-native-leveldb.cpp +++ b/cpp/react-native-leveldb.cpp @@ -213,14 +213,13 @@ void installLeveldb(jsi::Runtime& jsiRuntime, std::string documentDir) { throw jsi::JSError(runtime, "leveldbPut/invalid-params"); } - Packer packer; mpack_writer_t writer; size_t size; char* growable_buf; try { mpack_writer_init_growable(&writer, &growable_buf, &size); - packer.pack(arguments[2], runtime, &writer); + Packer::pack(arguments[2], runtime, &writer); if (mpack_writer_destroy(&writer) != mpack_ok) { throw jsi::JSError(runtime, "leveldbPut/ an error occured encoding the data"); @@ -264,7 +263,6 @@ void installLeveldb(jsi::Runtime& jsiRuntime, std::string documentDir) { auto names = record.getPropertyNames(runtime); auto length = names.length(runtime); - Packer packer; mpack_writer_t writer; size_t size; char* growable_buf; @@ -273,7 +271,7 @@ void installLeveldb(jsi::Runtime& jsiRuntime, std::string documentDir) { auto key = names.getValueAtIndex(runtime, i).asString(runtime); mpack_writer_init_growable(&writer, &growable_buf, &size); - packer.pack(record.getProperty(runtime, key), runtime, &writer); + Packer::pack(record.getProperty(runtime, key), runtime, &writer); if (mpack_writer_destroy(&writer) != mpack_ok) { throw jsi::JSError(runtime, "leveldbBatchObjects/ an error occurred encoding the data"); @@ -530,12 +528,11 @@ void installLeveldb(jsi::Runtime& jsiRuntime, std::string documentDir) { } mpack_reader_t reader; - Packer packer; jsi::Value parsed; try { mpack_reader_init_data(&reader, value.data(), value.size()); - parsed = packer.unpackElement(runtime, &reader, 0); + parsed = Packer::unpackElement(runtime, &reader, 0); } catch(...) { mpack_reader_destroy(&reader); throw; @@ -565,7 +562,6 @@ void installLeveldb(jsi::Runtime& jsiRuntime, std::string documentDir) { leveldb::Iterator* it = db->NewIterator(leveldb::ReadOptions()); mpack_reader_t reader; - Packer packer; try { for (it->SeekToFirst(); it->Valid(); it->Next()) { @@ -573,7 +569,7 @@ void installLeveldb(jsi::Runtime& jsiRuntime, std::string documentDir) { auto value = it->value(); mpack_reader_init_data(&reader, value.data(), value.size()); - auto parsed = packer.unpackElement(runtime, &reader, 0); + auto parsed = Packer::unpackElement(runtime, &reader, 0); if (mpack_ok != mpack_reader_destroy(&reader)) { throw jsi::JSError(runtime, "leveldbGetAllObjects/ failed to read data");