Skip to content

C++ interfaces: Write directly to std::string #6425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abarton5 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

C++ interfaces: Write directly to std::string #6425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 27 additions & 25 deletions c++/src/H5Attribute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,17 +317,14 @@ Attribute::getName() const
// (unfortunate in/out type sign mismatch)
size_t actual_name_size = static_cast<size_t>(name_size) + 1;

// Create buffer for C string
char *name_C = new char[actual_name_size]();
// Create space for C string
attr_name.resize(actual_name_size);

// Use overloaded function
name_size = getName(name_C, actual_name_size);

// Convert the C attribute name to return
attr_name = name_C;
// Use overloaded function, write directly into internal buffer
name_size = getName(&attr_name[0], actual_name_size);

// Clean up resource
delete[] name_C;
// Resize back to the actual length (excluding NUL)
attr_name.resize(name_size);
}

// Return attribute's name
Expand Down Expand Up @@ -385,17 +382,19 @@ Attribute::getName(H5std_string &attr_name, size_t len) const
}
// If length is provided, get that number of characters in name
else {
// Create buffer for C string
char *name_C = new char[len + 1]();
// Resize string to accommodate the requested length and terminal ASCII NUL
attr_name.resize(len + 1);

// Use overloaded function
name_size = getName(name_C, len + 1);

// Convert the C attribute name to return
attr_name = name_C;

// Clean up resource
delete[] name_C;
name_size = getName(&attr_name[0], len + 1);

// Truncate at the first null character to handle truncation/short buffers correctly
size_t null_pos = attr_name.find('\0');
if (null_pos != H5std_string::npos) {
attr_name.resize(null_pos);
} else {
attr_name.resize(name_size);
}
}
// Otherwise, keep attr_name intact

Expand Down Expand Up @@ -498,16 +497,19 @@ Attribute::p_read_fixed_len(const DataType &mem_type, H5std_string &strg) const

// If there is data, allocate buffer and read it.
if (attr_size > 0) {
char *strg_C = new char[attr_size + 1];
herr_t ret_value = H5Aread(id, mem_type.getId(), strg_C);
// Resize the string to the attribute's size and read directly into it
strg.resize(attr_size+1);
herr_t ret_value = H5Aread(id, mem_type.getId(), &strg[0]);
if (ret_value < 0) {
delete[] strg_C; // de-allocate for fixed-len string
throw AttributeIException("Attribute::read", "H5Aread failed");
}
// Get string from the C char* and release resource allocated locally
strg_C[attr_size] = '\0';
strg = strg_C;
delete[] strg_C;
// Replicate original C-string truncation (stops at first NUL character)
size_t null_pos = strg.find('\0');
if (null_pos != H5std_string::npos) {
strg.resize(null_pos);
}else{
strg.resize(attr_size);
}
}

@bmribler bmribler Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed changes are technically valid but trade code simplicity for marginal (likely zero) performance gain.

Allocation: This does not actually eliminate heap allocation; it simply moves it from an explicit new[] to the implicit heap allocation triggered by strg.resize().
Complexity & Risk: Bypassing standard string assignment forces us to manually reinvent C-string truncation logic (strg.find('\0')), increasing code complexity and correctness risk.

My recommendation is to leave the original code alone, or modernize it safely using a standard RAII std::vector buffer to cleanly eliminate the manual delete[] calls without adding truncation complexity.

}

Expand Down
14 changes: 4 additions & 10 deletions c++/src/H5DataSet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,19 +718,13 @@ DataSet::p_read_fixed_len(const hid_t mem_type_id, const hid_t mem_space_id, con

// If there is data, allocate buffer and read it.
if (data_size > 0) {
// Create buffer for C string
char *strg_C = new char[data_size + 1]();

herr_t ret_value = H5Dread(id, mem_type_id, mem_space_id, file_space_id, xfer_plist_id, strg_C);

// Resize the string to the dataset's size and read directly into it
strg.resize(data_size+1);
herr_t ret_value = H5Dread(id, mem_type_id, mem_space_id, file_space_id, xfer_plist_id, &strg[0]);
if (ret_value < 0) {
delete[] strg_C; // de-allocate for fixed-len string
throw DataSetIException("DataSet::read", "H5Dread failed for fixed length string");
}

// Get string from the C char* and release resource allocated locally
strg = H5std_string(strg_C, data_size);
delete[] strg_C;
strg.resize(data_size);
}
}

