use string instead of const char* gcs
diff --git a/tensorflow/c/experimental/filesystem/plugins/gcs/BUILD b/tensorflow/c/experimental/filesystem/plugins/gcs/BUILD
index 68d7de4..d2b77c9 100644
--- a/tensorflow/c/experimental/filesystem/plugins/gcs/BUILD
+++ b/tensorflow/c/experimental/filesystem/plugins/gcs/BUILD
@@ -30,7 +30,6 @@
"//tensorflow/c:tf_status",
"//tensorflow/c/experimental/filesystem:filesystem_interface",
"@com_github_googlecloudplatform_google_cloud_cpp//:storage_client",
- "@com_google_absl//absl/strings",
],
)
@@ -50,7 +49,6 @@
"gcs_filesystem.cc",
"gcs_filesystem_test.cc",
],
- local_defines = ["TF_GCS_FILESYSTEM_TEST"],
tags = [
"manual",
"notap",
diff --git a/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.cc b/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.cc
index bd55caf..61177bf 100644
--- a/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.cc
+++ b/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.cc
@@ -38,8 +38,8 @@
static void* plugin_memory_allocate(size_t size) { return calloc(1, size); }
static void plugin_memory_free(void* ptr) { free(ptr); }
-void ParseGCSPath(absl::string_view fname, bool object_empty_ok, char** bucket,
- char** object, TF_Status* status) {
+void ParseGCSPath(const std::string& fname, bool object_empty_ok,
+ std::string& bucket, std::string& object, TF_Status* status) {
size_t scheme_end = fname.find("://") + 2;
if (fname.substr(0, scheme_end + 1) != "gs://") {
TF_SetStatus(status, TF_INVALID_ARGUMENT,
@@ -48,33 +48,18 @@
}
size_t bucket_end = fname.find("/", scheme_end + 1);
- if (bucket_end == absl::string_view::npos) {
+ if (bucket_end == std::string::npos) {
TF_SetStatus(status, TF_INVALID_ARGUMENT,
"GCS path doesn't contain a bucket name.");
return;
}
- absl::string_view bucket_view =
- fname.substr(scheme_end + 1, bucket_end - scheme_end - 1);
- *bucket =
- static_cast<char*>(plugin_memory_allocate(bucket_view.length() + 1));
- memcpy(*bucket, bucket_view.data(), bucket_view.length());
- (*bucket)[bucket_view.length()] = '\0';
+ bucket = std::move(fname.substr(scheme_end + 1, bucket_end - scheme_end - 1));
- absl::string_view object_view = fname.substr(bucket_end + 1);
- if (object_view.empty()) {
- if (object_empty_ok) {
- *object = nullptr;
- return;
- } else {
- TF_SetStatus(status, TF_INVALID_ARGUMENT,
- "GCS path doesn't contain an object name.");
- return;
- }
+ object = std::move(fname.substr(bucket_end + 1));
+ if (object.empty() && !object_empty_ok) {
+ TF_SetStatus(status, TF_INVALID_ARGUMENT,
+ "GCS path doesn't contain an object name.");
}
- *object =
- static_cast<char*>(plugin_memory_allocate(object_view.length() + 1));
- // object_view.data() is a null-terminated string_view because fname is.
- strcpy(*object, object_view.data());
}
// SECTION 1. Implementation for `TF_RandomAccessFile`
@@ -89,8 +74,8 @@
// ----------------------------------------------------------------------------
namespace tf_writable_file {
typedef struct GCSFile {
- const char* bucket;
- const char* object;
+ const std::string bucket;
+ const std::string object;
gcs::Client* gcs_client; // not owned
TempFile outfile;
bool sync_need;
@@ -98,8 +83,6 @@
static void Cleanup(TF_WritableFile* file) {
auto gcs_file = static_cast<GCSFile*>(file->plugin_file);
- plugin_memory_free(const_cast<char*>(gcs_file->bucket));
- plugin_memory_free(const_cast<char*>(gcs_file->object));
delete gcs_file;
}
@@ -141,15 +124,14 @@
void NewWritableFile(const TF_Filesystem* filesystem, const char* path,
TF_WritableFile* file, TF_Status* status) {
- char* bucket;
- char* object;
- ParseGCSPath(path, false, &bucket, &object, status);
+ std::string bucket, object;
+ ParseGCSPath(path, false, bucket, object, status);
if (TF_GetCode(status) != TF_OK) return;
auto gcs_client = static_cast<gcs::Client*>(filesystem->plugin_filesystem);
char* temp_file_name = TF_GetTempFileName("");
file->plugin_file = new tf_writable_file::GCSFile(
- {bucket, object, gcs_client,
+ {std::move(bucket), std::move(object), gcs_client,
TempFile(temp_file_name, std::ios::binary | std::ios::out), true});
// We are responsible for freeing the pointer returned by TF_GetTempFileName
free(temp_file_name);
@@ -158,9 +140,8 @@
void NewAppendableFile(const TF_Filesystem* filesystem, const char* path,
TF_WritableFile* file, TF_Status* status) {
- char* bucket;
- char* object;
- ParseGCSPath(path, false, &bucket, &object, status);
+ std::string bucket, object;
+ ParseGCSPath(path, false, bucket, object, status);
if (TF_GetCode(status) != TF_OK) return;
auto gcs_client = static_cast<gcs::Client*>(filesystem->plugin_filesystem);
@@ -175,7 +156,7 @@
// If this file does not exist on server, we will need to sync it.
bool sync_need = (status_code == TF_NOT_FOUND);
file->plugin_file = new tf_writable_file::GCSFile(
- {bucket, object, gcs_client,
+ {std::move(bucket), std::move(object), gcs_client,
TempFile(temp_file_name, std::ios::binary | std::ios::app), sync_need});
free(temp_file_name);
TF_SetStatus(status, TF_OK, "");
diff --git a/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.h b/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.h
index cc8168e..bd95afa 100644
--- a/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.h
+++ b/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem.h
@@ -15,13 +15,12 @@
#ifndef TENSORFLOW_C_EXPERIMENTAL_FILESYSTEM_PLUGINS_GCS_GCS_FILESYSTEM_H_
#define TENSORFLOW_C_EXPERIMENTAL_FILESYSTEM_PLUGINS_GCS_GCS_FILESYSTEM_H_
-#include "absl/strings/string_view.h"
#include "google/cloud/storage/client.h"
#include "tensorflow/c/experimental/filesystem/filesystem_interface.h"
#include "tensorflow/c/tf_status.h"
-void ParseGCSPath(absl::string_view fname, bool object_empty_ok, char** bucket,
- char** object, TF_Status* status);
+void ParseGCSPath(const std::string& fname, bool object_empty_ok,
+ std::string& bucket, std::string& object, TF_Status* status);
namespace tf_gcs_filesystem {
void Init(TF_Filesystem* filesystem, TF_Status* status);
diff --git a/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem_test.cc b/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem_test.cc
index 96fef42..5e1afd9 100644
--- a/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem_test.cc
+++ b/tensorflow/c/experimental/filesystem/plugins/gcs/gcs_filesystem_test.cc
@@ -43,9 +43,27 @@
TF_Status* status_;
};
-// We have to add this test here because there must be at least one test.
-// This test will be removed in the future.
-TEST_F(GCSFilesystemTest, TestInit) { ASSERT_TF_OK(status_); }
+TEST_F(GCSFilesystemTest, ParseGCSPath) {
+ std::string bucket, object;
+ ParseGCSPath("gs://bucket/path/to/object", false, bucket, object, status_);
+ ASSERT_TF_OK(status_);
+ ASSERT_EQ(bucket, "bucket");
+ ASSERT_EQ(object, "path/to/object");
+
+ ParseGCSPath("gs://bucket/", true, bucket, object, status_);
+ ASSERT_TF_OK(status_);
+ ASSERT_EQ(bucket, "bucket");
+
+ ParseGCSPath("bucket/path/to/object", false, bucket, object, status_);
+ ASSERT_EQ(TF_GetCode(status_), TF_INVALID_ARGUMENT);
+
+ // bucket name must end with "/"
+ ParseGCSPath("gs://bucket", true, bucket, object, status_);
+ ASSERT_EQ(TF_GetCode(status_), TF_INVALID_ARGUMENT);
+
+ ParseGCSPath("gs://bucket/", false, bucket, object, status_);
+ ASSERT_EQ(TF_GetCode(status_), TF_INVALID_ARGUMENT);
+}
} // namespace
} // namespace tensorflow
