Skip to content

Commit

Permalink
url: const-ify APIs, and pass URL by ref
Browse files Browse the repository at this point in the history
Fixes warnings by Coverity Scan of inefficiences when passing by value
instead of passing by const reference.

PR-URL: nodejs/node#15615
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
  • Loading branch information
sam-github authored and addaleax committed Oct 4, 2017
1 parent b9cfd88 commit f6f7330
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
14 changes: 7 additions & 7 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ struct file_check {
bool failed = true;
uv_file file = -1;
};
inline const struct file_check check_file(URL search,
inline const struct file_check check_file(const URL& search,
bool close = false,
bool allow_dir = false) {
struct file_check ret;
Expand All @@ -349,7 +349,7 @@ inline const struct file_check check_file(URL search,
if (close) uv_fs_close(nullptr, &fs_req, fd, nullptr);
return ret;
}
URL resolve_extensions(URL search, bool check_exact = true) {
URL resolve_extensions(const URL& search, bool check_exact = true) {
if (check_exact) {
auto check = check_file(search, true);
if (!check.failed) {
Expand All @@ -365,10 +365,10 @@ URL resolve_extensions(URL search, bool check_exact = true) {
}
return URL("");
}
inline URL resolve_index(URL search) {
inline URL resolve_index(const URL& search) {
return resolve_extensions(URL("index", &search), false);
}
URL resolve_main(URL search) {
URL resolve_main(const URL& search) {
URL pkg("package.json", &search);
auto check = check_file(pkg);
if (!check.failed) {
Expand Down Expand Up @@ -402,7 +402,7 @@ URL resolve_main(URL search) {
}
return URL("");
}
URL resolve_module(std::string specifier, URL* base) {
URL resolve_module(std::string specifier, const URL* base) {
URL parent(".", base);
URL dir("");
do {
Expand All @@ -427,7 +427,7 @@ URL resolve_module(std::string specifier, URL* base) {
return URL("");
}

URL resolve_directory(URL search, bool read_pkg_json) {
URL resolve_directory(const URL& search, bool read_pkg_json) {
if (read_pkg_json) {
auto main = resolve_main(search);
if (!(main.flags() & URL_FLAGS_FAILED)) return main;
Expand All @@ -438,7 +438,7 @@ URL resolve_directory(URL search, bool read_pkg_json) {
} // anonymous namespace


URL Resolve(std::string specifier, URL* base, bool read_pkg_json) {
URL Resolve(std::string specifier, const URL* base, bool read_pkg_json) {
URL pure_url(specifier);
if (!(pure_url.flags() & URL_FLAGS_FAILED)) {
return pure_url;
Expand Down
2 changes: 1 addition & 1 deletion src/module_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace node {
namespace loader {

node::url::URL Resolve(std::string specifier, node::url::URL* base,
node::url::URL Resolve(std::string specifier, const node::url::URL* base,
bool read_pkg_json = false);

class ModuleWrap : public BaseObject {
Expand Down
4 changes: 2 additions & 2 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2081,7 +2081,7 @@ static void DomainToUnicode(const FunctionCallbackInfo<Value>& args) {
v8::NewStringType::kNormal).ToLocalChecked());
}

std::string URL::ToFilePath() {
std::string URL::ToFilePath() const {
if (context_.scheme != "file:") {
return "";
}
Expand All @@ -2102,7 +2102,7 @@ std::string URL::ToFilePath() {
}
#endif
std::string decoded_path;
for (std::string& part : context_.path) {
for (const std::string& part : context_.path) {
std::string decoded;
PercentDecode(part.c_str(), part.length(), &decoded);
for (char& ch : decoded) {
Expand Down
4 changes: 2 additions & 2 deletions src/node_url.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class URL {
return context_.fragment;
}

std::string path() {
std::string path() const {
std::string ret;
for (auto i = context_.path.begin(); i != context_.path.end(); i++) {
ret += '/';
Expand All @@ -165,7 +165,7 @@ class URL {

// Get the path of the file: URL in a format consumable by native file system
// APIs. Returns an empty string if something went wrong.
std::string ToFilePath();
std::string ToFilePath() const;

const Local<Value> ToObject(Environment* env) const;

Expand Down

0 comments on commit f6f7330

Please sign in to comment.