Skip to content

Commit

Permalink
refactor(filesystem): refactor file_utils unit tests (apache#1634)
Browse files Browse the repository at this point in the history 
apache#887

- Remove the useless return value of `get_normalized_path()`
- Simplify the code of file_utils.cpp
  • Loading branch information
@acelyc111
acelyc111 authored Oct 12, 2023
1 parent 6064d70 commit ed3d31e
Show file tree
Hide file tree
Showing 3 changed files with 320 additions and 916 deletions.
102 changes: 28 additions & 74 deletions src/utils/filesystem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ static inline int get_stat_internal(const std::string &npath, struct stat_ &st)
return err;
}

// TODO(yingchun): remove the return value because it's always 0.
int get_normalized_path(const std::string &path, std::string &npath)
void get_normalized_path(const std::string &path, std::string &npath)
{
char sep;
size_t i;
Expand All @@ -105,7 +104,7 @@ int get_normalized_path(const std::string &path, std::string &npath)

if (path.empty()) {
npath = "";
return 0;
return;
}

len = path.length();
Expand All @@ -131,8 +130,6 @@ int get_normalized_path(const std::string &path, std::string &npath)

CHECK_NE_MSG(tls_path_buffer[0], _FS_NULL, "Normalized path cannot be empty!");
npath = tls_path_buffer;

return 0;
}

static __thread struct
Expand Down Expand Up @@ -210,44 +207,31 @@ bool path_exists(const std::string &path)
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
get_normalized_path(path, npath);

return dsn::utils::filesystem::path_exists_internal(npath, FTW_NS);
}

bool directory_exists(const std::string &path)
{
std::string npath;
int err;

if (path.empty()) {
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
std::string npath;
get_normalized_path(path, npath);

return dsn::utils::filesystem::path_exists_internal(npath, FTW_D);
}

bool file_exists(const std::string &path)
{
std::string npath;
int err;

if (path.empty()) {
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
std::string npath;
get_normalized_path(path, npath);

return dsn::utils::filesystem::path_exists_internal(npath, FTW_F);
}
Expand All @@ -257,23 +241,18 @@ static bool get_subpaths(const std::string &path,
bool recursive,
int typeflags)
{
std::string npath;
bool ret;
int err;

if (path.empty()) {
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
std::string npath;
get_normalized_path(path, npath);

if (!dsn::utils::filesystem::path_exists_internal(npath, FTW_D)) {
return false;
}

bool ret;
switch (typeflags) {
case FTW_F:
ret = dsn::utils::filesystem::file_tree_walk(
Expand Down Expand Up @@ -351,17 +330,12 @@ static bool remove_directory(const std::string &npath)

bool remove_path(const std::string &path)
{
std::string npath;
int err;

if (path.empty()) {
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
std::string npath;
get_normalized_path(path, npath);

if (dsn::utils::filesystem::path_exists_internal(npath, FTW_F)) {
bool ret = (::remove(npath.c_str()) == 0);
Expand Down Expand Up @@ -391,20 +365,15 @@ bool rename_path(const std::string &path1, const std::string &path2)

bool deprecated_file_size(const std::string &path, int64_t &sz)
{
struct stat_ st;
std::string npath;
int err;

if (path.empty()) {
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
std::string npath;
get_normalized_path(path, npath);

err = dsn::utils::filesystem::get_stat_internal(npath, st);
struct stat_ st;
int err = dsn::utils::filesystem::get_stat_internal(npath, st);
if (err != 0) {
return false;
}
Expand Down Expand Up @@ -458,18 +427,14 @@ bool create_directory(const std::string &path)
std::string npath;
std::string cpath;
size_t len;
int err;

if (path.empty()) {
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
get_normalized_path(path, npath);

err = dsn::utils::filesystem::create_directory_component(npath);
int err = dsn::utils::filesystem::create_directory_component(npath);
if (err == 0) {
return true;
} else if (err != ENOENT) {
Expand Down Expand Up @@ -514,10 +479,7 @@ bool create_directory(const std::string &path)
bool create_file(const std::string &path)
{
std::string npath;
int err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
get_normalized_path(path, npath);

auto pos = npath.find_last_of("\\/");
if ((pos != std::string::npos) && (pos > 0)) {
Expand Down Expand Up @@ -599,23 +561,20 @@ std::string get_file_name(const std::string &path)

std::string path_combine(const std::string &path1, const std::string &path2)
{
int err;
std::string path3;
std::string npath;

if (path1.empty()) {
err = dsn::utils::filesystem::get_normalized_path(path2, npath);
get_normalized_path(path2, npath);
} else if (path2.empty()) {
err = dsn::utils::filesystem::get_normalized_path(path1, npath);
get_normalized_path(path1, npath);
} else {
path3 = path1;
std::string path3 = path1;
path3.append(1, _FS_SLASH);
path3.append(path2);

err = dsn::utils::filesystem::get_normalized_path(path3, npath);
get_normalized_path(path3, npath);
}

return ((err == 0) ? npath : "");
return npath;
}

bool get_current_directory(std::string &path)
Expand All @@ -632,20 +591,15 @@ bool get_current_directory(std::string &path)

bool last_write_time(const std::string &path, time_t &tm)
{
struct stat_ st;
std::string npath;
int err;

if (path.empty()) {
return false;
}

err = get_normalized_path(path, npath);
if (err != 0) {
return false;
}
std::string npath;
get_normalized_path(path, npath);

err = dsn::utils::filesystem::get_stat_internal(npath, st);
struct stat_ st;
int err = dsn::utils::filesystem::get_stat_internal(npath, st);
if (err != 0) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/utils/filesystem.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ namespace filesystem {

// TODO(yingchun): Consider using rocksdb APIs to rewrite the following functions.

int get_normalized_path(const std::string &path, std::string &npath);
void get_normalized_path(const std::string &path, std::string &npath);

bool get_absolute_path(const std::string &path1, std::string &path2);

Expand Down
Loading

0 comments on commit ed3d31e

Please sign in to comment.