Re: Review Request 111916: Port khtml_part away from kde_file.h
On Aug. 13, 2013, 7:29 a.m., Kevin Ottens wrote: Ship It! Vishesh, anything preventing you to push this patch? - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37643 --- On Aug. 12, 2013, 10:15 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 19, 2013, 9:32 a.m.) Review request for KDE Frameworks. Changes --- Use QFileInfo::symLinkTarget() instead of relying on readlink. Description --- Port khtml_part away from kde_file.h Diffs (updated) - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review38117 --- Ship it! Ship It! - Kevin Ottens On Aug. 19, 2013, 9:32 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 19, 2013, 9:32 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review38118 --- Ship it! Ship It! - David Faure On Aug. 19, 2013, 9:32 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 19, 2013, 9:32 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review38139 --- This review has been submitted with commit 499b52c5d6f169d9d78f6aff63d820f2ca0a1e7e by Vishesh Handa to branch frameworks. - Commit Hook On Aug. 19, 2013, 9:32 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 19, 2013, 9:32 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 19, 2013, 2:05 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37643 --- Ship it! Ship It! - Kevin Ottens On Aug. 12, 2013, 10:15 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
On Aug. 12, 2013, 2:43 p.m., Kevin Ottens wrote: khtml/khtml_part.cpp, line 3590 http://git.reviewboard.kde.org/r/111916/diff/3/?file=178130#file178130line3590 For extra safety I'd keep the ok here. David Faure wrote: No, you want to enter this branch for broken symlinks too. Ah I see. OK then. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37584 --- On Aug. 12, 2013, 10:15 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
On Aug. 12, 2013, 9:40 a.m., David Faure wrote: khtml/khtml_part.cpp, line 3580 http://git.reviewboard.kde.org/r/111916/diff/2/?file=178128#file178128line3580 Is this variable still used? Yes. A couple of lines below - int n = readlink ( path.toLocal8Bit().data(), buff_two, 1022); I can replace it over here if you want? On Aug. 12, 2013, 9:40 a.m., David Faure wrote: khtml/khtml_part.cpp, line 3585 http://git.reviewboard.kde.org/r/111916/diff/2/?file=178128#file178128line3585 That's not what lstat does The case where stat() fails and lstat() succeeds is the case of a broken symlink. In that case, QFileInfo::exists() returns false. I guess all we can do then is skip this second check, and use if (info.isSymlink()) below, without ok in front. I.e. always go into the symlink code, whether the symlink is valid or broken. It's slightly confusing that a file can not exist, but can still be a system link. - Vishesh --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37565 --- On Aug. 12, 2013, 9:35 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 9:35 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Changes --- Fixed problems Description --- Port khtml_part away from kde_file.h Diffs (updated) - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37584 --- khtml/khtml_part.cpp http://git.reviewboard.kde.org/r/111916/#comment27773 For extra safety I'd keep the ok here. - Kevin Ottens On Aug. 12, 2013, 10:15 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
On Aug. 12, 2013, 9:40 a.m., David Faure wrote: khtml/khtml_part.cpp, line 3580 http://git.reviewboard.kde.org/r/111916/diff/2/?file=178128#file178128line3580 Is this variable still used? Vishesh Handa wrote: Yes. A couple of lines below - int n = readlink ( path.toLocal8Bit().data(), buff_two, 1022); I can replace it over here if you want? That's what should use QFileInfo::symlinkTarget()... - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37565 --- On Aug. 12, 2013, 10:15 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
On Aug. 12, 2013, 2:43 p.m., Kevin Ottens wrote: khtml/khtml_part.cpp, line 3590 http://git.reviewboard.kde.org/r/111916/diff/3/?file=178130#file178130line3590 For extra safety I'd keep the ok here. No, you want to enter this branch for broken symlinks too. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37584 --- On Aug. 12, 2013, 10:15 a.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 12, 2013, 10:15 a.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp d944a29 Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
On Aug. 7, 2013, 4:12 p.m., David Faure wrote: This code should be made portable (to Windows), please use QFileInfo. Kevin Ottens wrote: As far as I know the QT_* macros would work on windows too. Using QFileInfo would be way nicer though. Depends on the *. win32-msvc2005/qplatformdefs.h:76:#define QT_STAT ::_stat but there's no QT_LSTAT. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37288 --- On Aug. 6, 2013, 6:14 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 6, 2013, 6:14 p.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp 189a98e Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
On Aug. 7, 2013, 4:12 p.m., David Faure wrote: This code should be made portable (to Windows), please use QFileInfo. As far as I know the QT_* macros would work on windows too. Using QFileInfo would be way nicer though. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37288 --- On Aug. 6, 2013, 6:14 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 6, 2013, 6:14 p.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp 189a98e Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/#review37288 --- This code should be made portable (to Windows), please use QFileInfo. - David Faure On Aug. 6, 2013, 6:14 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- (Updated Aug. 6, 2013, 6:14 p.m.) Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp 189a98e Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 111916: Port khtml_part away from kde_file.h
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111916/ --- Review request for KDE Frameworks. Description --- Port khtml_part away from kde_file.h Diffs - khtml/khtml_part.cpp 189a98e Diff: http://git.reviewboard.kde.org/r/111916/diff/ Testing --- Compiles. The tests seem to segfault with and without this patch. I'll try to diagnose it. Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
