Skip to content
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

Jump to bottom

Removes C++ dependency on H5private.h #774

Merged
merged 2 commits into from
Jun 23, 2021
Merged

Removes C++ dependency on H5private.h #774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
972c396
Removes C++ dependency on H5private.h
derobins Jun 18, 2021
f1924a7
Committing clang-format changes
github-actions[bot] Jun 18, 2021
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
14 changes: 7 additions & 7 deletions c++/src/H5Attribute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <iostream>
#include <string>

#include "H5private.h" // for HDfree
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down Expand Up @@ -313,7 +312,7 @@ Attribute::getName() const
H5std_string attr_name; // attribute name to return

// Preliminary call to get the size of the attribute name
ssize_t name_size = H5Aget_name(id, static_cast<size_t>(0), NULL);
ssize_t name_size = H5Aget_name(id, 0, NULL);

// If H5Aget_name failed, throw exception
if (name_size < 0) {
Expand All @@ -324,8 +323,8 @@ Attribute::getName() const
}
// Attribute's name exists, retrieve it
else if (name_size > 0) {
char *name_C = new char[name_size + 1]; // temporary C-string
HDmemset(name_C, 0, name_size + 1); // clear buffer
// Create buffer for C string
char *name_C = new char[name_size + 1]();

// Use overloaded function
name_size = getName(name_C, name_size + 1);
Expand All @@ -336,6 +335,7 @@ Attribute::getName() const
// Clean up resource
delete[] name_C;
}

// Return attribute's name
return (attr_name);
}
Expand Down Expand Up @@ -393,8 +393,8 @@ Attribute::getName(H5std_string &attr_name, size_t len) const
}
// If length is provided, get that number of characters in name
else {
char *name_C = new char[len + 1]; // temporary C-string
HDmemset(name_C, 0, len + 1); // clear buffer
// Create buffer for C string
char *name_C = new char[len + 1]();

// Use overloaded function
name_size = getName(name_C, len + 1);
Expand Down Expand Up @@ -551,7 +551,7 @@ Attribute::p_read_variable_len(const DataType &mem_type, H5std_string &strg) con

// Get string from the C char* and release resource allocated by C API
strg = strg_C;
HDfree(strg_C);
free(strg_C);
Copy link
Contributor

Choose a reason for hiding this comment

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

delete?

Copy link
Member Author

Choose a reason for hiding this comment

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

Same here. You can't mix malloc/free and new/delete.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know. It was an accident, but you fixed it by changing HDfree to free, and I asked should it be delete?

Copy link
Member Author

Choose a reason for hiding this comment

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

It shouldn't be delete[]. If you expand the code, you'll see that srgc_C is an unallocated char pointer. The memory is allocated by the library and should be freed using free().

}

#ifndef DOXYGEN_SHOULD_SKIP_THIS
Expand Down
1 change: 0 additions & 1 deletion c++/src/H5CommonFG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include <string>

#include "H5private.h" // for HDstrcpy
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down
7 changes: 3 additions & 4 deletions c++/src/H5DataSet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <iostream>
#include <string>

