chinmoyr requested changes to this revision.
chinmoyr added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> filejob.cpp:181
> + slaveDone();
> +// Scheduler::doJob(this);
> + q->emitResult();
Nitpick; spacing gotcha
> filejob.h:48
> /**
> - * Read block
> + * This function attempts to read up to \p size bytes from the URL
> passed to
> + * KIO::open() and returns the bytes received via the data() signal.
Document the methods in SlaveBase as well.
> slavebase.h:107
> + * @see close()
> + */
> + void closed();
@since 5.62
> slaveinterface.h:90
> + MSG_SLAVE_STATUS_V2,
> + MSG_CLOSED
> // add new ones here once a release is done, to avoid breaking binary
> compatibility
Nitpick; add a ,
Next time someone adds an enum it will change only one line.
> file.cpp:520
>
> - if (!res.isEmpty()) {
> - data(res);
> - bytes -= res.size();
> - } else {
> - // empty array designates eof
> - data(QByteArray());
> - if (!mFile->atEnd()) {
> - error(KIO::ERR_CANNOT_READ, mFile->fileName());
> - close();
> - }
> - break;
> - }
> - if (bytes <= 0) {
> - break;
> - }
> + qint64 bytesRead = mFile->read(buffer.data(), bytes);
> +
A loop is required here. The docs don't really specify anything about reading
data in one go.
> file.cpp:584
> + closeWithoutFinish();
> + closed();
> }
missing a finished() here
> file.h:117
> + // Close without calling finish(). Use this to close after error.
> + void closeWithoutFinish();
> +
Nitpick; why not just closeFile() ?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D23194
To: feverfew, dfaure, fvogt, chinmoyr, apol
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns
