Skip to content

Commit

Permalink
Improve code with std::unique_ptr (#2643)
Browse files Browse the repository at this point in the history
* **Memory management improvements**
Switch to using `unique_ptr` instead of raw pointers to prevent memory leaks and ease resource management.
* **Fix memory leaks in StreamTransformer.cpp**
Address memory leaks, enhancing application stability and performance.
* **Simplify and improve CommandOutput.cpp**
Replace a code segment with a more efficient and straightforward approach.
* **Refactor Basic_Audio sample application**
Incorporate smart pointers and reorganize some code segments for better code quality and memory safety.
  • Loading branch information
mikee47 authored May 27, 2023
1 parent c01e326 commit 02835d8
Show file tree
Hide file tree
Showing 28 changed files with 183 additions and 251 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <Network/TcpClient.h>
#include "TcpTransport.h"
#include "TcpClientStream.h"
#include <memory>

namespace Hosted
{
Expand All @@ -27,12 +28,7 @@ class TcpClientTransport : public TcpTransport
TcpClientTransport(TcpClient& client)
{
client.setReceiveDelegate(TcpClientDataDelegate(&TcpClientTransport::process, this));
stream = new TcpClientStream(client);
}

~TcpClientTransport()
{
delete stream;
stream.reset(new TcpClientStream(client));
}

protected:
Expand All @@ -46,7 +42,7 @@ class TcpClientTransport : public TcpTransport
}

private:
TcpClientStream* stream = nullptr;
std::unique_ptr<TcpClientStream> stream;
};

} // namespace Transport
Expand Down
39 changes: 18 additions & 21 deletions Sming/Components/Network/Arch/Esp8266/Platform/AccessPointImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ bool AccessPointImpl::config(const String& ssid, String password, AUTH_MODE mode
debugf("AP configuration was updated");
} else {
debugf("Set AP configuration in background");
if(runConfig != nullptr) {
delete runConfig;
}
runConfig = new softap_config();
memcpy(runConfig, &config, sizeof(softap_config));
runConfig.reset(new softap_config(config));
}
} else {
debugf("AP configuration loaded");
Expand Down Expand Up @@ -190,22 +186,23 @@ std::unique_ptr<StationList> AccessPointImpl::getStations() const

void AccessPointImpl::onSystemReady()
{
if(runConfig != nullptr) {
noInterrupts();
bool enabled = isEnabled();
enable(true, false);
wifi_softap_dhcps_stop();

if(!wifi_softap_set_config(runConfig)) {
debugf("Can't set AP config on system ready event!");
} else {
debugf("AP configuration was updated on system ready event");
}
delete runConfig;
runConfig = nullptr;
if(!runConfig) {
return;
}

noInterrupts();
bool enabled = isEnabled();
enable(true, false);
wifi_softap_dhcps_stop();

wifi_softap_dhcps_start();
enable(enabled, false);
interrupts();
if(!wifi_softap_set_config(runConfig.get())) {
debugf("Can't set AP config on system ready event!");
} else {
debugf("AP configuration was updated on system ready event");
}
runConfig.reset();

wifi_softap_dhcps_start();
enable(enabled, false);
interrupts();
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ class AccessPointImpl : public AccessPointClass, protected ISystemReadyHandler
void onSystemReady() override;

private:
softap_config* runConfig = nullptr;
std::unique_ptr<softap_config> runConfig;
};
18 changes: 7 additions & 11 deletions Sming/Components/rboot/include/Network/RbootHttpUpdater.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,21 @@ class RbootHttpUpdater : protected HttpClient
struct Item {
String url;
uint32_t targetOffset;
size_t size; // << max allowed size
RbootOutputStream* stream{nullptr}; // (optional) output stream to use.
size_t size; // << max allowed size
std::unique_ptr<RbootOutputStream> stream; // (optional) output stream to use.

Item(String url, uint32_t targetOffset, size_t size, RbootOutputStream* stream)
: url(url), targetOffset(targetOffset), size(size), stream(stream)
: url(url), targetOffset(targetOffset), size(size)
{
}

~Item()
{
delete stream;
this->stream.reset(stream);
}

RbootOutputStream* getStream()
{
if(stream == nullptr) {
stream = new RbootOutputStream(targetOffset, size);
if(!stream) {
stream.reset(new RbootOutputStream(targetOffset, size));
}
return stream;
return stream.get();
}
};

Expand Down
13 changes: 6 additions & 7 deletions Sming/Components/ssl/Axtls/AxConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <Network/Ssl/Validator.h>
#include "AxCertificate.h"
#include "AxError.h"
#include <memory>

namespace Ssl
{
Expand All @@ -26,7 +27,6 @@ class AxConnection : public Connection

~AxConnection()
{
delete certificate;
// Typically sends out closing message
ssl_free(ssl);
}
Expand Down Expand Up @@ -60,17 +60,16 @@ class AxConnection : public Connection

const Certificate* getCertificate() const override
{
if(certificate == nullptr && ssl->x509_ctx != nullptr) {
certificate = new AxCertificate(ssl);
if(!certificate && ssl->x509_ctx != nullptr) {
certificate.reset(new AxCertificate(ssl));
}

return certificate;
return certificate.get();
}

void freeCertificate() override
{
delete certificate;
certificate = nullptr;
certificate.reset();
}

int read(InputBuffer& input, uint8_t*& output) override;
Expand All @@ -93,7 +92,7 @@ class AxConnection : public Connection

private:
SSL* ssl{nullptr};
mutable AxCertificate* certificate{nullptr};
mutable std::unique_ptr<AxCertificate> certificate;
InputBuffer* input{nullptr};
};

Expand Down
16 changes: 8 additions & 8 deletions Sming/Components/ssl/include/Network/Ssl/Session.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "KeyCertPair.h"
#include "ValidatorList.h"
#include <Platform/System.h>
#include <memory>

class TcpConnection;

Expand Down Expand Up @@ -121,7 +122,6 @@ class Session
~Session()
{
close();
delete sessionId;
}

/**
Expand All @@ -130,7 +130,7 @@ class Session
*/
const SessionId* getSessionId() const
{
return sessionId;
return sessionId.get();
}

/**
Expand All @@ -147,8 +147,8 @@ class Session
*/
void setConnection(Connection* connection)
{
assert(this->connection == nullptr);
this->connection = connection;
assert(!this->connection);
this->connection.reset(connection);
}

/**
Expand All @@ -157,7 +157,7 @@ class Session
*/
Connection* getConnection()
{
return connection;
return connection.get();
}

/**
Expand Down Expand Up @@ -223,9 +223,9 @@ class Session
void endHandshake();

private:
Context* context = nullptr;
Connection* connection = nullptr;
SessionId* sessionId = nullptr;
std::unique_ptr<Context> context;
std::unique_ptr<Connection> connection;
std::unique_ptr<SessionId> sessionId;
CpuFrequency curFreq = CpuFrequency(0);
};

Expand Down
43 changes: 20 additions & 23 deletions Sming/Components/ssl/src/Session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ bool Session::onAccept(TcpConnection* client, tcp_pcb* tcp)
return false;
}

if(context == nullptr) {
if(!context) {
assert(factory != nullptr);
context = factory->createContext(*this);
if(context == nullptr) {
context.reset(factory->createContext(*this));
if(!context) {
return false;
}

Expand All @@ -73,14 +73,14 @@ bool Session::onConnect(tcp_pcb* tcp)
{
debug_d("SSL %p: Starting connection...", this);

assert(connection == nullptr);
assert(context == nullptr);
assert(!connection);
assert(!context);

// Client Session
delete context;
context.reset();
assert(factory != nullptr);
context = factory->createContext(*this);
if(context == nullptr) {
context.reset(factory->createContext(*this));
if(!context) {
return false;
}

Expand All @@ -90,16 +90,16 @@ bool Session::onConnect(tcp_pcb* tcp)
return false;
}

if(sessionId != nullptr && sessionId->isValid()) {
if(sessionId && sessionId->isValid()) {
debug_d("-----BEGIN SSL SESSION PARAMETERS-----");
debug_d("SessionId: %s", toString(*sessionId).c_str());
debug_d("------END SSL SESSION PARAMETERS------");
}

beginHandshake();

connection = context->createClient(tcp);
if(connection == nullptr) {
connection.reset(context->createClient(tcp));
if(!connection) {
endHandshake();
return false;
}
Expand Down Expand Up @@ -134,19 +134,16 @@ void Session::close()
{
debug_d("SSL %p: closing ...", this);

delete connection;
connection = nullptr;

delete context;
context = nullptr;
connection.reset();
context.reset();

hostName = nullptr;
maxBufferSize = MaxBufferSize::Default;
}

int Session::read(InputBuffer& input, uint8_t*& output)
{
if(connection == nullptr) {
if(!connection) {
debug_w("SSL: no connection");
return -1;
}
Expand All @@ -165,7 +162,7 @@ int Session::read(InputBuffer& input, uint8_t*& output)

int Session::write(const uint8_t* data, size_t length)
{
if(connection == nullptr) {
if(!connection) {
debug_e("!! SSL Session connection is NULL");
return ERR_CONN;
}
Expand All @@ -181,7 +178,7 @@ int Session::write(const uint8_t* data, size_t length)

bool Session::validateCertificate()
{
if(connection == nullptr) {
if(!connection) {
debug_w("SSL: connection not set, assuming cert. is OK");
return true;
}
Expand All @@ -202,16 +199,16 @@ void Session::handshakeComplete(bool success)
if(success) {
// If requested, take a copy of the session ID for later re-use
if(options.sessionResume) {
if(sessionId == nullptr) {
sessionId = new SessionId;
if(!sessionId) {
sessionId.reset(new SessionId);
}
*sessionId = connection->getSessionId();
}
} else {
debug_w("SSL Handshake failed");
}

if(options.freeKeyCertAfterHandshake && connection != nullptr) {
if(options.freeKeyCertAfterHandshake && connection) {
connection->freeCertificate();
}
}
Expand All @@ -235,7 +232,7 @@ size_t Session::printTo(Print& p) const
n += p.println(keyCert.getCertificateLength());
n += p.print(_F(" Cert PK Length: "));
n += p.println(keyCert.getKeyLength());
if(connection != nullptr) {
if(connection) {
n += connection->printTo(p);
}

Expand Down
Loading

0 comments on commit 02835d8

Please sign in to comment.