#include "H5private.h" // for HDfree
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down Expand Up @@ -717,8 +716,8 @@ 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) {
char *strg_C = new char[data_size + 1];
HDmemset(strg_C, 0, data_size + 1); // clear buffer
// 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);

Expand Down Expand Up @@ -759,7 +758,7 @@ DataSet::p_read_variable_len(const hid_t mem_type_id, const hid_t mem_space_id,

// Get string from the C char* and release resource allocated by C API
strg = strg_C;
HDfree(strg_C);
free(strg_C);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be delete?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. We get that memory from the HDF5 C library, so it wasn't allocated with new.

Copy link
Contributor

@bmribler bmribler Jun 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I remember why I used HDfree, because of the very reason you said here. The right fix is to remove "new char[data_size + 1];" above. The library will allocate for strg_C.

Copy link
Member Author

Choose a reason for hiding this comment

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

That new char[data_size + 1] line is in a different function. In this function, strg_C is just defined as a char pointer and not initialized.

}

#ifndef DOXYGEN_SHOULD_SKIP_THIS
Expand Down
8 changes: 4 additions & 4 deletions c++/src/H5DataType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "H5DataType.h"
#include "H5AtomType.h"
#include "H5PredType.h"
#include "H5private.h"
#include "H5AbstractDs.h"
#include "H5DataSet.h"
#include "H5Attribute.h"
Expand Down Expand Up @@ -316,8 +315,9 @@ DataType::encode()

// Allocate buffer and call C function again to encode
if (buf_size > 0) {
encoded_buf = static_cast<unsigned char *>(HDcalloc(1, buf_size));
ret_value = H5Tencode(id, encoded_buf, &buf_size);
encoded_buf = new unsigned char[buf_size]();

ret_value = H5Tencode(id, encoded_buf, &buf_size);
if (ret_value < 0) {
throw DataTypeIException("DataType::encode", "H5Tencode failed");
}
Expand Down Expand Up @@ -970,7 +970,7 @@ DataType::close()

// Free and reset buffer of encoded object description if it's been used
if (encoded_buf != NULL) {
HDfree(encoded_buf);
delete[] encoded_buf;
buf_size = 0;
}
}
Expand Down
4 changes: 1 addition & 3 deletions c++/src/H5DxferProp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include <string>

#include "H5private.h" // for HDmemset
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down Expand Up @@ -324,8 +323,7 @@ DSetMemXferPropList::getDataTransform() const
// If expression exists, calls C routine again to get it
else if (exp_len > 0) {
// Temporary buffer for char* expression
char *exp_C = new char[exp_len + 1];
HDmemset(exp_C, 0, exp_len + 1); // clear buffer
char *exp_C = new char[exp_len + 1]();

// Used overloaded function
exp_len = getDataTransform(exp_C, exp_len + 1);
Expand Down
4 changes: 1 addition & 3 deletions c++/src/H5EnumType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include <string>

#include "H5private.h" // for HDmemset
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down Expand Up @@ -219,8 +218,7 @@ 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
HDmemset(name_C, 0, size + 1); // clear buffer
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);
Expand Down
4 changes: 1 addition & 3 deletions c++/src/H5IdComponent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include <string>

#include "H5private.h" // for HDmemset
#include "H5Include.h"
#include "H5Exception.h"
#include "H5Library.h"
Expand Down Expand Up @@ -399,8 +398,7 @@ IdComponent::p_get_file_name() const
}

// Call H5Fget_name again to get the actual file name
char *name_C = new char[name_size + 1]; // temporary C-string for C API
HDmemset(name_C, 0, name_size + 1); // clear buffer
char *name_C = new char[name_size + 1]();

name_size = H5Fget_name(temp_id, name_C, name_size + 1);

Expand Down
17 changes: 7 additions & 10 deletions c++/src/H5Location.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include <iostream>
using namespace std;

#include "H5private.h" // for HDmemset
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down Expand Up @@ -367,8 +366,7 @@ H5Location::getComment(const char *name, size_t buf_size) const
tmp_len = comment_len;

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

// Used overloaded function
ssize_t temp_len = getComment(name, tmp_len + 1, comment_C);
Expand Down Expand Up @@ -1835,8 +1833,8 @@ H5Location::getLinkval(const char *name, size_t size) const

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

