Skip to content

Commit

Permalink
add some nullptr checks
Browse files Browse the repository at this point in the history
  • Loading branch information
ydaveluy committed Jan 2, 2024
1 parent c8988a1 commit 7f7e571
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 42 deletions.
4 changes: 4 additions & 0 deletions include/Xsmp/Collection.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class AbstractCollection: public ::Smp::ICollection<T> {
using iterator = typename ::Smp::ICollection<T>::iterator;

T* at(::Smp::String8 name) const override {
if (!name)
return nullptr;
auto it = _map.find(name);
return it == _map.cend() ? nullptr : it->second;

Expand Down Expand Up @@ -110,6 +112,8 @@ class ContainingCollection final : public ::Xsmp::Object,
using iterator = typename ::Smp::ICollection<T>::iterator;

T* at(::Smp::String8 name) const override {
if (!name)
return nullptr;
auto it = _map.find(name);
return it == _map.cend() ? nullptr : it->second.get();

Expand Down
4 changes: 2 additions & 2 deletions src/Xsmp/LibraryHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace {
/// - windows: <libraryName>.dll
std::string LibraryFileName(const char *libraryName) {
#if (defined(_WIN32) || defined(_WIN64))
return libraryName + std::string(".dll");
return std::string(libraryName) + ".dll";
#elif defined(__APPLE__)
return std::string("lib") + libraryName + ".dylib";
#else
Expand All @@ -51,7 +51,7 @@ void* LoadLibrary(const char *libraryName) {
}

void CloseLibrary(void *handle) {
#ifdef _WIN32
#if (defined(_WIN32) || defined(_WIN64))
FreeLibrary(static_cast<HMODULE>(handle));
#else
dlclose(handle);
Expand Down
12 changes: 5 additions & 7 deletions src/Xsmp/Object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,17 @@ ::Smp::String8 checkName(::Smp::String8 name, ::Smp::IObject const *parent) {
if (!name)
::Xsmp::Exception::throwInvalidObjectName(parent, "<nullptr>");

// the name must start with a letter and Not be an ISO/ANSI C++ keyword.

// the name must start with a letter
if (!std::isalpha(static_cast<unsigned char>(name[0])))
::Xsmp::Exception::throwInvalidObjectName(parent, name);

std::size_t i = 1;
// skip following letters, digits and "_"
while (std::isalnum(static_cast<unsigned char>(name[i])) || (name[i] == '_')) {
++i;
}

if (name[i] == '\0')
return name;

// parse an array
// parse an optional array
while (name[i] == '[') {
++i;
// index must start with a digit
Expand All @@ -58,6 +55,7 @@ ::Smp::String8 checkName(::Smp::String8 name, ::Smp::IObject const *parent) {
::Xsmp::Exception::throwInvalidObjectName(parent, name);
}

// check end of name
if (name[i] != '\0')
::Xsmp::Exception::throwInvalidObjectName(parent, name);

Expand Down Expand Up @@ -85,7 +83,7 @@ ::Smp::IObject* Object::GetParent() const {
}

void Object::SetDescription(::Smp::String8 description) noexcept {
_description = description;
_description = description ? description : "";
}

} // namespace Xsmp
8 changes: 4 additions & 4 deletions src/Xsmp/Publication/Publication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,10 @@ void Publication::PublishProperty(::Smp::String8 name,
}

::Smp::IField* Publication::GetField(::Smp::String8 fullName) const {

if (auto *field = dynamic_cast<::Smp::IField*>(::Xsmp::Helper::Resolve(
GetFields(), fullName)))
return field;
if (fullName)
if (auto *field = dynamic_cast<::Smp::IField*>(::Xsmp::Helper::Resolve(
GetFields(), fullName)))
return field;

::Xsmp::Exception::throwInvalidFieldName(_parent, fullName);
}
Expand Down
6 changes: 3 additions & 3 deletions src/Xsmp/StorageReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace {
std::ifstream createInputStream(::Smp::String8 path, ::Smp::String8 filename,
const ::Smp::IObject *object) {

auto fullPath = fs::path(path) / filename;
auto fullPath = fs::path(path ? path : "") / (filename ? filename : "");
std::ifstream is { fullPath, std::ios::binary };

if (!is.good())
Expand All @@ -41,8 +41,8 @@ std::ifstream createInputStream(::Smp::String8 path, ::Smp::String8 filename,

StorageReader::StorageReader(::Smp::String8 path, ::Smp::String8 filename,
const ::Smp::IObject *object) :
_path(path), _filename(filename), _object { object }, _is(
createInputStream(path, filename, object)) {
_path(path ? path : ""), _filename(filename ? filename : ""), _object {
object }, _is(createInputStream(path, filename, object)) {

}

Expand Down
10 changes: 5 additions & 5 deletions src/Xsmp/StorageWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ namespace Xsmp {
namespace {
std::ofstream createOutputStream(::Smp::String8 path, ::Smp::String8 filename,
const ::Smp::IObject *object) {
if (!fs::is_directory(path))
fs::create_directories(path);
if (!fs::is_directory(path ? path : ""))
fs::create_directories(path ? path : "");

auto fullPath = fs::path(path) / filename;
auto fullPath = fs::path(path ? path : "") / (filename ? filename : "");
std::ofstream os { fullPath, std::ios::binary };

if (!os.good())
Expand All @@ -44,8 +44,8 @@ std::ofstream createOutputStream(::Smp::String8 path, ::Smp::String8 filename,

StorageWriter::StorageWriter(::Smp::String8 path, ::Smp::String8 filename,
const ::Smp::IObject *object) :
_path(path), _filename(filename), _object { object }, _os {
createOutputStream(path, filename, object) } {
_path(path ? path : ""), _filename(filename ? filename : ""), _object {
object }, _os { createOutputStream(path, filename, object) } {

}

Expand Down
41 changes: 20 additions & 21 deletions src/python/ecss_smp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@
#define STRINGIFY(x) #x
#define MACRO_STRINGIFY(x) STRINGIFY(x)

namespace PYBIND11_NAMESPACE {

struct TypeHierarchy {

template<typename T>
Expand Down Expand Up @@ -269,21 +267,21 @@ static inline const TypeHierarchy IObjectHierarchy =

});

class IObjectClass: public detail::generic_type {
class IObjectClass: public py::detail::generic_type {

public:
using type = ::Smp::IObject;
using holder_type = std::unique_ptr<type>;
explicit IObjectClass(const type *obj) {

detail::type_record record;
auto type_name = detail::clean_type_id(typeid(*obj).name());
py::detail::type_record record;
auto type_name = py::detail::clean_type_id(typeid(*obj).name());
record.name = type_name.c_str(); // TODO add random ID ?
record.type = &typeid(*obj);
record.type_size = sizeof(*obj);
record.type_align = alignof(type);
// Store the dynamic class in the root module ecss_smp
record.scope = module_::import("ecss_smp");
record.scope = py::module_::import("ecss_smp");

record.holder_size = sizeof(holder_type);
record.init_instance = init_instance;
Expand All @@ -296,12 +294,12 @@ class IObjectClass: public detail::generic_type {

processHierarchy(IObjectHierarchy, obj, record);

detail::generic_type::initialize(record);
py::detail::generic_type::initialize(record);

}

static bool processHierarchy(const TypeHierarchy &hierarchy,
const type *object, detail::type_record &rec) {
const type *object, py::detail::type_record &rec) {
bool ignore = false;

for (const auto &elem : hierarchy.childs) {
Expand All @@ -318,9 +316,10 @@ class IObjectClass: public detail::generic_type {
/// instance. Should be called as soon as the `type` value_ptr is set for an instance. Takes
/// an optional pointer to an existing holder to use; if not specified and the instance is
/// `.owned`, a new holder will be constructed to manage the value pointer.
static void init_instance(detail::instance *inst, const void *holder_ptr) {
auto v_h = detail::value_and_holder(inst,
detail::get_type_info(typeid(type)), 0, 0);
static void init_instance(py::detail::instance *inst,
const void *holder_ptr) {
auto v_h = py::detail::value_and_holder(inst,
py::detail::get_type_info(typeid(type)), 0, 0);
if (!v_h.instance_registered()) {
register_instance(inst, v_h.value_ptr(), v_h.type);
v_h.set_instance_registered();
Expand All @@ -330,28 +329,29 @@ class IObjectClass: public detail::generic_type {
}

/// Deallocates an instance; via holder, if constructed; otherwise via operator delete.
static void dealloc(detail::value_and_holder &v_h) {
static void dealloc(py::detail::value_and_holder &v_h) {
// We could be deallocating because we are cleaning up after a Python exception.
// If so, the Python error indicator will be set. We need to clear that before
// running the destructor, in case the destructor code calls more Python.
// If we don't, the Python API will exit with an exception, and pybind11 will
// throw error_already_set from the C++ destructor which is forbidden and triggers
// std::terminate().
error_scope scope;
py::error_scope scope;
if (v_h.holder_constructed()) {
v_h.holder<holder_type>().~holder_type();
v_h.set_holder_constructed(false);
}
else {
detail::call_operator_delete(v_h.value_ptr<type>(),
py::detail::call_operator_delete(v_h.value_ptr<type>(),
v_h.type->type_size, v_h.type->type_align);
}
v_h.value_ptr() = nullptr;
}

private:

static void init_holder_from_existing(const detail::value_and_holder &v_h,
static void init_holder_from_existing(
const py::detail::value_and_holder &v_h,
const holder_type *holder_ptr,
std::false_type /*is_copy_constructible*/) {
new (std::addressof(v_h.holder<holder_type>())) holder_type(
Expand All @@ -360,36 +360,35 @@ class IObjectClass: public detail::generic_type {

/// Initialize holder object, variant 2: try to construct from existing holder object, if
/// possible
static void init_holder(const detail::instance *inst,
detail::value_and_holder &v_h, const holder_type *holder_ptr,
static void init_holder(const py::detail::instance *inst,
py::detail::value_and_holder &v_h, const holder_type *holder_ptr,
const void* /* dummy -- not enable_shared_from_this<T>) */) {
if (holder_ptr) {
init_holder_from_existing(v_h, holder_ptr,
std::is_copy_constructible<holder_type>());
v_h.set_holder_constructed();
}
else if (detail::always_construct_holder<holder_type>::value
else if (py::detail::always_construct_holder<holder_type>::value
|| inst->owned) {
new (std::addressof(v_h.holder<holder_type>())) holder_type(
v_h.value_ptr<type>());
v_h.set_holder_constructed();
}
}
};
} // namespace PYBIND11_NAMESPACE

const void* IObjectHook(const ::Smp::IObject *src,
const std::type_info *&type) {

if (src) {
type = &typeid(*src);
if (py::detail::get_local_type_info(*type) == nullptr) {
py::IObjectClass(src).doc() = "Automatic Python binding for '"
IObjectClass(src).doc() = "Automatic Python binding for '"
+ Xsmp::Helper::TypeName(src) + "'.";
}

// return a pointer to IObject (instead of dynamic_cast<const void*> )
return static_cast<const Smp::IObject*>(src);
return static_cast<const void*>(src);
}
type = nullptr;
return nullptr;
Expand Down

0 comments on commit 7f7e571

Please sign in to comment.