Skip to content

Commit

Permalink
Fix std::string in union.
Browse files Browse the repository at this point in the history
The setup before this commit violates the C++ standard.
If two classes with non-standard constructors and/or
destructors are in a union their constructors and/or
destructors have to be called explicitly when
switching between the value types.

Using a union is also unsafe as it's illegal to
access the "wrong" of the two members. But the API
doesn't have a way to tell which member is the active.

The new solution doesn't use unions but allows clients
to safely access even the wrong value.
  • Loading branch information
gostefan committed Sep 5, 2024
1 parent 6263786 commit e85f164
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 10 deletions.
21 changes: 15 additions & 6 deletions sources/include/citygml/externalreference.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,23 @@


namespace citygml {
union LIBCITYGML_EXPORT ExternalObjectReference {
class LIBCITYGML_EXPORT ExternalObjectReference {
public:
ExternalObjectReference() = default;
~ExternalObjectReference() = default;

const std::string& getName() const;
const std::string& getUri() const;
void setName(const std::string& name);
void setUri(const std::string& uri);

private:
PRAGMA_WARN_DLL_BEGIN
std::string name;
std::string uri;
std::string value;
PRAGMA_WARN_DLL_END
ExternalObjectReference();
~ExternalObjectReference();

enum class ObjectRefType { NAME, URI };
ObjectRefType type;
};

class LIBCITYGML_EXPORT ExternalReference: public Object {
Expand Down
28 changes: 26 additions & 2 deletions sources/src/citygml/externalreference.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,34 @@
#include <citygml/externalreference.h>

namespace citygml {
ExternalObjectReference::ExternalObjectReference() {
namespace {
const std::string EMPTY_STRING("");
} // anonymous namespace

const std::string& ExternalObjectReference::getName() const {
if (type == ObjectRefType::NAME) {
return value;
} else {
return EMPTY_STRING;
}
}

const std::string& ExternalObjectReference::getUri() const {
if (type == ObjectRefType::URI) {
return value;
} else {
return EMPTY_STRING;
}
}

void ExternalObjectReference::setName(const std::string& name) {
type = ObjectRefType::NAME;
value = name;
}

ExternalObjectReference::~ExternalObjectReference() {
void ExternalObjectReference::setUri(const std::string& uri) {
type = ObjectRefType::URI;
value = uri;
}

ExternalReference::ExternalReference(std::string const& id) : Object(id) {
Expand Down
4 changes: 2 additions & 2 deletions sources/src/parser/externalreferenceparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ namespace citygml {
} else if (node == NodeType::CORE_InformationSystemNode) {
model->informationSystem = characters;
} else if (node == NodeType::CORE_NameNode) {
model->externalObject.name = characters;
model->externalObject.setName(characters);
} else if (node == NodeType::CORE_UriNode) {
model->externalObject.uri = characters;
model->externalObject.setUri(characters);
}

return GMLObjectElementParser::parseChildElementEndTag(node, characters);
Expand Down

0 comments on commit e85f164

Please sign in to comment.