Skip to content

Commit

Permalink
Add virtual dtor to classes with virtual functions
Browse files Browse the repository at this point in the history
IncluderInterface has virtual functions but does not have a virtual
destructor. This class is derived from by FileIncluder w hich overrides
those functions.

Because there is an interface in use here, it is safe to assume some
container is storing IncluderInterface*. If the container instead held
FileIncluder* then the virtual functions wouldn't be needed.

This causes a problem since FileIncluder has member variables. The
destructor of FileIncluder knows to also destruct those member variables
(including deallocating their dynamically allocated memory).

But when IncluderInterface's destructor is called, it is not virtual and
will not call FileIncluder's destructor. So these member variables are
never destroyed and their dynamically allocated memory will be leaked.

In this case, FileIncluder stores a std::unordered_set<std::string>
which will be leaked.

This patch adds a virtual destructor to IncluderInterface to make sure
FileIncluder's destructor is called and this memory isn't leaked.

Use =default and don't redeclare IncluderInterface's dtor
  • Loading branch information
ProgramMax authored and dneto0 committed Jun 18, 2018
1 parent a2c044c commit 30af9f9
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 0 deletions.
2 changes: 2 additions & 0 deletions glslc/src/file_includer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ shaderc_include_result* MakeErrorIncludeResult(const char* message) {
return new shaderc_include_result{"", 0, message, strlen(message)};
}

FileIncluder::~FileIncluder() = default;

shaderc_include_result* FileIncluder::GetInclude(
const char* requested_source, shaderc_include_type include_type,
const char* requesting_source, size_t) {
Expand Down
3 changes: 3 additions & 0 deletions glslc/src/file_includer.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ class FileIncluder : public shaderc::CompileOptions::IncluderInterface {
public:
explicit FileIncluder(const shaderc_util::FileFinder* file_finder)
: file_finder_(*file_finder) {}

~FileIncluder() override;

// Resolves a requested source file of a given type from a requesting
// source into a shaderc_include_result whose contents will remain valid
// until it's released.
Expand Down

0 comments on commit 30af9f9

Please sign in to comment.