Re: Review Request: Don't hang when determining MIME type of corrupted files
On Monday 22 August 2011 20:12:13 Miroslav Ľos wrote: 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). The most common case for mimetype determination is from the file manager listing local files, in which case findByUrl will use a QFile for content- determination. Ah, and BTW I have just found a magic rule that needs 4K of data: vmware-player.xml says: match type=string value='config.version = ' offset=0:4096/ 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. I'm not sure what you mean there. The unnecessary 16K read did hit several more EIO's on the broken CD I have, but it is just cosmetic I guess. Yes I'm not sure it's worth optimizing for this special case. Nonetheless, I find most uses of these functions ever will be through QFile or QBuffer Sure. It's QFile that I have in mind here, when I say that seeking back and forth will be slow, in unbuffered mode. Finally, I wonder if buffering makes a difference as we are only using a few small block reads, not many getChar()'s. It's about seeking and reading, vs just having the data in memory. * 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. Right. Small bug, but a corner case (only matters if extension unknown). -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
Re: Review Request: GSoC: Errors handling during file transfer.
On Wednesday 24 August 2011 16:24:05 Cyril Oblikov wrote: I wrote: But did you at least run jobtest (in kdelibs/kio/tests) to make sure that the new code doesn't break it? E.g. the creation of a dialog for sure broke it, but now you've fixed that bit. No, I didn't use tests. Are there any docs about kdelibs testing? Two solutions: * to compile and run a single test, just cd into kdelibs/kio/tests; make jobtest ; ./jobtest * to always compile all unit tests (recommended, but takes a bit more time during compilation) pass -DKDE4_BUILD_TESTS=TRUE to cmake. To run all these tests, make test. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).
Re: playground-libs/libkvkontakte has moved to kdereview
2011/8/9 Alexander Potashev aspotas...@gmail.com: playground-libs/libkvkontakte moved to kdereview today. The next target for this project is extragear/libs. libkvkontakte has been in kdereview for more than two weeks already. Is it OK to move it into extragear-libs now? -- Alexander Potashev
Re: Re: playground-libs/libkvkontakte has moved to kdereview
A Dijous, 25 d'agost de 2011, Alexander Potashev vàreu escriure: 2011/8/9 Alexander Potashev aspotas...@gmail.com: playground-libs/libkvkontakte moved to kdereview today. The next target for this project is extragear/libs. libkvkontakte has been in kdereview for more than two weeks already. Is it OK to move it into extragear-libs now? I thought you were going to get rid of the private members and use a d-pointer instead? Albert -- Alexander Potashev
Re: Re: playground-libs/libkvkontakte has moved to kdereview
2011/8/25 Albert Astals Cid aa...@kde.org: I thought you were going to get rid of the private members and use a d-pointer instead? What is the point of this? I think it will be OK to keep all class members in the main (public) classes and use d-ptr only in case of real necessity. -- Alexander Potashev
Re: Re: Re: playground-libs/libkvkontakte has moved to kdereview
A Dijous, 25 d'agost de 2011, Alexander Potashev vàreu escriure: 2011/8/25 Albert Astals Cid aa...@kde.org: I thought you were going to get rid of the private members and use a d-pointer instead? What is the point of this? I think it will be OK to keep all class members in the main (public) classes and use d-ptr only in case of real necessity. The point is that usually you do not know what the library will end up doing and by using d-pointers everywhere you make it easier for yourself to maintain binary compatibility in the future. But this is of course something you as maintainer of the library have to decide if it is worth or not. Albert -- Alexander Potashev
Re: Re: Re: playground-libs/libkvkontakte has moved to kdereview
2011/8/25 Albert Astals Cid aa...@kde.org: The point is that usually you do not know what the library will end up doing and by using d-pointers everywhere you make it easier for yourself to maintain binary compatibility in the future. But in the case that most classes won't grow in the future by using my strategy (keeping d-ptr NULL when possible) we cut down the number of memory allocations, and also simplify the existing code that doesn't work with the private class' data. So, I'm going to keep following the current strategy. -- Alexander Potashev
Re: Review Request: Fix KUrlCompletionPrivate::urlCompletion's behavior
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102425/#review6007 --- This review has been submitted with commit 67b93998237a09aa2166a6c9554b7e3a3aa02921 by Dawit Alemayehu to branch frameworks. - Commit On Aug. 25, 2011, 3:55 a.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102425/ --- (Updated Aug. 25, 2011, 3:55 a.m.) Review request for kdelibs and David Faure. Summary --- The attached patch fixes the logic problems in KUrlCompletionPrivate::urlCompletion that cause it to incorrectly attempt remote directory name completions. With this patch simply entering sftp:/ or ftp:/ into a line edit that uses KUrlCompletion will not result in a completion attempt. Additionally the new code should be much more clearer than the previous one. Diffs - kio/kio/kurlcompletion.cpp 18ea90a Diff: http://git.reviewboard.kde.org/r/102425/diff Testing --- Validated with unit test and tested using Konqueror's location bar. Thanks, Dawit
Re: kdeinit
Something I'm going to do, and I hope that some of the metalworkers will follow is to get involved in the new platform (uStuff, NM, BlueZ...), only by winning our relevance we will be able to control the platform again. Also, I'd like to point something: In number of people I think that KDE has way more than GNOME BUT they have paid 8h/day people that's where the huge difference is.
Review Request: Allow externally deleted files to be removed from view in Dolphin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102435/ --- Review request for KDE Base Apps and Peter Penz. Summary --- Currently, if a file is deleted by an external application, and the directory containing that file is open in Dolphin, the change is not reflected in Dolphin until we refresh. This patch fixes it. Diffs - dolphin/src/kitemviews/kfileitemmodel.cpp 189aa75 Diff: http://git.reviewboard.kde.org/r/102435/diff Testing --- Yes, tested locally. Thanks, Tirtha
Review Request: Support GnuPG2 in KNewstuff3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102439/ --- Review request for kdelibs. Summary --- So far knewstuff has gpg hardcoded as the executable name for GnuPG. This patch also allows the use of gpg2 instead, if the gpg executable is not present (both are compatible as far as we are concerned here). Since kdepim* requires GnuPG2, you are most likely using gpg2 already anyway, via backward compatibility forwards. On MeeGo however this is not available and packages for GnuPG1 and 2 are not installable at the same time. Diffs - knewstuff/knewstuff3/core/security.cpp 5654787 Diff: http://git.reviewboard.kde.org/r/102439/diff Testing --- Thanks, Volker
Re: Review Request: Support GnuPG2 in KNewstuff3
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102439/#review6018 --- This review has been submitted with commit 9eb9f85698a3c0283f8e50b1a904b5b53d5f80ab by Volker Krause to branch KDE/4.7. - Commit On Aug. 25, 2011, 9:02 p.m., Volker Krause wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102439/ --- (Updated Aug. 25, 2011, 9:02 p.m.) Review request for kdelibs. Summary --- So far knewstuff has gpg hardcoded as the executable name for GnuPG. This patch also allows the use of gpg2 instead, if the gpg executable is not present (both are compatible as far as we are concerned here). Since kdepim* requires GnuPG2, you are most likely using gpg2 already anyway, via backward compatibility forwards. On MeeGo however this is not available and packages for GnuPG1 and 2 are not installable at the same time. Diffs - knewstuff/knewstuff3/core/security.cpp 5654787 Diff: http://git.reviewboard.kde.org/r/102439/diff Testing --- Thanks, Volker
