Skip to content

Commit

Permalink
[cpp_api] Update Deleter to not free null pointers. (#5262)
Browse files Browse the repository at this point in the history
[SC-53365](https://app.shortcut.com/tiledb-inc/story/53365/cpp-api-invalid-tiledb-object-error-messages-cause-noise)

This PR updates the `Deleter` class do nothing when a null pointer is
passed to it. Previously it would pass the pointer to `tiledb_***_free`,
which would cause an error that gets ignored because most APIs return
`void`, and emit an error log message. This resulted in lots of [noise
in CI
logs](https://github.com/TileDB-Inc/TileDB/actions/runs/10530673714/job/29181096416?pr=5255#step:14:2728).

There are some cases in the C++ API where a null handle would be
attempted to be freed, like
[here](https://github.com/TileDB-Inc/TileDB/blob/9b4e5ea0c8b117716bc96dac71338ffc0d98a2db/tiledb/sm/cpp_api/array.h#L283)
or
[here](https://github.com/TileDB-Inc/TileDB/blob/9b4e5ea0c8b117716bc96dac71338ffc0d98a2db/tiledb/sm/cpp_api/current_domain.h#L74).
Some of them can be eliminated by refactoring the initialization of the
smart pointers, but this change is nevertheless valuable, because not
all cases can be migrated, and null smart pointers are valid either way.

---
TYPE: CPP_API
DESC: Fix error log messages when using the `Array` class in the C++
API.
  • Loading branch information
teo-tsirpanis authored Sep 3, 2024
1 parent 097f155 commit 84ff80c
Showing 1 changed file with 46 additions and 0 deletions.
46 changes: 46 additions & 0 deletions tiledb/sm/cpp_api/deleter.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,94 +74,140 @@ class Deleter {
/* ********************************* */

void operator()(tiledb_vfs_fh_t* p) const {
if (p == nullptr)
return;
tiledb_vfs_fh_free(&p);
}

void operator()(tiledb_array_t* p) const {
if (p == nullptr)
return;
tiledb_array_free(&p);
}

void operator()(tiledb_subarray_t* p) const {
if (p == nullptr)
return;
tiledb_subarray_free(&p);
}

void operator()(tiledb_query_t* p) const {
if (p == nullptr)
return;
tiledb_query_free(&p);
}

void operator()(tiledb_query_condition_t* p) const {
if (p == nullptr)
return;
tiledb_query_condition_free(&p);
}

void operator()(tiledb_array_schema_t* p) const {
if (p == nullptr)
return;
tiledb_array_schema_free(&p);
}

void operator()(tiledb_array_schema_evolution_t* p) const {
if (p == nullptr)
return;
tiledb_array_schema_evolution_free(&p);
}

void operator()(tiledb_attribute_t* p) const {
if (p == nullptr)
return;
tiledb_attribute_free(&p);
}

void operator()(tiledb_dimension_t* p) const {
if (p == nullptr)
return;
tiledb_dimension_free(&p);
}

void operator()(tiledb_dimension_label_t* p) const {
if (p == nullptr)
return;
tiledb_dimension_label_free(&p);
}

void operator()(tiledb_domain_t* p) const {
if (p == nullptr)
return;
tiledb_domain_free(&p);
}

void operator()(tiledb_current_domain_t* p) const {
if (p == nullptr)
return;
tiledb_current_domain_free(&p);
}

void operator()(tiledb_ndrectangle_t* p) const {
if (p == nullptr)
return;
tiledb_ndrectangle_free(&p);
}

void operator()(tiledb_enumeration_t* p) const {
if (p == nullptr)
return;
tiledb_enumeration_free(&p);
}

void operator()(tiledb_vfs_t* p) const {
if (p == nullptr)
return;
tiledb_vfs_free(&p);
}

void operator()(tiledb_filter_t* p) const {
if (p == nullptr)
return;
tiledb_filter_free(&p);
}

void operator()(tiledb_filter_list_t* p) const {
if (p == nullptr)
return;
tiledb_filter_list_free(&p);
}

void operator()(tiledb_fragment_info_t* p) const {
if (p == nullptr)
return;
tiledb_fragment_info_free(&p);
}

void operator()(tiledb_error_t* p) const {
if (p == nullptr)
return;
tiledb_error_free(&p);
}

void operator()(tiledb_group_t* p) const {
if (p == nullptr)
return;
tiledb_group_free(&p);
}

void operator()(tiledb_consolidation_plan_t* p) const {
if (p == nullptr)
return;
tiledb_consolidation_plan_free(&p);
}

void operator()(tiledb_query_channel_t* p) const {
if (p == nullptr)
return;
tiledb_query_channel_free(ctx_->ptr().get(), &p);
}

void operator()(tiledb_channel_operation_t* p) const {
if (p == nullptr)
return;
tiledb_aggregate_free(ctx_->ptr().get(), &p);
}

Expand Down

0 comments on commit 84ff80c

Please sign in to comment.