D23194: Making FileJob behave consistently.

[Chinmoy Ranjan Pradhan]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190816/b4a5c846/attachment.html>