Created attachment 87602 [details] patch for kwindowinfo_mac.cpp After building and installing rekonq under OS X/MacPorts, I quickly ran into a systematic crash each time I accepted the settings dialog after making a change. A little debugging showed that the snippet Q_FOREACH(const QWeakPointer<RekonqWindow> &pointer, wList) { if (KWindowInfo(pointer.data()->effectiveWinId(), NET::WMDesktop, 0).isOnCurrentDesktop()) return pointer.data(); } will lead to calling CFRelease with a NULL argument in KWindowInfo::Private::~Private(). Cocoa allows sending a message (like [obj release]) to a nil object, but the CoreFoundation version CFRelease will crash when passed a NULL pointer. This is confirmed by the API documentation. Attached is a very simple patch that adds the required check before calling CFRelease - as is done all other times that function is called. The patch was made with KDE 4.12.5 but should apply to the current and development versions too given that kwindowinfo_mac.cpp hasn't been touched in over a year (https://projects.kde.org/projects/kde/kdelibs/repository/revisions/master/changes/kdeui/windowmanagement/kwindowinfo_mac.cpp).
See also issue 334827!
*** Bug 334827 has been marked as a duplicate of this bug. ***
A review request would probably make more sense.
(In reply to comment #3) > A review request would probably make more sense. For a very evident bug and equally straightforward patch?? Online CFRelease documentation: https://developer.apple.com/library/mac/documentation/CoreFoundation/Reference/CFTypeRef/Reference/reference.html#//apple_ref/c/func/CFRelease "Special Considerations If cf is NULL, this will cause a runtime error and your application will crash. "
yes a review request is needed for the very obvious reason that it will be lost on bugzilla. You see I did not open the bug report because I saw that it is about MacOS and cannot do anything there so wouldn't spot that there is a patch attached.
Frankly, then what's the sense about having a bug tracker?? And why would someone who "cannot do anything" on OS X get involved with a patch that's only relevant to that OS? Anyway, I just tried to submit a review request for patches against 4.12.5 that improve OS integration, and found they were refused because not git diff patches. Tell me how to submit the patches as is and I'll submit the request.
(In reply to comment #6) > Frankly, then what's the sense about having a bug tracker?? the bug tracker is for bugs, not for patches - at least that's the way we work here in KDE. > And why would > someone who "cannot do anything" on OS X get involved with a patch that's > only relevant to that OS? because I'm the maintainer of the framework and are supposed to ACK the change. > > Anyway, I just tried to submit a review request for patches against 4.12.5 > that improve OS integration, and found they were refused because not git > diff patches. Tell me how to submit the patches as is and I'll submit the > request. Unfortunately it needs to be git formatted. That's also needed for pushing - after all we want to keep the author information.
Well, this *is* a bug report. You're free to disregard the patch and simply correct the bug going by the documents and other invocations of CFRelease. Maybe that'll be quicker than me finding time and motivation to figure out how to share a patch that already took me enough time (and for which I don't care the least to be the recognised author) :)
because reviewboard is better suited to announce (and review) patches. problem on the "straightforward" aspect is that most devs here (inc. me) do not operate on OSX - iff there is any strategy on dealing with CF, only one or two ppl. might know and they'll more likely look at a patch presented for review than at some random, pot. module related, bug. Situation is similar w/ eg convert_CFString_to_QString(CFStringRef str) which segfaults if you dump in a nullptr "str" what at least klocale (mac) seems prone to because the apparently CFStringRef( CFLocaleGetValue( m_macLocale, key ) ); can return one. See http://lists.kde.org/?t=139514981600001&r=1&w=2 -> If OSX is a target system for KDE, we severely lack developers, so if you're interested in KDE on OSX and apparently have coding skills, it would be *really* great if you would seek for commit rights and get in touch with other OSX using developers. There is certainly ambition, but also a severe lack of manpower.
> -> If OSX is a target system for KDE, we severely lack developers, so if you're interested in KDE > on OSX and apparently have coding skills, it would be *really* great if you would seek for > commit rights and get in touch with other OSX using developers. Not that I am particularly fluent in C++ or familiar with Qt/KDE frameworks, but I certainly wouldn't mind to obtain commit rights and contribute at my own pace (i.e. not necessarily spearheading development of the latest and greatest with all the requirements that come with it).
René (RJVB) is already part of our group at KDE Software on Mac OS X <kde-mac@kde.org> and a frequent contributor on the MacPorts lists. Please accept his patch. IMHO, a patch is a patch, regardless of how it is submitted or from where - even if it is written on old newspaper "with a thumbnail dipped in tar" and it washes up on the seashore in a bottle... :-) Our only concern should be if it is valid, properly written, low risk and works. And if a user reports a bug and submits a good patch, that is treasure indeed - or it used to be in the old days of KDE. This patch can only affect the Mac OS X version of KDE 4. Martin, if you need any tests done on an Apple OS X platform, please let Marko (Comment 1) or me know. Re git-format patches, not every programmer in the world has or understands git. I usually receive a patch like the attached by cd'ing to the source code in my local repository, using the "patch" utility and then using "git diff" to get it in git format, or even "git checkout" to get the source files back as they were. Is it that hard?
So, I filed the patch at RB. https://git.reviewboard.kde.org/r/119240/
Git commit d32b52996ce48d484bbf54a62321872a482e8d74 by Marko Käning. Committed on 14/07/2014 at 22:09. Pushed by kaning into branch 'KDE/4.13'. Preventing a crash in the KWindowInfo::Private destructor on OSX René J.V. Bertin found that Apple's developer documentation for CFRelease mentions that its argument may not be a NULL pointer. M +3 -1 kdeui/windowmanagement/kwindowinfo_mac.cpp http://commits.kde.org/kdelibs/d32b52996ce48d484bbf54a62321872a482e8d74
Git commit 10398255165afda1a0ecddd1671bbb599846bb35 by Marko Käning. Committed on 15/07/2014 at 22:51. Pushed by kaning into branch 'master'. Preventing a crash in the KWindowInfo::Private destructor on OSX René J.V. Bertin found that Apple's developer documentation for CFRelease mentions that its argument may not be a NULL pointer. CCMAIL: kde-frameworks-devel@kde.org REVIEW: 119240 M +3 -1 src/kwindowinfo_mac.cpp http://commits.kde.org/kwindowsystem/10398255165afda1a0ecddd1671bbb599846bb35