Differential D22889 Diff 62963 src/util/externalcommandhelper.cpp

Changeset View

Standalone View

View Options

src/util/externalcommandhelper.cpp

Show First 20 Lines • Show All 52 Lines • ▼ Show 20 Line(s)50/** Reads the given number of bytes from the sourceDevice into the given buffer.
53 @return zero on success, non-zero on failure 53​ @return zero on success, non-zero on failure
54*/ 54​*/
55int ExternalCommandHelper::helperMain(int argc, char **argv) 55​int ExternalCommandHelper::helperMain(int argc, char **argv)
56{ 56​{
57 QCoreApplication app(argc, argv); 57​ QCoreApplication app(argc, argv);
58 58​
59 if (!QDBusConnection::systemBus().isConnected() || 59​ if (!QDBusConnection::systemBus().isConnected() ||
60 !QDBusConnection::systemBus().registerService(QStringLiteral("org.kde.kpmcore.helperinterface")) || 60​ !QDBusConnection::systemBus().registerService(QStringLiteral("org.kde.kpmcore.helperinterface")) ||
61 !QDBusConnection::systemBus().registerObject(QStringLiteral("/Helper"), this, QDBusConnection::ExportAllSlots)) { 61​ !QDBusConnection::systemBus().registerService(QStringLiteral("org.kde.kpmcore.applicationinterface")) ||
62​ !QDBusConnection::systemBus().registerObject(QStringLiteral("/Helper"), this, QDBusConnection::ExportAllSlots) ||
broulikUnsubmitted
Done

One should register objects before registering the service as the service might become visible to the bus in an inconsistent state (i.e. show up before the objects are registered)

broulik: One should register objects before registering the service as the service might become visible…
63​ !QDBusConnection::systemBus().registerObject(QStringLiteral("/Application"), this, QDBusConnection::ExportAllSlots)) {
cjlcarvalhoUnsubmitted
Not Done

What happens when a service/object is registered and the other one is not?

cjlcarvalho: What happens when a service/object is registered and the other one is not?
64​
62 qDebug() << "Failed to initialize the Helper"; 65​ qDebug() << "Failed to initialize the Helper";
63 qWarning() << QDBusConnection::systemBus().lastError().message(); 66​ qWarning() << QDBusConnection::systemBus().lastError().message();
64 67​
65 ​
66 // We have no reason to live when Main GUI app has expired 68​ // We have no reason to live when Main GUI app has expired
67 ​
68 ​
69 qApp->quit(); 69​ qApp->quit();
70 70​
71 return app.exec(); 71​ return app.exec();
72 } 72​ }
73 73​
74 m_loop = std::make_unique<QEventLoop>(); 74​ m_loop = std::make_unique<QEventLoop>();
75 emit reportProgress(QVariantMap()); 75​
76​ /*================Call App slot===================*/
77​ if (!QDBusConnection::systemBus().isConnected()) {
78​ qWarning() << QDBusConnection::systemBus().lastError().message();
79​ QTimer::singleShot(1000, this, &QCoreApplication::quit);
80​ return false;
81​ }
82​
83​ auto interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.applicationinterface"),
84​ QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
85​ interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
86​
87​ // We send zero percent new data on initial start-up
88​ QDBusPendingCall pcall = interface->emitNewData(0);
89​
90​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
91​ QEventLoop loop;
92​
93​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
94​ loop.exit();
95​
96​ if (watcher->isError())
97​ qWarning() << watcher->error();
98​ };
99​
100​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
101​ loop.exec();
102​ /*================End Call App slot===================*/
76 103​
77 // QDBus Service watcher which keeps an eye on the client (Main GUI app) 104​ // QDBus Service watcher which keeps an eye on the client (Main GUI app)
78 // End the loop and return only once the client has unregistered over the QDBus. 105​ // End the loop and return only once the client has unregistered over the QDBus.
79 auto serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.kde.kpmcore.applicationinterface"), 106​ auto serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.kde.kpmcore.applicationinterface"),
80 QDBusConnection::systemBus(), 107​ QDBusConnection::systemBus(),
81 QDBusServiceWatcher::WatchForUnregistration, 108​ QDBusServiceWatcher::WatchForUnregistration,
82 this); 109​ this);
83 110​
84 connect(serviceWatcher, &QDBusServiceWatcher::serviceUnregistered, 111​ connect(serviceWatcher, &QDBusServiceWatcher::serviceUnregistered,
85 [this]() { 112​ [this]() {
86 m_loop->exit(); 113​ m_loop->exit();
87 }); 114​ });
88 115​
89 m_loop->exec(); 116​ m_loop->exec();
90 117​
91 qApp->quit(); 118​ qApp->quit();
92 119​
93 return app.exec(); 120​ return app.exec();
94} 121​}
95 122​
96 ​
97/** Reads the given number of bytes from the sourceDevice into the given buffer. 123​/** Reads the given number of bytes from the sourceDevice into the given buffer.
98 @param sourceDevice device or file to read from 124​ @param sourceDevice device or file to read from
99 @param buffer buffer to store the bytes read in 125​ @param buffer buffer to store the bytes read in
100 @param offset offset where to begin reading 126​ @param offset offset where to begin reading
101 @param size the number of bytes to read 127​ @param size the number of bytes to read
102 @return true on success 128​ @return true on success
103*/ 129​*/
104bool ExternalCommandHelper::readData(const QString& sourceDevice, QByteArray& buffer, const qint64 offset, const qint64 size) 130​bool ExternalCommandHelper::readData(const QString& sourceDevice, QByteArray& buffer, const qint64 offset, const qint64 size)
▲ Show 20 Lines • Show All 43 Lines • ▼ Show 20 Line(s)161{
148 if (device.write(buffer) != buffer.size()) { 174​ if (device.write(buffer) != buffer.size()) {
149 qCritical() << xi18n("Could not write to device <filename>%1</filename>.", targetDevice); 175​ qCritical() << xi18n("Could not write to device <filename>%1</filename>.", targetDevice);
150 return false; 176​ return false;
151 } 177​ }
152 178​
153 return true; 179​ return true;
154} 180​}
155 181​
156// If targetDevice is empty then return QByteArray with data that was read from disk. 182​// If targetDevice is empty then return QByteArray with data that was read from disk.
broulikUnsubmitted
Done