Expand Down
17 changes: 9 additions & 8 deletions c++/src/H5EnumType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,19 +205,20 @@ EnumType::insert(const H5std_string &name, void *value) const
H5std_string
EnumType::nameOf(void *value, size_t size) const
{
char *name_C = new char[size + 1](); // temporary C-string for C API

// Calls C routine H5Tenum_nameof to get the name of the specified enum type
herr_t ret_value = H5Tenum_nameof(id, value, name_C, size);
H5std_string name;
name.resize(size+1);

// Calls C routine H5Tenum_nameof to get the name of the specified enum type directly into the string buffer
herr_t ret_value = H5Tenum_nameof(id, value, &name[0], size + 1);
// If H5Tenum_nameof returns a negative value, raise an exception,
if (ret_value < 0) {
delete[] name_C;
throw DataTypeIException("EnumType::nameOf", "H5Tenum_nameof failed");
}
// otherwise, create the string to hold the datatype name and return it
H5std_string name(name_C);
delete[] name_C;
// Truncate at the first null character to account for the C API's null-termination
size_t null_pos = name.find('\0');
if (null_pos != H5std_string::npos) {
name.resize(null_pos);
}
return (name);
}

Expand Down
58 changes: 20 additions & 38 deletions c++/src/H5Location.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,21 +300,13 @@ H5Location::getComment(const char *name, size_t buf_size) const
if (tmp_len == 0)
tmp_len = static_cast<size_t>(comment_len);

// Temporary buffer for char* comment
char *comment_C = new char[tmp_len + 1]();

// Used overloaded function
ssize_t temp_len = getComment(name, tmp_len + 1, comment_C);
comment.resize(tmp_len + 1);
ssize_t temp_len = getComment(name, tmp_len + 1, &comment[0]);
if (temp_len < 0) {
delete[] comment_C;
throw LocationException("H5Location::getComment", "H5Oget_comment_by_name failed");
}

// Convert the C comment to return
comment = comment_C;

// Clean up resource
delete[] comment_C;
comment.resize(temp_len);
}

// Return the string comment
Expand Down Expand Up @@ -1653,17 +1645,17 @@ H5Location::getLinkval(const char *name, size_t size) const

// if link has value, retrieve the value, otherwise, return null string
if (val_size > 0) {
// Create buffer for C string
value_C = new char[val_size + 1]();

ret_value = H5Lget_val(getId(), name, value_C, val_size, H5P_DEFAULT);
value.resize(val_size + 1);
ret_value = H5Lget_val(getId(), name, &value[0], val_size, H5P_DEFAULT);
if (ret_value < 0) {
delete[] value_C;
throwException("getLinkval", "H5Lget_val failed");
}

value = H5std_string(value_C);
delete[] value_C;
// Truncate at the first null character to get the size correct
size_t null_pos = value.find('\0');
if (null_pos != H5std_string::npos) {
value.resize(null_pos);
}
}
return (value);
}
Expand Down Expand Up @@ -1823,20 +1815,16 @@ H5Location::getObjnameByIdx(hsize_t idx) const
// (unfortunate in/out type sign mismatch)
size_t actual_name_len = static_cast<size_t>(name_len) + 1;

// Create buffer for C string
char *name_C = new char[actual_name_len]();

name_len = H5Lget_name_by_idx(getId(), ".", H5_INDEX_NAME, H5_ITER_INC, idx, name_C, actual_name_len,
H5P_DEFAULT);
H5std_string name;
name.resize(actual_name_len);
name_len = H5Lget_name_by_idx(getId(), ".", H5_INDEX_NAME, H5_ITER_INC, idx, &name[0], actual_name_len,
H5P_DEFAULT);

