Skip to content

Commit

Permalink
Add a dedicated class to manage lifetime of string handles.
Browse files Browse the repository at this point in the history
It is much easier to use and correctly handles null handles.
  • Loading branch information
teo-tsirpanis committed Apr 3, 2023
1 parent c0f62f6 commit 8dde4aa
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 38 deletions.
1 change: 1 addition & 0 deletions tiledb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ if (TILEDB_CPP_API)
${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/cpp_api/query_experimental.h
${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/cpp_api/schema_base.h
${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/cpp_api/stats.h
${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/cpp_api/string_handle_holder.h
${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/cpp_api/subarray.h
${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/cpp_api/subarray_experimental.h
${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/cpp_api/type.h
Expand Down
7 changes: 0 additions & 7 deletions tiledb/sm/cpp_api/deleter.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,6 @@ class Deleter {
tiledb_consolidation_plan_free(&p);
}

void operator()(tiledb_string_t* p) const {
capi_status_t result = tiledb_status(tiledb_string_free(&p));
if (result != TILEDB_OK) {
log_warn("Could not free string; Error code: " + std::to_string(result));
}
}

private:
/* ********************************* */
/* PRIVATE ATTRIBUTES */
Expand Down
9 changes: 4 additions & 5 deletions tiledb/sm/cpp_api/fragment_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "deleter.h"
#include "exception.h"
#include "object.h"
#include "string_handle_holder.h"
#include "tiledb.h"
#include "type.h"

Expand Down Expand Up @@ -94,12 +95,10 @@ class FragmentInfo {
/** Returns the name of the fragment with the given index. */
std::string fragment_name(uint32_t fid) const {
auto& ctx = ctx_.get();
tiledb_string_t* name;
impl::StringHandleHolder name;
ctx.handle_error(tiledb_fragment_info_get_fragment_name_v2(
ctx.ptr().get(), fragment_info_.get(), fid, &name));
auto name_ptr =
std::unique_ptr<tiledb_string_t, tiledb::impl::Deleter>(name);
return impl::handle_to_string(name_ptr);
ctx.ptr().get(), fragment_info_.get(), fid, name.c_ptr()));
return name.str();
}

/**
Expand Down
22 changes: 8 additions & 14 deletions tiledb/sm/cpp_api/group_experimental.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#define TILEDB_CPP_API_GROUP_EXPERIMENTAL_H

#include "context.h"
#include "string_handle_holder.h"
#include "tiledb.h"

namespace tiledb {
Expand Down Expand Up @@ -413,29 +414,22 @@ class Group {
tiledb::Object member(uint64_t index) const {
auto& ctx = ctx_.get();
tiledb_ctx_t* c_ctx = ctx.ptr().get();
tiledb_string_t* uri;
impl::StringHandleHolder uri;
tiledb_object_t type;
tiledb_string_t* name;
impl::StringHandleHolder name;
ctx.handle_error(tiledb_group_get_member_by_index_v2(
c_ctx, group_.get(), index, &uri, &type, &name));
auto uri_ptr = std::unique_ptr<tiledb_string_t, impl::Deleter>(uri);
auto name_ptr = std::unique_ptr<tiledb_string_t, impl::Deleter>(name);
std::optional<std::string> name_opt = std::nullopt;
if (name != nullptr) {
name_opt = impl::handle_to_string(name_ptr);
}
return tiledb::Object(type, impl::handle_to_string(uri_ptr), name_opt);
c_ctx, group_.get(), index, uri.c_ptr(), &type, name.c_ptr()));
return tiledb::Object(type, uri.str(), name.str_opt());
}

tiledb::Object member(std::string name) const {
auto& ctx = ctx_.get();
tiledb_ctx_t* c_ctx = ctx.ptr().get();
tiledb_string_t* uri;
impl::StringHandleHolder uri;
tiledb_object_t type;
ctx.handle_error(tiledb_group_get_member_by_name_v2(
c_ctx, group_.get(), name.c_str(), &uri, &type));
auto uri_ptr = std::unique_ptr<tiledb_string_t, impl::Deleter>(uri);
return tiledb::Object(type, impl::handle_to_string(uri_ptr), name);
c_ctx, group_.get(), name.c_str(), uri.c_ptr(), &type));
return tiledb::Object(type, uri.str(), name);
}

/**
Expand Down
114 changes: 114 additions & 0 deletions tiledb/sm/cpp_api/string_handle_holder.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* @file string_handle_holder.h
*
* @section LICENSE
*
* The MIT License
*
* @copyright Copyright (c) 2017-2023 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @section DESCRIPTION
*
* This file declares the C++ API for the TileDB StringHandleHolder object.
*/

#ifndef TILEDB_CPP_API_STRING_HANDLE_HOLDER_H
#define TILEDB_CPP_API_STRING_HANDLE_HOLDER_H

#include <cassert>
#include <optional>
#include <stdexcept>
#include "tiledb.h"

namespace tiledb::impl {

/**
* Manages the lifetime of a tiledb_string_t* handle and provides operations on
* it.
*/
class StringHandleHolder {
public:
StringHandleHolder() {
string_ = nullptr;
}
~StringHandleHolder() {
if (string_ == nullptr) {
return;
}

capi_status_t result = tiledb_status(tiledb_string_free(&string_));
if (result != TILEDB_OK) {
log_warn("Could not free string; Error code: " + std::to_string(result));
}
}

// Disable copy and move.
StringHandleHolder(const StringHandleHolder&) = delete;
StringHandleHolder operator=(const StringHandleHolder&) = delete;
StringHandleHolder(const StringHandleHolder&&) = delete;
StringHandleHolder operator=(const StringHandleHolder&&) = delete;

/**
* Returns a tiledb_string_t** pointer to be passed in native code.
* This method should be called only once.
*/
inline tiledb_string_t** c_ptr() {
// We should call this function only once when the object is uninitialized.
assert(string_ == nullptr);
return &string_;
}

/**
* Returns a C++ string with the handle's data.
*
* If the handle is null, returns a defaullt value, which is the empty string
* if not specified.
*/
inline std::string str(const std::string& default_value = "") const {
return str_opt().value_or(default_value);
}

/**
* Returns a C++ string with the handle's data, or nullopt if the handle is
* null.
*/
std::optional<std::string> str_opt() const {
if (string_ == nullptr) {
return std::nullopt;
}
const char* c;
size_t size;
capi_status_t status =
tiledb_status(tiledb_string_view(string_, &c, &size));
if (status != TILEDB_OK) {
throw std::runtime_error(
"Could not view string; Error code: " + std::to_string(status));
}
return std::string(c, size);
}

private:
tiledb_string_t* string_;
};

} // namespace tiledb::impl

#endif // TILEDB_CPP_API_STRING_HANDLE_HOLDER_H
12 changes: 0 additions & 12 deletions tiledb/sm/cpp_api/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -395,18 +395,6 @@ inline void check_config_error(tiledb_error_t* err) {
}
}

template <class Deleter>
inline std::string handle_to_string(
const std::unique_ptr<tiledb_string_t, Deleter>& str) {
const char* name_c;
size_t length;
if (tiledb_status(tiledb_string_view(str.get(), &name_c, &length)) !=
TILEDB_OK) {
throw std::runtime_error("tiledb_string_view failed.");
}
return std::string(name_c, length);
}

} // namespace impl

} // namespace tiledb
Expand Down

0 comments on commit 8dde4aa

Please sign in to comment.