> On Aug. 21, 2011, 10:07 a.m., David Faure wrote:
> > Thanks Peter and Miroslav. The analysis looks correct, the pre-read part of 
> > the patch looks good. I'm just wondering about using Unbuffered. If someone 
> > installs a mimetype definition with multiple rules trying to match some 
> > bytes after the 2K limit, then all this seeking-and-reading back and forth 
> > will be very slow, in unbuffered mode (since neither cache will be used).

I find Unbuffered causing slowness improbable. From what I've seen in uses of 
KMimeType::findBy\w*Content in kdelibs, they all (e.g. ftp* and http kioslaves) 
rather provide their own buffer in a QByteArray rather than a QIODevice; most 
just provide the path (which is only opened using QFile if it is_local_file).

All QFile's buffering is implemented in the QIODevice superclass, it adds 
Unbuffered in open() to its openMode for its fileEngine() backend. Thus, no 
buffering is propagated down. The unnecessary 16K read did hit several more 
EIO's on the broken CD I have, but it is just cosmetic I guess.

Nonetheless, I find most uses of these functions ever will be through QFile or 
QBuffer and any client passing another QIODevice may open it itself. Maybe a 
note in the documentation could help them do it if necessary.

Finally, I wonder if buffering makes a difference as we are only using a few 
small block reads, not many getChar()'s. Then again, the documentation to 
QTcpSocket says they cannot be opened (i.e. ignore I guess) Unbuffered, which 
is what any other QIODevices may do as well.

* ftp passes a 1K-capped buffer (kioslave/ftp/ftp.cpp:2471). That may be 
insufficient for the single rule on my system that needed 1029 bytes.


- Miroslav


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102391/#review5872
-----------------------------------------------------------


On Aug. 20, 2011, 5:21 p.m., Peter Penz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102391/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2011, 5:21 p.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> -------
> 
> If KMimeTypeRepository::findFromContent() tries to determine MIME from a file 
> that cannot be read, such as on a corrupted optical disc, a read attempt is 
> made in KMimeMagicMatch::match() for every available rule, resulting in UI 
> hangs (e.g. file dialogs, dolphin) for tens of minutes (see 
> https://bugs.kde.org/show_bug.cgi?id=280446 for more details).
> 
> I've submitted this patch here on behalf of Miroslav ?os, who has submitted 
> the bug-report and also has written the patch.
> 
> 
> This addresses bug 280446.
>     http://bugs.kde.org/show_bug.cgi?id=280446
> 
> 
> Diffs
> -----
> 
>   kdecore/services/kmimetype.cpp 955bf62 
>   kdecore/services/kmimetyperepository.cpp 6ff3d16 
> 
> Diff: http://git.reviewboard.kde.org/r/102391/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Peter
> 
>

Reply via email to