if (name_len < 0) {
delete[] name_C;
throwException("getObjnameByIdx", "H5Lget_name_by_idx failed");
}

// clean up and return the string
H5std_string name = H5std_string(name_C);
delete[] name_C;
// Correct size
name.resize(name_len);
return (name);
}

Expand Down Expand Up @@ -1876,19 +1864,13 @@ H5Location::getObjnameByIdx(hsize_t idx, char *name, size_t size) const
ssize_t
H5Location::getObjnameByIdx(hsize_t idx, H5std_string &name, size_t size) const
{
// Create buffer for C string
char *name_C = new char[size + 1]();

// call overloaded function to get the name
ssize_t name_len = getObjnameByIdx(idx, name_C, size + 1);
name.resize(size + 1);
ssize_t name_len = getObjnameByIdx(idx, &name[0], size + 1);
if (name_len < 0) {
delete[] name_C;
throwException("getObjnameByIdx", "H5Lget_name_by_idx failed");
}

// clean up and return the string
name = H5std_string(name_C);
delete[] name_C;
// Correct to final size
name.resize(name_len);
return (name_len);
}

Expand Down
34 changes: 14 additions & 20 deletions c++/src/H5Object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,17 +483,14 @@ H5Object::getObjName() const
// (unfortunate in/out type sign mismatch)
size_t actual_name_size = static_cast<size_t>(name_size) + 1;

// Create buffer for C string
char *name_C = new char[actual_name_size]();

// Use overloaded function
name_size = getObjName(name_C, actual_name_size);

// Convert the C object name to return
obj_name = name_C;

// Clean up resource
delete[] name_C;
// Resize string to accommodate the name and terminal ASCII NUL
obj_name.resize(actual_name_size);

// Write directly into the string's contiguous buffer
name_size = getObjName(&obj_name[0], actual_name_size);

// Resize back to the actual length (excluding NUL)
obj_name.resize(name_size);
}

// Return object's name
Expand Down Expand Up @@ -524,17 +521,14 @@ H5Object::getObjName(H5std_string &obj_name, size_t len) const
}
// If length is provided, get that number of characters in name
else {
// Create buffer for C string
char *name_C = new char[len + 1]();

// Use overloaded function
name_size = getObjName(name_C, len + 1);
// Resize string to accommodate the requested length and terminal ASCII NUL
obj_name.resize(len + 1);

// Convert the C object name to return
obj_name = name_C;
// Write directly into the string's contiguous buffer
name_size = getObjName(&obj_name[0], len + 1);

// Clean up resource
delete[] name_C;
// Resize back to the actual length
obj_name.resize(name_size);
}
// Otherwise, keep obj_name intact

Expand Down
17 changes: 9 additions & 8 deletions c++/src/H5PropList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,20 +434,21 @@ PropList::getProperty(const char *name) const
// Get property size first
size_t size = getPropSize(name);

// Allocate buffer then get the property
char *prop_strg_C = new char[size + 1]();
H5std_string prop_strg;
prop_strg.resize(size+1);

herr_t ret_value = H5Pget(id, name, prop_strg_C); // call C API
// Get the property directly into the string's buffer
herr_t ret_value = H5Pget(id, name, &prop_strg[0]); // call C API\

// Throw exception if H5Pget returns failure
if (ret_value < 0) {
delete[] prop_strg_C;
throw PropListIException(inMemFunc("getProperty"), "H5Pget failed");
}

// Return property value as a string after deleting temp C-string
H5std_string prop_strg(prop_strg_C);
delete[] prop_strg_C;
// Truncate at the first null character to account for the C API's null-termination
size_t null_pos = prop_strg.find('\0');
if (null_pos != H5std_string::npos) {
prop_strg.resize(null_pos);
}
return (prop_strg);
}
//--------------------------------------------------------------------------
Expand Down