Differential D22889 Diff 63037 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​ QDBusInterface *iface = new QDBusInterface(QStringLiteral("org.kde.kpmcore.applicationinterface"), QStringLiteral("/Application"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus());
78​
79​ if (!iface->isValid()) {
80​ return -1;
81​ }
82​
83​ iface->setTimeout(10 * 24 * 3600 * 1000);
84​
85​ // We send zero percent new data on initial start-up
86​ QDBusPendingCall pcall = iface->asyncCall(QLatin1String("emitNewData"), 0);
87​
88​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
89​ QEventLoop loop;
90​
91​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
92​ loop.exit();
93​
94​ if (watcher->isError())
95​ qWarning() << watcher->error();
96​ };
97​
98​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
99​ loop.exec();
100​ /*================End Call App slot===================*/
76 101​
77 // QDBus Service watcher which keeps an eye on the client (Main GUI app) 102​ // 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. 103​ // End the loop and return only once the client has unregistered over the QDBus.
79 auto serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.kde.kpmcore.applicationinterface"), 104​ auto serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.kde.kpmcore.applicationinterface"),
80 QDBusConnection::systemBus(), 105​ QDBusConnection::systemBus(),
81 QDBusServiceWatcher::WatchForUnregistration, 106​ QDBusServiceWatcher::WatchForUnregistration,
82 this); 107​ this);
83 108​
84 connect(serviceWatcher, &QDBusServiceWatcher::serviceUnregistered, 109​ connect(serviceWatcher, &QDBusServiceWatcher::serviceUnregistered,
85 [this]() { 110​ [this]() {
86 m_loop->exit(); 111​ m_loop->exit();
87 }); 112​ });
88 113​
89 m_loop->exec(); 114​ m_loop->exec();
90 115​
91 qApp->quit(); 116​ qApp->quit();
92 117​
93 return app.exec(); 118​ return app.exec();
94} 119​}
95 120​
96 ​
97/** Reads the given number of bytes from the sourceDevice into the given buffer. 121​/** Reads the given number of bytes from the sourceDevice into the given buffer.
98 @param sourceDevice device or file to read from 122​ @param sourceDevice device or file to read from
99 @param buffer buffer to store the bytes read in 123​ @param buffer buffer to store the bytes read in
100 @param offset offset where to begin reading 124​ @param offset offset where to begin reading
101 @param size the number of bytes to read 125​ @param size the number of bytes to read
102 @return true on success 126​ @return true on success
103*/ 127​*/
104bool ExternalCommandHelper::readData(const QString& sourceDevice, QByteArray& buffer, const qint64 offset, const qint64 size) 128​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)159{
148 if (device.write(buffer) != buffer.size()) { 172​ if (device.write(buffer) != buffer.size()) {
149 qCritical() << xi18n("Could not write to device <filename>%1</filename>.", targetDevice); 173​ qCritical() << xi18n("Could not write to device <filename>%1</filename>.", targetDevice);
150 return false; 174​ return false;
151 } 175​ }
152 176​
153 return true; 177​ return true;
154} 178​}
155 179​
156// If targetDevice is empty then return QByteArray with data that was read from disk. 180​// 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) 181​QVariantMap ExternalCommandHelper::copyblocks(const QString& sourceDevice, const qint64 sourceFirstByte, const qint64 sourceLength, const QString& targetDevice, const qint64 targetFirstByte, const qint64 blockSize)
158{ 182​{
159 QVariantMap reply; 183​ QVariantMap reply;
160 reply[QStringLiteral("success")] = true; 184​ reply[QStringLiteral("success")] = true;
161 185​
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; 186​ const qint64 blocksToCopy = sourceLength / blockSize;
163 qint64 readOffset = sourceFirstByte; 187​ qint64 readOffset = sourceFirstByte;
164 qint64 writeOffset = targetFirstByte; 188​ qint64 writeOffset = targetFirstByte;
165 qint32 copyDirection = 1; 189​ qint32 copyDirection = 1;
166 190​
167 if (targetFirstByte > sourceFirstByte) { 191​ if (targetFirstByte > sourceFirstByte) {
168 readOffset = sourceFirstByte + sourceLength - blockSize; 192​ readOffset = sourceFirstByte + sourceLength - blockSize;
169 writeOffset = targetFirstByte + sourceLength - blockSize; 193​ writeOffset = targetFirstByte + sourceLength - blockSize;
170 copyDirection = -1; 194​ copyDirection = -1;
171 } 195​ }
172 196​
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; 197​ const qint64 lastBlock = sourceLength % blockSize;
174 198​
175 qint64 bytesWritten = 0; 199​ qint64 bytesWritten = 0;
176 qint64 blocksCopied = 0; 200​ qint64 blocksCopied = 0;
177 201​
178 QByteArray buffer; 202​ QByteArray buffer;
179 int percent = 0; 203​ int percent = 0;
180 QTime t; ​
181 204​
205​ QTime t;
182 t.start(); 206​ t.start();
broulikUnsubmitted
Done

This probably wants to be a QElapsedTimer

broulik: This probably wants to be a `QElapsedTimer`
183 207​
184 QVariantMap report; 208​ /*================Call App slot===================*/
209​ QDBusInterface *iface = new QDBusInterface(QStringLiteral("org.kde.kpmcore.applicationinterface"), QStringLiteral("/Application"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus());
185 210​
186 report[QStringLiteral("report")] = xi18nc("@info:progress", "Copying %1 blocks (%2 bytes) from %3 to %4, direction: %5.", blocksToCopy, 211​ if (!iface->isValid()) {
212​ return QVariantMap();
213​ }
214​
215​ iface->setTimeout(10 * 24 * 3600 * 1000);
216​
217​ // We send zero percent new data on initial start-up
218​ QDBusPendingCall pcall = iface->asyncCall(QLatin1String("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") 219​ sourceLength, readOffset, writeOffset, copyDirection == 1 ? i18nc("direction: left", "left")
188 : i18nc("direction: right", "right")); 220​ : i18nc("direction: right", "right")));
221​
222​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
223​ QEventLoop loop;
224​
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
225​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
226​ loop.exit();
189 227​
190 emit reportProgress(report); 228​ if (watcher->isError())
229​ qWarning() << watcher->error();
230​ };
231​
232​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
233​ loop.exec();
234​ /*================End Call App slot===================*/
191 235​
192 bool rval = true; 236​ bool rval = true;
193 237​
194 while (blocksCopied < blocksToCopy && !targetDevice.isEmpty()) { 238​ while (blocksCopied < blocksToCopy && !targetDevice.isEmpty()) {
195 if (!(rval = readData(sourceDevice, buffer, readOffset + blockSize * blocksCopied * copyDirection, blockSize))) 239​ if (!(rval = readData(sourceDevice, buffer, readOffset + blockSize * blocksCopied * copyDirection, blockSize)))
196 break; 240​ break;
197 241​
198 if (!(rval = writeData(targetDevice, buffer, writeOffset + blockSize * blocksCopied * copyDirection))) 242​ if (!(rval = writeData(targetDevice, buffer, writeOffset + blockSize * blocksCopied * copyDirection)))
199 break; 243​ break;
200 244​
201 bytesWritten += buffer.size(); 245​ bytesWritten += buffer.size();
202 246​
203 if (++blocksCopied * 100 / blocksToCopy != percent) { 247​ if (++blocksCopied * 100 / blocksToCopy != percent) {
204 percent = blocksCopied * 100 / blocksToCopy; 248​ percent = blocksCopied * 100 / blocksToCopy;
205 249​
206 if (percent % 5 == 0 && t.elapsed() > 1000) { 250​ if (percent % 5 == 0 && t.elapsed() > 1000) {
207 const qint64 mibsPerSec = (blocksCopied * blockSize / 1024 / 1024) / (t.elapsed() / 1000); 251​ const qint64 mibsPerSec = (blocksCopied * blockSize / 1024 / 1024) / (t.elapsed() / 1000);
208 const qint64 estSecsLeft = (100 - percent) * t.elapsed() / percent / 1000; 252​ 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()); 253​
210 emit reportProgress(report); 254​ /*================Call App slot===================*/
255​ QDBusInterface *iface = new QDBusInterface(QStringLiteral("org.kde.kpmcore.applicationinterface"), QStringLiteral("/Application"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus());
256​
257​ if (!iface->isValid()) {
258​ return QVariantMap();
211 } 259​ }
212 emit progress(percent); 260​
261​ iface->setTimeout(10 * 24 * 3600 * 1000);
262​
263​ // We send zero percent new data on initial start-up
264​ QDBusPendingCall pcall = iface->asyncCall(QLatin1String("emitNewData"), xi18nc("@info:progress", "Copying %1 MiB/second, estimated time left: %2", mibsPerSec, QTime(0, 0).addSecs(estSecsLeft).toString()));
265​
266​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
267​ QEventLoop loop;
268​
269​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
270​ loop.exit();
271​
272​ if (watcher->isError())
273​ qWarning() << watcher->error();
274​ };
275​
276​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
277​ loop.exec();
278​ /*================End Call App slot===================*/
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…
279​
280​ }
281​ /*================Call App slot===================*/
282​ QDBusInterface *iface = new QDBusInterface(QStringLiteral("org.kde.kpmcore.applicationinterface"), QStringLiteral("/Application"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus());
283​
284​ if (!iface->isValid()) {
285​ return QVariantMap();
286​ }
287​
288​ iface->setTimeout(10 * 24 * 3600 * 1000);
289​
290​ // We send zero percent new data on initial start-up
291​ QDBusPendingCall pcall = iface->asyncCall(QLatin1String("emitNewData"), percent);
292​
293​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
294​ QEventLoop loop;
295​
296​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
297​ loop.exit();
298​
299​ if (watcher->isError())
300​ qWarning() << watcher->error();
301​ };
302​
303​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
304​ loop.exec();
305​ /*================End Call App slot===================*/
213 } 306​ }
214 } 307​ }
215 308​
216 // copy the remainder 309​ // copy the remainder
217 if (rval && lastBlock > 0) { 310​ if (rval && lastBlock > 0) {
218 Q_ASSERT(lastBlock < blockSize); 311​ Q_ASSERT(lastBlock < blockSize);
219 312​
220 const qint64 lastBlockReadOffset = copyDirection > 0 ? readOffset + blockSize * blocksCopied : sourceFirstByte; 313​ const qint64 lastBlockReadOffset = copyDirection > 0 ? readOffset + blockSize * blocksCopied : sourceFirstByte;
221 const qint64 lastBlockWriteOffset = copyDirection > 0 ? writeOffset + blockSize * blocksCopied : targetFirstByte; 314​ const qint64 lastBlockWriteOffset = copyDirection > 0 ? writeOffset + blockSize * blocksCopied : targetFirstByte;
222 315​
223 report[QStringLiteral("report")]= xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset); 316​ /*================Call App slot===================*/
317​ QDBusInterface *iface = new QDBusInterface(QStringLiteral("org.kde.kpmcore.applicationinterface"), QStringLiteral("/Application"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus());
318​
319​ if (!iface->isValid()) {
320​ return QVariantMap();
321​ }
322​
323​ iface->setTimeout(10 * 24 * 3600 * 1000);
324​
325​ // We send zero percent new data on initial start-up
326​ QDBusPendingCall pcall = iface->asyncCall(QLatin1String("emitNewData"), xi18nc("@info:progress", "Copying remainder of block size %1 from %2 to %3.", lastBlock, lastBlockReadOffset, lastBlockWriteOffset));
327​
328​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
329​ QEventLoop loop;
330​
331​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
332​ loop.exit();
333​
334​ if (watcher->isError())
335​ qWarning() << watcher->error();
336​ };
337​
338​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
339​ loop.exec();
340​
224 341​
225 emit reportProgress(report); 342​ /*================End Call App slot===================*/
226 343​
227 rval = readData(sourceDevice, buffer, lastBlockReadOffset, lastBlock); 344​ rval = readData(sourceDevice, buffer, lastBlockReadOffset, lastBlock);
228 345​
229 if (rval) { 346​ if (rval) {
230 if (targetDevice.isEmpty()) 347​ if (targetDevice.isEmpty())
231 reply[QStringLiteral("targetByteArray")] = buffer; 348​ reply[QStringLiteral("targetByteArray")] = buffer;
232 else 349​ else
233 rval = writeData(targetDevice, buffer, lastBlockWriteOffset); 350​ rval = writeData(targetDevice, buffer, lastBlockWriteOffset);
234 } 351​ }
235 352​
236 if (rval) { 353​ if (rval) {
237 emit progress(100); 354​ /*================Call App slot===================*/
355​ QDBusInterface *iface = new QDBusInterface(QStringLiteral("org.kde.kpmcore.applicationinterface"), QStringLiteral("/Application"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus());
356​
357​ if (!iface->isValid()) {
358​ return QVariantMap();
359​ }
360​
361​ iface->setTimeout(10 * 24 * 3600 * 1000);
362​
363​ // We send zero percent new data on initial start-up
364​ QDBusPendingCall pcall = iface->asyncCall(QLatin1String("emitNewData"), 100);
365​
366​ QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(pcall, this);
367​ QEventLoop loop;
368​
369​ auto exitLoop = [&] (QDBusPendingCallWatcher *watcher) {
370​ loop.exit();
371​
372​ if (watcher->isError())
373​ qWarning() << watcher->error();
374​ };
375​
376​ connect(watcher, &QDBusPendingCallWatcher::finished, exitLoop);
377​ loop.exec();
378​ /*================End Call App slot===================*/
379​
238 bytesWritten += buffer.size(); 380​ bytesWritten += buffer.size();
239 } 381​ }
240 } 382​ }
241 383​
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)); 384​ /*================Call App slot===================*/
385​ QDBusInterface *iface2 = new QDBusInterface(QStringLiteral("org.kde.kpmcore.applicationinterface"), QStringLiteral("/Application"), QStringLiteral("org.kde.kpmcore.externalcommand"), QDBusConnection::systemBus());
386​
387​ if (!iface2->isValid()) {
388​ return QVariantMap();
389​ }
390​
391​ iface2->setTimeout(10 * 24 * 3600 * 1000);
392​
393​ // We send zero percent new data on initial start-up
394​ QDBusPendingCall pcall2 = iface2->asyncCall(QLatin1String("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)));
395​
396​ QDBusPendingCallWatcher *watcher2 = new QDBusPendingCallWatcher(pcall2, this);
397​ QEventLoop loop2;
398​
399​ auto exitLoop2 = [&] (QDBusPendingCallWatcher *watcher2) {
400​ loop2.exit();
401​
402​ if (watcher2->isError())
403​ qWarning() << watcher2->error();
404​ };
243 405​
244 emit reportProgress(report); 406​ connect(watcher2, &QDBusPendingCallWatcher::finished, exitLoop2);
407​ loop2.exec();
408​ /*================End Call App slot===================*/
245 409​
246 reply[QStringLiteral("success")] = rval; 410​ reply[QStringLiteral("success")] = rval;
247 return reply; 411​ return reply;
248} 412​}
249 413​
250bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte) 414​bool ExternalCommandHelper::writeData(const QByteArray& buffer, const QString& targetDevice, const qint64 targetFirstByte)
251{ 415​{
252 // Do not allow using this helper for writing to arbitrary location 416​ // Do not allow using this helper for writing to arbitrary location
▲ Show 20 Lines • Show All 69 Lines • Show Last 20 Lines