Skip to content

Commit

Permalink
[Messaging] Removing unnecessary string copying and casts (rdkcentral…
Browse files Browse the repository at this point in the history 
…#1601)

* Removing unncessary const casts

* Adding a new optional parameter to NullTerminatedText to avoid unnecessary string copies when cutting the string

* Using the new parameter in NullTerminatedText instead of making a string copy and cutting the text manually

* Optimizing the SetNullTerminatedText method

* Simplifying the method further to avoid unnecessary std::min and code repetition

* Removing unnecessary string copy in Output method from DirectOutput in messaging

* Removing a string creation in Output methods from ConsolStreamRedirect,  putting the buffer into TextMessage constructor instead

* Adding a new constructor to TextMessage which takes buffer and length, use the new maxLength param of NullTerinatedText in Serialize

* Changing the maxLength param to be default as ~0, passing length of the string straight in its constructor so that no excess memory is allocated
  • Loading branch information
@VeithMetro
VeithMetro authored May 15, 2024
1 parent e44638f commit eff6ab5
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 30 deletions.
12 changes: 6 additions & 6 deletions Source/core/Frame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,11 +465,11 @@ namespace Core {

_offset += _container->SetText<TYPENAME>(_offset, text);
}
void NullTerminatedText(const string& text)
void NullTerminatedText(const string& text, const SIZE_CONTEXT maxLength = ~0)
{
ASSERT(_container != nullptr);

_offset += _container->SetNullTerminatedText(_offset, text);
_offset += _container->SetNullTerminatedText(_offset, text, maxLength);
}

private:
Expand Down Expand Up @@ -647,16 +647,16 @@ namespace Core {
return (SetBuffer<TYPENAME>(offset, static_cast<TYPENAME>(convertedText.length()), reinterpret_cast<const uint8_t*>(convertedText.c_str())));
}

SIZE_CONTEXT SetNullTerminatedText(const SIZE_CONTEXT offset, const string& value)
SIZE_CONTEXT SetNullTerminatedText(const SIZE_CONTEXT offset, const string& value, const SIZE_CONTEXT maxLength)
{
std::string convertedText(Core::ToString(value));
SIZE_CONTEXT requiredLength(static_cast< SIZE_CONTEXT>(convertedText.length() + 1));
std::string convertedText(Core::ToString(value).data(), (((maxLength != static_cast<SIZE_CONTEXT>(~0)) && ((value.length() + 1) > maxLength)) ? (maxLength - 1) : value.length()));
SIZE_CONTEXT requiredLength(static_cast<SIZE_CONTEXT>(convertedText.length() + 1));

if ((offset + requiredLength) >= _size) {
Size(offset + requiredLength);
}

::memcpy(&(_data[offset]), convertedText.c_str(), convertedText.length() + 1);
::memcpy(&(_data[offset]), convertedText.c_str(), requiredLength);

return (requiredLength);
}
Expand Down
6 changes: 3 additions & 3 deletions Source/core/MessageStore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ namespace Core {
ASSERT(bufferSize >= (length + extra));

if (bufferSize >= (length + extra)) {
Core::FrameType<0> frame(const_cast<uint8_t*>(buffer) + length, bufferSize - length, bufferSize - length);
Core::FrameType<0> frame(buffer + length, bufferSize - length, bufferSize - length);
Core::FrameType<0>::Writer frameWriter(frame, 0);
frameWriter.Number(_timeStamp);
length += extra;
Expand Down Expand Up @@ -261,7 +261,7 @@ namespace Core {
ASSERT(bufferSize >= (length + extra));

if (bufferSize >= (length + extra)) {
Core::FrameType<0> frame(const_cast<uint8_t*>(buffer) + length, bufferSize - length, bufferSize - length);
Core::FrameType<0> frame(buffer + length, bufferSize - length, bufferSize - length);
Core::FrameType<0>::Writer frameWriter(frame, 0);
frameWriter.NullTerminatedText(_className);
frameWriter.NullTerminatedText(_fileName);
Expand Down Expand Up @@ -333,7 +333,7 @@ namespace Core {
ASSERT(bufferSize >= (length + extra));

if (bufferSize >= (length + extra)) {
Core::FrameType<0> frame(const_cast<uint8_t*>(buffer) + length, bufferSize - length, bufferSize - length);
Core::FrameType<0> frame(buffer + length, bufferSize - length, bufferSize - length);
Core::FrameType<0>::Writer frameWriter(frame, 0);
frameWriter.NullTerminatedText(_callsign);
length += extra;
Expand Down
2 changes: 1 addition & 1 deletion Source/extensions/warningreporting/WarningReportingUnit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ namespace WarningReporting {
}

void WarningReportingUnit::ReportWarningEvent(const char identifier[], const IWarningEvent& information)
{
{
WPEFramework::Core::Messaging::Metadata metadata(WPEFramework::Core::Messaging::Metadata::type::REPORTING, information.Category(), WPEFramework::Core::Messaging::MODULE_REPORTING);
WPEFramework::Core::Messaging::MessageInfo messageInfo(metadata, WPEFramework::Core::Time::Now().Ticks());
WPEFramework::Core::Messaging::IStore::WarningReporting report(messageInfo, identifier);
Expand Down
6 changes: 2 additions & 4 deletions Source/messaging/ConsoleStreamRedirect.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ namespace WPEFramework {
if (OperationalStream::StandardOut::IsEnabled() == true) {
Core::Messaging::MessageInfo messageInfo(OperationalStream::StandardOut::Metadata(), Core::Time::Now().Ticks());
Core::Messaging::IStore::OperationalStream operationalStream(messageInfo);
string text(buffer, length);
TextMessage data(text);
TextMessage data(length, buffer);
MessageUnit::Instance().Push(operationalStream, &data);
}
}
Expand All @@ -46,8 +45,7 @@ namespace WPEFramework {
if (OperationalStream::StandardError::IsEnabled() == true) {
Core::Messaging::MessageInfo messageInfo(OperationalStream::StandardError::Metadata(), Core::Time::Now().Ticks());
Core::Messaging::IStore::OperationalStream operationalStream(messageInfo);
string text(buffer, length);
TextMessage data(text);
TextMessage data(length, buffer);
MessageUnit::Instance().Push(operationalStream, &data);
}
}
Expand Down
6 changes: 2 additions & 4 deletions Source/messaging/DirectOutput.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,15 @@ namespace WPEFramework {
ASSERT(message != nullptr);
ASSERT(messageInfo.Type() != Core::Messaging::Metadata::type::INVALID);

string result = messageInfo.ToString(_abbreviate).c_str() + message->Data();

#ifndef __WINDOWS__
if (_isSyslog == true) {
//use longer messages for syslog
syslog(LOG_NOTICE, "%s\n", result.c_str());
syslog(LOG_NOTICE, "%s%s\n", messageInfo.ToString(_abbreviate).c_str(), message->Data().c_str());
}
else
#endif
{
std::cout << result << std::endl;
std::cout << messageInfo.ToString(_abbreviate).c_str() << message->Data() << std::endl;
}
}
} // namespace Messaging
Expand Down
18 changes: 6 additions & 12 deletions Source/messaging/TextMessage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,22 @@ namespace Messaging {
: _text(text)
{
}
TextMessage(const uint16_t length, const TCHAR buffer[])
: _text(buffer, length)
{
}

TextMessage(const TextMessage&) = delete;
TextMessage& operator=(const TextMessage&) = delete;

uint16_t Serialize(uint8_t buffer[], const uint16_t bufferSize) const override
{
uint16_t length = 0;
Core::FrameType<0> frame(buffer, bufferSize, bufferSize);
Core::FrameType<0>::Writer writer(frame, 0);

if (bufferSize < _text.size() + 1) {
string cutString(_text, 0, bufferSize - 1);
writer.NullTerminatedText(cutString);
length = bufferSize;

}
else {
writer.NullTerminatedText(_text);
length = static_cast<uint16_t>(_text.size() + 1);
}
writer.NullTerminatedText(_text, bufferSize);

return (length);
return (std::min(bufferSize, static_cast<uint16_t>(_text.size() + 1)));
}

uint16_t Deserialize(const uint8_t buffer[], const uint16_t bufferSize) override
Expand Down

0 comments on commit eff6ab5

Please sign in to comment.