This is a lie.

broulik: This is a lie.
157QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize) 183​QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize)
158{ 184​{
159 QVariantMap reply; 185​ QVariantMap reply;
160 reply[QStringLiteral("success")] = true; 186​ reply[QStringLiteral("success")] = true;
161 187​
broulikUnsubmitted
Done

QDBusInterface constructor blocks introspecting DBus. You probably want to use an interface generated from an XML file to avoid this.

broulik: `QDBusInterface` constructor blocks introspecting DBus. You probably want to use an interface…
162 const qint64 blocksToCopy = sourceLength / blockSize; 188​ const qint64 blocksToCopy = sourceLength / blockSize;
163 qint64 readOffset = sourceFirstByte; 189​ qint64 readOffset = sourceFirstByte;
164 qint64 writeOffset = targetFirstByte; 190​ qint64 writeOffset = targetFirstByte;
165 qint32 copyDirection = 1; 191​ qint32 copyDirection = 1;
166 192​
167 if (targetFirstByte > sourceFirstByte) { 193​ if (targetFirstByte > sourceFirstByte) {
168 readOffset = sourceFirstByte + sourceLength - blockSize; 194​ readOffset = sourceFirstByte + sourceLength - blockSize;
169 writeOffset = targetFirstByte + sourceLength - blockSize; 195​ writeOffset = targetFirstByte + sourceLength - blockSize;
170 copyDirection = -1; 196​ copyDirection = -1;
171 } 197​ }
172 198​
broulikUnsubmitted
Not Done

What's the point of the event loop?

broulik: What's the point of the event loop?
173 const qint64 lastBlock = sourceLength % blockSize; 199​ const qint64 lastBlock = sourceLength % blockSize;
174 200​
175 qint64 bytesWritten = 0; 201​ qint64 bytesWritten = 0;
176 qint64 blocksCopied = 0; 202​ qint64 blocksCopied = 0;
177 203​
178 QByteArray buffer; 204​ QByteArray buffer;
179 int percent = 0; 205​ int percent = 0;
180 QTime t; ​
181 206​
207​ QTime t;
182 t.start(); 208​ t.start();
broulikUnsubmitted
Done

This probably wants to be a QElapsedTimer

broulik: This probably wants to be a `QElapsedTimer`
183 209​
184 QVariantMap report; 210​ /*================Call App slot===================*/
211​ if (!QDBusConnection::systemBus().isConnected()) {
212​ qWarning() << QDBusConnection::systemBus().lastError().message();
213​ QTimer::singleShot(1000, this, &QCoreApplication::quit);
214​ return QVariantMap();
215​ }
216​
217​ auto interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.applicationinterface"),
218​ QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
219​ interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
185 220​
186 report[QStringLiteral("report")] = xi18nc("@info:progress", "Copying %1 blocks (%2 bytes) from %3 to %4, direction: %5.", blocksToCopy, 221​ // We send report progress as new data to main Application
222​ QDBusPendingCall pcall = interface->emitNewData(xi18nc("@info:progress", "Copying %1 blocks (%2 bytes) from %3 to %4, direction: %5.", blocksToCopy,
187 sourceLength, readOffset, writeOffset, copyDirection == 1 ? i18nc("direction: left", "left") 223​ sourceLength, readOffset, writeOffset, copyDirection == 1 ? i18nc("direction: left", "left")
188 : i18nc("direction: right", "right")); 224​ : i18nc("direction: right", "right")));
189 225​
190 emit reportProgress(report); 226​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
broulikUnsubmitted
Done

You're leaking the watcher, call watcher->deleteLater() in the finished handler

broulik: You're leaking the watcher, call `watcher->deleteLater()` in the `finished` handler
227​ QEventLoop loop;
228​
229​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
230​ loop.exit();
231​
232​ if (watcher->isError())
233​ qWarning() << watcher->error();
234​ };
235​
236​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
237​ loop.exec();
238​ /*================End Call App slot===================*/
191 239​
192 bool rval = true; 240​ bool rval = true;
193 241​
194 while (blocksCopied < blocksToCopy && !targetDevice.isEmpty()) { 242​ while (blocksCopied < blocksToCopy && !targetDevice.isEmpty()) {
195 if (!(rval = readData(sourceDevice, buffer, readOffset + blockSize * blocksCopied * copyDirection, blockSize))) 243​ if (!(rval = readData(sourceDevice, buffer, readOffset + blockSize * blocksCopied * copyDirection, blockSize)))
196 break; 244​ break;
197 245​
198 if (!(rval = writeData(targetDevice, buffer, writeOffset + blockSize * blocksCopied * copyDirection))) 246​ if (!(rval = writeData(targetDevice, buffer, writeOffset + blockSize * blocksCopied * copyDirection)))
199 break; 247​ break;
200 248​
201 bytesWritten += buffer.size(); 249​ bytesWritten += buffer.size();
202 250​
203 if (++blocksCopied * 100 / blocksToCopy != percent) { 251​ if (++blocksCopied * 100 / blocksToCopy != percent) {
204 percent = blocksCopied * 100 / blocksToCopy; 252​ percent = blocksCopied * 100 / blocksToCopy;
205 253​
206 if (percent % 5 == 0 && t.elapsed() > 1000) { 254​ if (percent % 5 == 0 && t.elapsed() > 1000) {
207 const qint64 mibsPerSec = (blocksCopied * blockSize / 1024 / 1024) / (t.elapsed() / 1000); 255​ const qint64 mibsPerSec = (blocksCopied * blockSize / 1024 / 1024) / (t.elapsed() / 1000);
208 const qint64 estSecsLeft = (100 - percent) * t.elapsed() / percent / 1000; 256​ const qint64 estSecsLeft = (100 - percent) * t.elapsed() / percent / 1000;
209 report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying %1 MiB/second, estimated time left: %2", mibsPerSec, QTime(0, 0).addSecs(estSecsLeft).toString()); 257​
210 emit reportProgress(report); 258​ /*================Call App slot===================*/
259​ if (!QDBusConnection::systemBus().isConnected()) {
260​ qWarning() << QDBusConnection::systemBus().lastError().message();
261​ QTimer::singleShot(1000, this, &QCoreApplication::quit);
262​ return QVariantMap();
263​ }
264​
265​ auto interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.applicationinterface"),
266​ QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
267​ interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
268​
269​ // We send report progress as new data to main Application
270​ QDBusPendingCall pcall = interface->emitNewData(xi18nc("@info:progress", "Copying %1 MiB/second, estimated time left: %2", mibsPerSec, QTime(0, 0).addSecs(estSecsLeft).toString()));
271​
272​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
273​ QEventLoop loop;
274​
275​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
276​ loop.exit();
277​
278​ if (watcher->isError())
279​ qWarning() << watcher->error();
280​ };
281​
282​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
stikonasUnsubmitted
Done

Most of this code (except for asyncCall is the same in all cases).

It would make sense to factor it out into a separate function which might e.g. take an argument that is passed to emitNewData.

If you do just this you might still need two function (one for int overload that passed progress percents, one for the other call with QStrings).

It might be possible to unify it more but at leat do that...

stikonas: Most of this code (except for asyncCall is the same in all cases). It would make sense to…
283​ loop.exec();
284​ /*================End Call App slot===================*/
285​
286​ }
287​ /*================Call App slot===================*/
288​ if (!QDBusConnection::systemBus().isConnected()) {
289​ qWarning() << QDBusConnection::systemBus().lastError().message();
290​ QTimer::singleShot(1000, this, &QCoreApplication::quit);
291​ return QVariantMap();
211 } 292​ }
212 emit progress(percent); 293​
294​ auto interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.applicationinterface"),
295​ QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
296​ interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
297​
298​ // We send report progress as new data to main Application
299​ QDBusPendingCall pcall = interface->emitNewData(percent);
300​
301​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
302​ QEventLoop loop;
303​
304​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
305​ loop.exit();
306​
307​ if (watcher->isError())
308​ qWarning() << watcher->error();
309​ };
310​
311​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
312​ loop.exec();
313​ /*================End Call App slot===================*/
213 } 314​ }
214 } 315​ }
215 316​
216 // copy the remainder 317​ // copy the remainder
217 if (rval && lastBlock > 0) { 318​ if (rval && lastBlock > 0) {
218 Q_ASSERT(lastBlock < blockSize); 319​ Q_ASSERT(lastBlock < blockSize);
219 320​
220 const qint64 lastBlockReadOffset = copyDirection > 0 ? readOffset + blockSize * blocksCopied : sourceFirstByte; 321​ const qint64 lastBlockReadOffset = copyDirection > 0 ? readOffset + blockSize * blocksCopied : sourceFirstByte;
221 const qint64 lastBlockWriteOffset = copyDirection > 0 ? writeOffset + blockSize * blocksCopied : targetFirstByte; 322​ const qint64 lastBlockWriteOffset = copyDirection > 0 ? writeOffset + blockSize * blocksCopied : targetFirstByte;
222 323​
223 report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset); 324​ report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset);
224 325​
225 emit reportProgress(report); 326​ /*================Call App slot===================*/
327​ if (!QDBusConnection::systemBus().isConnected()) {
328​ qWarning() << QDBusConnection::systemBus().lastError().message();
329​ QTimer::singleShot(1000, this, &QCoreApplication::quit);
330​ return QVariantMap();
331​ }
332​
333​ auto interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.applicationinterface"),
334​ QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
335​ interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
336​
337​ // We send zero percent new data on initial start-up
338​ QDBusPendingCall pcall = interface->emitNewData(xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset));
339​
340​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
341​ QEventLoop loop;
342​
343​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
344​ loop.exit();
345​
346​ if (watcher->isError())
347​ qWarning() << watcher->error();
348​ };
349​
350​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
351​ loop.exec();
352​ /*================End Call App slot===================*/
226 353​
227 rval = readData(sourceDevice, buffer, lastBlockReadOffset, lastBlock); 354​ rval = readData(sourceDevice, buffer, lastBlockReadOffset, lastBlock);
228 355​
229 if (rval) { 356​ if (rval) {
230 if (targetDevice.isEmpty()) 357​ if (targetDevice.isEmpty())
231 reply[QStringLiteral("targetByteArray")] = buffer; 358​ reply[QStringLiteral("targetByteArray")] = buffer;
232 else 359​ else
233 rval = writeData(targetDevice, buffer, lastBlockWriteOffset); 360​ rval = writeData(targetDevice, buffer, lastBlockWriteOffset);
234 } 361​ }
235 362​
236 if (rval) { 363​ if (rval) {
237 emit progress(100); 364​ /*================Call App slot===================*/
365​ if (!QDBusConnection::systemBus().isConnected()) {
366​ qWarning() << QDBusConnection::systemBus().lastError().message();
367​ QTimer::singleShot(1000, this, &QCoreApplication::quit);
368​ return QVariantMap();
369​ }
370​
371​ auto interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.applicationinterface"),
372​ QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
373​ interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
374​
375​ // We send zero percent new data on initial start-up
376​ QDBusPendingCall pcall = interface->emitNewData(100);
377​
378​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
379​ QEventLoop loop;
380​
381​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
382​ loop.exit();
383​
384​ if (watcher->isError())
385​ qWarning() << watcher->error();
386​ };
387​
388​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
389​ loop.exec();
390​ /*================End Call App slot===================*/
391​
238 bytesWritten += buffer.size(); 392​ bytesWritten += buffer.size();
239 } 393​ }
240 } 394​ }
241 395​
242 report[QStringLiteral("report")] = xi18ncp("@info:progress argument 2 is a string such as 7 bytes (localized accordingly)", "Copying 1 block (%2) finished.", "Copying %1 blocks (%2) finished.", blocksCopied, i18np("1 byte", "%1 bytes", bytesWritten)); 396​ /*================Call App slot===================*/
397​ if (!QDBusConnection::systemBus().isConnected()) {
398​ qWarning() << QDBusConnection::systemBus().lastError().message();
399​ QTimer::singleShot(1000, this, &QCoreApplication::quit);
400​ return QVariantMap();
401​ }
402​
403​ auto interface = new org::kde::kpmcore::externalcommand(QStringLiteral("org.kde.kpmcore.applicationinterface"),
404​ QStringLiteral("/Application"), QDBusConnection::systemBus(), this);
405​ interface->setTimeout(10 * 24 * 3600 * 1000); // 10 days
406​
407​ // We send zero percent new data on initial start-up
408​ QDBusPendingCall pcall2 = interface->emitNewData(xi18ncp("@info:progress argument 2 is a string such as 7 bytes (localized accordingly)", "Copying 1 block (%2) finished.", "Copying %1 blocks (%2) finished.", blocksCopied, i18np("1 byte", "%1 bytes", bytesWritten)));
409​
410​ QDBusPendingCallWatcher *watcher2 = new QDBusPendingCallWatcher(pcall2, this);
411​ QEventLoop loop2;
412​
413​ auto exitLoop2 = [&] (QDBusPendingCallWatcher *watcher2) {
414​ loop2.exit();
415​
416​ if (watcher2->isError())
417​ qWarning() << watcher2->error();
418​ };
243 419​
244 emit reportProgress(report); 420​ connect(watcher2, &QDBusPendingCallWatcher::finished, exitLoop2);
421​ loop2.exec();
422​ /*================End Call App slot===================*/
245 423​
246 reply[QStringLiteral("success")] = rval; 424​ reply[QStringLiteral("success")] = rval;
247 return reply; 425​ return reply;
248} 426​}
249 427​
250bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte) 428​bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte)
251{ 429​{
252 // Do not allow using this helper for writing to arbitrary location 430​ // Do not allow using this helper for writing to arbitrary location
▲ Show 20 Lines • Show All 69 Lines • Show Last 20 Lines