Skip to content

Commit

Permalink
Fix bare pointer in topic_tools::ShapeShifter (#1722)
Browse files Browse the repository at this point in the history
The Copy constructor and assignment operator on the ShapeShifter class
were wrong because the class was using a bare pointer for its internal
buffer.  This switches to using a std::vector, so the constructors are
automatically handled correctly.
  • Loading branch information
amiller27 authored Apr 27, 2020
1 parent ae2501f commit 40ad896
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 36 deletions.
27 changes: 10 additions & 17 deletions tools/topic_tools/include/topic_tools/shape_shifter.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,7 @@ class TOPIC_TOOLS_DECL ShapeShifter
std::string md5, datatype, msg_def, latching;
bool typed;

uint8_t *msgBuf;
uint32_t msgBufUsed;
uint32_t msgBufAlloc;

std::vector<uint8_t> msgBuf;
};

}
Expand Down Expand Up @@ -207,37 +204,33 @@ boost::shared_ptr<M> ShapeShifter::instantiate() const

boost::shared_ptr<M> p(boost::make_shared<M>());

ros::serialization::IStream s(msgBuf, msgBufUsed);
// The IStream never modifies its data, and nothing else has access to this
// object, so the const_cast here is ok
ros::serialization::IStream s(const_cast<unsigned char*>(msgBuf.data()),
msgBuf.size());
ros::serialization::deserialize(s, *p);

return p;
}

template<typename Stream>
void ShapeShifter::write(Stream& stream) const {
if (msgBufUsed > 0)
memcpy(stream.advance(msgBufUsed), msgBuf, msgBufUsed);
if (msgBuf.size() > 0)
memcpy(stream.advance(msgBuf.size()), msgBuf.data(), msgBuf.size());
}

template<typename Stream>
void ShapeShifter::read(Stream& stream)
{
stream.getLength();
stream.getData();

// stash this message in our buffer
if (stream.getLength() > msgBufAlloc)
{
delete[] msgBuf;
msgBuf = new uint8_t[stream.getLength()];
msgBufAlloc = stream.getLength();
}
msgBufUsed = stream.getLength();
memcpy(msgBuf, stream.getData(), stream.getLength());
msgBuf.resize(stream.getLength());
memcpy(msgBuf.data(), stream.getData(), stream.getLength());
}

} // namespace topic_tools


#endif

22 changes: 3 additions & 19 deletions tools/topic_tools/src/shape_shifter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,9 @@ using namespace topic_tools;

bool ShapeShifter::uses_old_API_ = false;

ShapeShifter::ShapeShifter()
: typed(false),
msgBuf(NULL),
msgBufUsed(0),
msgBufAlloc(0)
{
}


ShapeShifter::~ShapeShifter()
{
if (msgBuf)
delete[] msgBuf;

msgBuf = NULL;
msgBufAlloc = 0;
}
ShapeShifter::ShapeShifter() : typed(false) {}

ShapeShifter::~ShapeShifter() {}

std::string const& ShapeShifter::getDataType() const { return datatype; }

Expand Down Expand Up @@ -88,6 +73,5 @@ ros::Publisher ShapeShifter::advertise(ros::NodeHandle& nh, const std::string& t

uint32_t ShapeShifter::size() const
{
return msgBufUsed;
return msgBuf.size();
}

0 comments on commit 40ad896

Please sign in to comment.