ret_value = H5Lget_val(getId(), name, value_C, val_size, H5P_DEFAULT);
if (ret_value < 0) {
Expand Down Expand Up @@ -2046,9 +2044,8 @@ H5Location::getObjnameByIdx(hsize_t idx) const
if (name_len < 0)
throwException("getObjnameByIdx", "H5Lget_name_by_idx failed");

// now, allocate C buffer to get the name
char *name_C = new char[name_len + 1];
HDmemset(name_C, 0, name_len + 1); // clear buffer
// Create buffer for C string
char *name_C = new char[name_len + 1]();

name_len =
H5Lget_name_by_idx(getId(), ".", H5_INDEX_NAME, H5_ITER_INC, idx, name_C, name_len + 1, H5P_DEFAULT);
Expand Down Expand Up @@ -2102,8 +2099,8 @@ 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
{
char *name_C = new char[size + 1]; // temporary C-string for object name
HDmemset(name_C, 0, size + 1); // clear buffer
// 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);
Expand Down
24 changes: 14 additions & 10 deletions c++/src/H5Object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

#include <string>

#include "H5private.h" // for HDmemset
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down Expand Up @@ -45,9 +44,12 @@ namespace H5 {
extern "C" {

static herr_t
userAttrOpWrpr(H5_ATTR_UNUSED hid_t loc_id, const char *attr_name, H5_ATTR_UNUSED const H5A_info_t *ainfo,
void *op_data)
userAttrOpWrpr(hid_t loc_id, const char *attr_name, const H5A_info_t *ainfo, void *op_data)
{
// Unused
(void)loc_id;
(void)ainfo;

H5std_string s_attr_name = H5std_string(attr_name);
UserData4Aiterate *myData = reinterpret_cast<UserData4Aiterate *>(op_data);
myData->op(*myData->location, s_attr_name, myData->opData);
Expand All @@ -57,9 +59,11 @@ userAttrOpWrpr(H5_ATTR_UNUSED hid_t loc_id, const char *attr_name, H5_ATTR_UNUSE
// userVisitOpWrpr interfaces between the user's function and the
// C library function H5Ovisit3
static herr_t
userVisitOpWrpr(H5_ATTR_UNUSED hid_t obj_id, const char *attr_name, const H5O_info2_t *obj_info,
void *op_data)
userVisitOpWrpr(hid_t obj_id, const char *attr_name, const H5O_info2_t *obj_info, void *op_data)
{
// Unused
(void)obj_id;

H5std_string s_attr_name = H5std_string(attr_name);
UserData4Visit *myData = reinterpret_cast<UserData4Visit *>(op_data);
int status = myData->op(*myData->obj, s_attr_name, obj_info, myData->opData);
Expand Down Expand Up @@ -482,7 +486,7 @@ H5Object::getObjName() const
H5std_string obj_name; // object name to return

// Preliminary call to get the size of the object name
ssize_t name_size = H5Iget_name(getId(), NULL, static_cast<size_t>(0));
ssize_t name_size = H5Iget_name(getId(), NULL, 0);

// If H5Iget_name failed, throw exception
if (name_size < 0) {
Expand All @@ -493,8 +497,8 @@ H5Object::getObjName() const
}
// Object's name exists, retrieve it
else if (name_size > 0) {
char *name_C = new char[name_size + 1]; // temporary C-string
HDmemset(name_C, 0, name_size + 1); // clear buffer
// Create buffer for C string
char *name_C = new char[name_size + 1]();

// Use overloaded function
name_size = getObjName(name_C, name_size + 1);
Expand Down Expand Up @@ -534,8 +538,8 @@ H5Object::getObjName(H5std_string &obj_name, size_t len) const
}
// If length is provided, get that number of characters in name
else {
char *name_C = new char[len + 1]; // temporary C-string
HDmemset(name_C, 0, len + 1); // clear buffer
// Create buffer for C string
char *name_C = new char[len + 1]();

// Use overloaded function
name_size = getObjName(name_C, len + 1);
Expand Down
7 changes: 5 additions & 2 deletions c++/src/H5PredType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "H5DataType.h"
#include "H5AtomType.h"
#include "H5PredType.h"
#include "H5private.h"

namespace H5 {

Expand Down Expand Up @@ -90,8 +89,12 @@ PredType::operator=(const PredType &rhs)
// These dummy functions do not inherit from DataType - they'll
// throw an DataTypeIException if invoked.
void
PredType::commit(H5_ATTR_UNUSED H5Location &loc, H5_ATTR_UNUSED const char *name)
PredType::commit(H5Location &loc, const char *name)
{
// Unused
(void)loc;
(void)name;

throw DataTypeIException("PredType::commit",
"Error: Attempted to commit a predefined datatype. Invalid operation!");
}
Expand Down
4 changes: 1 addition & 3 deletions c++/src/H5PropList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

#include <string>

#include "H5private.h" // for HDmemset
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"
Expand Down Expand Up @@ -458,8 +457,7 @@ PropList::getProperty(const char *name) const
size_t size = getPropSize(name);

// Allocate buffer then get the property
char *prop_strg_C = new char[size + 1]; // temporary C-string for C API
HDmemset(prop_strg_C, 0, size + 1); // clear buffer
char *prop_strg_C = new char[size + 1]();

herr_t ret_value = H5Pget(id, name, prop_strg_C); // call C API

Expand Down
6 changes: 4 additions & 2 deletions c++/src/H5StrType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "H5StrType.h"
#include "H5DataSet.h"
#include "H5PredType.h"
#include "H5private.h"

namespace H5 {

Expand Down Expand Up @@ -105,8 +104,11 @@ StrType::StrType(const PredType &pred_type, const size_t &size) : AtomType()
// This constructor replaced the previous one.
// Programmer Binh-Minh Ribler - Nov 28, 2005
//--------------------------------------------------------------------------
StrType::StrType(H5_ATTR_UNUSED const int dummy, const size_t &size) : AtomType()
StrType::StrType(const int dummy, const size_t &size) : AtomType()
{
// Unused
(void)dummy;

// use DataType::copy to make a copy of the string predefined type
// then set its length
copy(PredType::C_S1);
Expand Down