From 87b87745c466f45e3e75dd1b0e88c2ffc14b6245 Mon Sep 17 00:00:00 2001 From: Alexey Esaulenko Date: Thu, 18 Jan 2024 10:57:27 +0400 Subject: [PATCH 1/2] FirmwareUploader: fix memory leaks --- firmwareuploaderwindow.cpp | 73 ++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/firmwareuploaderwindow.cpp b/firmwareuploaderwindow.cpp index 05028be1..0a6be758 100644 --- a/firmwareuploaderwindow.cpp +++ b/firmwareuploaderwindow.cpp @@ -133,38 +133,38 @@ void FirmwareUploaderWindow::timerElapsed() void FirmwareUploaderWindow::sendFirmwareChunk() { - CANFrame *output = new CANFrame; + CANFrame output; int firmwareLocation = currentSendingPosition * 4; int xorByte = 0; - output->setExtendedFrameFormat(false); + output.setExtendedFrameFormat(false); QByteArray bytes(7,0); - output->bus = bus; - output->setFrameId(baseAddress + 0x16); - output->payload()[0] = currentSendingPosition & 0xFF; - output->payload()[1] = (currentSendingPosition >> 8) & 0xFF; - output->payload()[2] = firmwareData[firmwareLocation++]; - output->payload()[3] = firmwareData[firmwareLocation++]; - output->payload()[4] = firmwareData[firmwareLocation++]; - output->payload()[5] = firmwareData[firmwareLocation++]; - for (int i = 0; i < 6; i++) xorByte = xorByte ^ static_cast(output->payload()[i]); - output->payload()[6] = xorByte; - output->setPayload(bytes); - CANConManager::getInstance()->sendFrame(*output); + output.bus = bus; + output.setFrameId(baseAddress + 0x16); + output.payload()[0] = currentSendingPosition & 0xFF; + output.payload()[1] = (currentSendingPosition >> 8) & 0xFF; + output.payload()[2] = firmwareData[firmwareLocation++]; + output.payload()[3] = firmwareData[firmwareLocation++]; + output.payload()[4] = firmwareData[firmwareLocation++]; + output.payload()[5] = firmwareData[firmwareLocation++]; + for (int i = 0; i < 6; i++) xorByte ^= static_cast(output.payload()[i]); + output.payload()[6] = xorByte; + output.setPayload(bytes); + CANConManager::getInstance()->sendFrame(output); timer->start(); } void FirmwareUploaderWindow::sendFirmwareEnding() { - CANFrame *output = new CANFrame; - output->setExtendedFrameFormat(false); - output->bus = bus; + CANFrame output; + output.setExtendedFrameFormat(false); + output.bus = bus; QByteArray bytes(4,0); - output->setFrameId(baseAddress + 0x30); - output->payload()[3] = 0xC0; - output->payload()[2] = 0xDE; - output->payload()[1] = 0xFA; - output->payload()[0] = 0xDE; - output->setPayload(bytes); + output.setFrameId(baseAddress + 0x30); + output.payload()[3] = 0xC0; + output.payload()[2] = 0xDE; + output.payload()[1] = 0xFA; + output.payload()[0] = 0xDE; + output.setPayload(bytes); //sendCANFrame(output, bus); } @@ -182,12 +182,12 @@ void FirmwareUploaderWindow::handleStartStopTransfer() qDebug() << "Base address: " + QString::number(baseAddress); CANConManager::getInstance()->addTargettedFrame(bus, baseAddress + 0x10, 0x7FF, this); CANConManager::getInstance()->addTargettedFrame(bus, baseAddress + 0x20, 0x7FF, this); - CANFrame *output = new CANFrame; - output->setExtendedFrameFormat(false); + CANFrame output; + output.setExtendedFrameFormat(false); QByteArray bytes(8,0); - output->bus = bus; - output->setFrameId(baseAddress); - output->setFrameType(QCanBusFrame::DataFrame); + output.bus = bus; + output.setFrameId(baseAddress); + output.setFrameType(QCanBusFrame::DataFrame); bytes[0] = 0xEF; bytes[1] = 0xBE; @@ -197,8 +197,8 @@ void FirmwareUploaderWindow::handleStartStopTransfer() bytes[5] = (token >> 8) & 0xFF; bytes[6] = (token >> 16) & 0xFF; bytes[7] = (token >> 24) & 0xFF; - output->setPayload(bytes); - CANConManager::getInstance()->sendFrame(*output); + output.setPayload(bytes); + CANConManager::getInstance()->sendFrame(output); } else //stop anything in process { @@ -209,7 +209,6 @@ void FirmwareUploaderWindow::handleStartStopTransfer() void FirmwareUploaderWindow::handleLoadFile() { - QString filename; QFileDialog dialog; QStringList filters; @@ -221,7 +220,7 @@ void FirmwareUploaderWindow::handleLoadFile() if (dialog.exec() == QDialog::Accepted) { - filename = dialog.selectedFiles()[0]; + QString filename = dialog.selectedFiles().constFirst(); loadBinaryFile(filename); } @@ -232,22 +231,20 @@ void FirmwareUploaderWindow::loadBinaryFile(QString filename) if (transferInProgress) handleStartStopTransfer(); - QFile *inFile = new QFile(filename); + QFile inFile(filename); - if (!inFile->open(QIODevice::ReadOnly)) + if (!inFile.open(QIODevice::ReadOnly)) { - delete inFile; return; } - firmwareData = inFile->readAll(); + firmwareData = inFile.readAll(); currentSendingPosition = 0; firmwareSize = firmwareData.length(); updateProgress(); - inFile->close(); - delete inFile; + inFile.close(); } From 9e1d10b467f73b5c8ac861729775034a9d194586 Mon Sep 17 00:00:00 2001 From: Alexey Esaulenko Date: Thu, 18 Jan 2024 16:10:37 +0400 Subject: [PATCH 2/2] FirmwareUpdater: fix populating CAN frames frame.payload() returns a copy (!) of data, so assignig to it has no effect --- firmwareuploaderwindow.cpp | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/firmwareuploaderwindow.cpp b/firmwareuploaderwindow.cpp index 0a6be758..bc234e24 100644 --- a/firmwareuploaderwindow.cpp +++ b/firmwareuploaderwindow.cpp @@ -140,14 +140,14 @@ void FirmwareUploaderWindow::sendFirmwareChunk() QByteArray bytes(7,0); output.bus = bus; output.setFrameId(baseAddress + 0x16); - output.payload()[0] = currentSendingPosition & 0xFF; - output.payload()[1] = (currentSendingPosition >> 8) & 0xFF; - output.payload()[2] = firmwareData[firmwareLocation++]; - output.payload()[3] = firmwareData[firmwareLocation++]; - output.payload()[4] = firmwareData[firmwareLocation++]; - output.payload()[5] = firmwareData[firmwareLocation++]; - for (int i = 0; i < 6; i++) xorByte ^= static_cast(output.payload()[i]); - output.payload()[6] = xorByte; + bytes[0] = currentSendingPosition & 0xFF; + bytes[1] = (currentSendingPosition >> 8) & 0xFF; + bytes[2] = firmwareData[firmwareLocation++]; + bytes[3] = firmwareData[firmwareLocation++]; + bytes[4] = firmwareData[firmwareLocation++]; + bytes[5] = firmwareData[firmwareLocation++]; + for (int i = 0; i < 6; i++) xorByte ^= static_cast(bytes[i]); + bytes[6] = xorByte; output.setPayload(bytes); CANConManager::getInstance()->sendFrame(output); timer->start(); @@ -160,10 +160,10 @@ void FirmwareUploaderWindow::sendFirmwareEnding() output.bus = bus; QByteArray bytes(4,0); output.setFrameId(baseAddress + 0x30); - output.payload()[3] = 0xC0; - output.payload()[2] = 0xDE; - output.payload()[1] = 0xFA; - output.payload()[0] = 0xDE; + bytes[3] = (char)0xC0; + bytes[2] = (char)0xDE; + bytes[1] = (char)0xFA; + bytes[0] = (char)0xDE; output.setPayload(bytes); //sendCANFrame(output, bus); } @@ -189,10 +189,10 @@ void FirmwareUploaderWindow::handleStartStopTransfer() output.setFrameId(baseAddress); output.setFrameType(QCanBusFrame::DataFrame); - bytes[0] = 0xEF; - bytes[1] = 0xBE; - bytes[2] = 0xAD; - bytes[3] = 0xDE; + bytes[0] = (char)0xEF; + bytes[1] = (char)0xBE; + bytes[2] = (char)0xAD; + bytes[3] = (char)0xDE; bytes[4] = token & 0xFF; bytes[5] = (token >> 8) & 0xFF; bytes[6] = (token >> 16) & 0xFF;