From 452f015697bda56ed8e2a4344761c2c57010333a Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Wed, 25 May 2022 11:30:07 +0900 Subject: [PATCH 1/4] src,fs: refactor duplicated code in fs.readdir `AfterScanDirWithTypes` is almost same as `AfterScanDir` except for handling the `with file types` option. This merges the two functions into one. Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/node_file.cc | 82 +++++++++++++++--------------------------------- src/node_file.h | 3 ++ 2 files changed, 28 insertions(+), 57 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index f4238e7d68cce0..5b02f53be27e19 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -768,43 +768,6 @@ void AfterStringPtr(uv_fs_t* req) { } } -void AfterScanDir(uv_fs_t* req) { - FSReqBase* req_wrap = FSReqBase::from_req(req); - FSReqAfterScope after(req_wrap, req); - - if (!after.Proceed()) { - return; - } - Environment* env = req_wrap->env(); - Local error; - int r; - std::vector> name_v; - - for (;;) { - uv_dirent_t ent; - - r = uv_fs_scandir_next(req, &ent); - if (r == UV_EOF) - break; - if (r != 0) { - return req_wrap->Reject(UVException( - env->isolate(), r, nullptr, req_wrap->syscall(), req->path)); - } - - MaybeLocal filename = - StringBytes::Encode(env->isolate(), - ent.name, - req_wrap->encoding(), - &error); - if (filename.IsEmpty()) - return req_wrap->Reject(error); - - name_v.push_back(filename.ToLocalChecked()); - } - - req_wrap->Resolve(Array::New(env->isolate(), name_v.data(), name_v.size())); -} - void AfterScanDirWithTypes(uv_fs_t* req) { FSReqBase* req_wrap = FSReqBase::from_req(req); FSReqAfterScope after(req_wrap, req); @@ -821,6 +784,8 @@ void AfterScanDirWithTypes(uv_fs_t* req) { std::vector> name_v; std::vector> type_v; + const bool with_file_types = req_wrap->with_file_types(); + for (;;) { uv_dirent_t ent; @@ -832,23 +797,23 @@ void AfterScanDirWithTypes(uv_fs_t* req) { UVException(isolate, r, nullptr, req_wrap->syscall(), req->path)); } - MaybeLocal filename = - StringBytes::Encode(isolate, - ent.name, - req_wrap->encoding(), - &error); - if (filename.IsEmpty()) + Local filename; + if (!StringBytes::Encode(isolate, ent.name, req_wrap->encoding(), &error) + .ToLocal(&filename)) { return req_wrap->Reject(error); + } + name_v.push_back(filename); - name_v.push_back(filename.ToLocalChecked()); - type_v.emplace_back(Integer::New(isolate, ent.type)); + if (with_file_types) type_v.emplace_back(Integer::New(isolate, ent.type)); } - Local result[] = { - Array::New(isolate, name_v.data(), name_v.size()), - Array::New(isolate, type_v.data(), type_v.size()) - }; - req_wrap->Resolve(Array::New(isolate, result, arraysize(result))); + if (with_file_types) { + Local result[] = {Array::New(isolate, name_v.data(), name_v.size()), + Array::New(isolate, type_v.data(), type_v.size())}; + req_wrap->Resolve(Array::New(isolate, result, arraysize(result))); + } else { + req_wrap->Resolve(Array::New(isolate, name_v.data(), name_v.size())); + } } void Access(const FunctionCallbackInfo& args) { @@ -1645,13 +1610,16 @@ static void ReadDir(const FunctionCallbackInfo& args) { FSReqBase* req_wrap_async = GetReqWrap(args, 3); if (req_wrap_async != nullptr) { // readdir(path, encoding, withTypes, req) - if (with_types) { - AsyncCall(env, req_wrap_async, args, "scandir", encoding, - AfterScanDirWithTypes, uv_fs_scandir, *path, 0 /*flags*/); - } else { - AsyncCall(env, req_wrap_async, args, "scandir", encoding, - AfterScanDir, uv_fs_scandir, *path, 0 /*flags*/); - } + req_wrap_async->set_with_file_types(with_types); + AsyncCall(env, + req_wrap_async, + args, + "scandir", + encoding, + AfterScanDirWithTypes, + uv_fs_scandir, + *path, + 0 /*flags*/); } else { // readdir(path, encoding, withTypes, undefined, ctx) CHECK_EQ(argc, 5); FSReqWrapSync req_wrap_sync; diff --git a/src/node_file.h b/src/node_file.h index cf98c5c933bb84..482446eb624436 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -89,8 +89,10 @@ class FSReqBase : public ReqWrap { enum encoding encoding() const { return encoding_; } bool use_bigint() const { return use_bigint_; } bool is_plain_open() const { return is_plain_open_; } + bool with_file_types() const { return with_file_types_; } void set_is_plain_open(bool value) { is_plain_open_ = value; } + void set_with_file_types(bool value) { with_file_types_ = value; } FSContinuationData* continuation_data() const { return continuation_data_.get(); @@ -116,6 +118,7 @@ class FSReqBase : public ReqWrap { bool has_data_ = false; bool use_bigint_ = false; bool is_plain_open_ = false; + bool with_file_types_ = false; const char* syscall_ = nullptr; BaseObjectPtr binding_data_; From dca7ebd5b7998e517855ef90f8ad72c30517182e Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Wed, 25 May 2022 22:19:33 +0900 Subject: [PATCH 2/4] fixup: rename function name Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/node_file.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index 5b02f53be27e19..e32be7a6e31cf2 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -768,7 +768,7 @@ void AfterStringPtr(uv_fs_t* req) { } } -void AfterScanDirWithTypes(uv_fs_t* req) { +void AfterScanDir(uv_fs_t* req) { FSReqBase* req_wrap = FSReqBase::from_req(req); FSReqAfterScope after(req_wrap, req); @@ -1616,7 +1616,7 @@ static void ReadDir(const FunctionCallbackInfo& args) { args, "scandir", encoding, - AfterScanDirWithTypes, + AfterScanDir, uv_fs_scandir, *path, 0 /*flags*/); From 9f7cd76d19076d249ed4fb03db5b14e8b489a09b Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Wed, 25 May 2022 22:44:00 +0900 Subject: [PATCH 3/4] check: verify linting cc Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/node_file.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_file.cc b/src/node_file.cc index e32be7a6e31cf2..5f2ebc32a3e47b 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -804,7 +804,8 @@ void AfterScanDir(uv_fs_t* req) { } name_v.push_back(filename); - if (with_file_types) type_v.emplace_back(Integer::New(isolate, ent.type)); + if (with_file_types) + type_v.emplace_back(Integer::New(isolate, ent.type)); } if (with_file_types) { From 9506649b68be445a06839bacebaa6c302cc8279c Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Wed, 25 May 2022 23:04:56 +0900 Subject: [PATCH 4/4] fixup: revert lint check Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com --- src/node_file.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index 5f2ebc32a3e47b..e32be7a6e31cf2 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -804,8 +804,7 @@ void AfterScanDir(uv_fs_t* req) { } name_v.push_back(filename); - if (with_file_types) - type_v.emplace_back(Integer::New(isolate, ent.type)); + if (with_file_types) type_v.emplace_back(Integer::New(isolate, ent.type)); } if (with_file_types) {