Skip to content

Commit

Permalink
cpp: fix potential FileSequence memory leak during copy, and use uniq…
Browse files Browse the repository at this point in the history
…ue_ptr for data memeber
  • Loading branch information
justinfx committed Apr 14, 2024
1 parent 05d8562 commit 9027b5b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Changes:
2.12.0
----------------

* cpp - fix potential FileSequence memory leak during copy; switch to unique_ptr private data member
* cpp - use shared_ptr for internal map management instead of custom deleter
* cpp - clang-tidy formatting

Expand Down
5 changes: 2 additions & 3 deletions cpp/fileseq.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ Status findSequencesOnDisk(FileSequences &seqs,
Frame frame;
size_t frameWidth;
std::string name;
FileSequence fs;
std::shared_ptr<SeqInfo> seqInfo;
SeqPatternMatch match;
SeqsMap::iterator seqFound;
Expand Down Expand Up @@ -399,7 +398,7 @@ Status findSequencesOnDisk(FileSequences &seqs,
if (singleFiles) {
buf << name;

fs = FileSequence(buf.str(), style, &status);
FileSequence fs {buf.str(), style, &status};
if (!status) {
return status;
}
Expand Down Expand Up @@ -516,7 +515,7 @@ Status findSequencesOnDisk(FileSequences &seqs,
buf.str(root); // reset
buf << name << frange << pad << ext;

fs = FileSequence(buf.str(), style, &status);
FileSequence fs {buf.str(), style, &status};
// TODO: Maybe don't bail on the entire loop
// if we can't process this particular sequence.
// Report the error by some means (logger, ...)
Expand Down
30 changes: 20 additions & 10 deletions cpp/sequence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,22 +116,32 @@ bool FileSequence::init(const std::string &path, PadStyle padStyle, Status* ok)
return true;
}

FileSequence::~FileSequence() {
if (m_seqData) {
delete m_seqData;
m_seqData = NULL;
}
}
FileSequence::~FileSequence() {} // NOLINT(*-use-equals-default)

FileSequence::FileSequence(const FileSequence& rhs) {
if (m_seqData == NULL) {
delete m_seqData;
if (rhs.m_seqData) {
m_seqData = std::unique_ptr<internal::FileSequenceData>(
new internal::FileSequenceData(*rhs.m_seqData));
} else {
m_seqData = std::unique_ptr<internal::FileSequenceData>(
new internal::FileSequenceData());
}

m_seqData = new internal::FileSequenceData(*(rhs.m_seqData));
m_frameSet = rhs.m_frameSet;
}

FileSequence &FileSequence::operator=(FileSequence rhs) {
// Swap with copied arg
swap(*this, rhs);
return *this;
}

void swap(FileSequence &first, FileSequence &second) {
using std::swap;

swap(first.m_seqData, second.m_seqData);
swap(first.m_frameSet, second.m_frameSet);
}

bool FileSequence::isValid() const {
// Sanity check
if (m_seqData == nullptr) {
Expand Down
15 changes: 3 additions & 12 deletions cpp/sequence.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,10 @@ class FileSequence {
FileSequence(const FileSequence& rhs);

// Assignment
FileSequence& operator=(FileSequence rhs) {
// Swap with copied arg
swap(*this, rhs);
return *this;
}
FileSequence& operator=(FileSequence rhs);

// Swap functionality
friend void swap(FileSequence &first, FileSequence &second) {
using std::swap;

swap(first.m_seqData, second.m_seqData);
swap(first.m_frameSet, second.m_frameSet);
}
friend void swap(FileSequence &first, FileSequence &second);

/*!
Return whether the sequence was properly parsed and
Expand Down Expand Up @@ -321,7 +312,7 @@ class FileSequence {
bool singleFiles,
PadStyle style);
private:
internal::FileSequenceData* m_seqData;
std::unique_ptr<internal::FileSequenceData> m_seqData;
FrameSet m_frameSet;

};
Expand Down

0 comments on commit 9027b5b

Please sign in to comment.