From kde-devel Fri Oct 17 16:39:31 2008 From: "David Boosalis" Date: Fri, 17 Oct 2008 16:39:31 +0000 To: kde-devel Subject: Re: Fix for kfilewidget.cpp Message-Id: <870c99310810170939t545adee5q6545ee702c78fef4 () mail ! gmail ! com> X-MARC-Message: https://marc.info/?l=kde-devel&m=122426170103881 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1952825439==" --===============1952825439== Content-Type: multipart/alternative; boundary="----=_Part_55136_7752061.1224261571603" ------=_Part_55136_7752061.1224261571603 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline Thanks for pointing me to the write email group Andreas. I did as you suggested and did a complete and complete build of koffice. Unfortunately the results are the same - core dumps. Turning to coding philosophies I think the checks for null pointers in this case is desirable. In this case you pointed out that it is impossible for the pointers to be null and cause the crash, and poor programming caused crash. While this maybe the case, if a simple check for null pointers was done it would cover these corner cases cause by bad programming elsewhere and allows the application (kword) to run just fine. I am not saying we should promote bad programming, but rather utilize good progaming such as checking for nulls where ever practical, and thus making sure the user's experience is put first (no core dumps). In this case KFileWidget is not created that often, and it's showEvent() method is seldom,called so some simple checks and simple warning messages cost little. (A paintEvent() method might be a different story). This is my opionion, don't mean to ruffle any feathers. I'll see about subscribing to the core group and posting there. -David Boosalis On Fri, Oct 17, 2008 at 2:56 AM, Andreas Pakulat wrote: > On 16.10.08 21:39:12, David Boosalis wrote: > > HI KKDE maintainers. > > Slightly wrong mailinglist, kde-core-devel is better for code that lives in > kdelibs. > > > The change that fixes is to apply checks for null pointers in > > KFileWidget::showEvent(). I don't know what these pointers are, and > perhaps > > it is an error in koffice applications, but a simple check as shown below > > does catch the error and allow kword to come up. > > Well, maybe the author of the code expects those member to be always > non-null, which means that there's an error elsewhere that needs to be > found and fixed. > > Looking through the code in kfilewidget, neither the d member nor the > d->ops member can possibly be null. Both are created inside the > constructore of KFileWidget and both are deleted inside the destructor of > KFileWidget/KFileWidgetPrivate and nowhere else. So the only option left is > that koffice uses KFileWidget in a wrong way, or your builds are wrong > (meaning should remove install and builddir and start building from > scratch). > > Andreas > > -- > Truth will out this morning. (Which may really mess things up.) > > >> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to > unsubscribe << > ------=_Part_55136_7752061.1224261571603 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline
Thanks for pointing me to the write email group Andreas.  I did as you suggested and did a complete and complete build of koffice.  Unfortunately the results are the same -  core dumps.

Turning to coding philosophies I think the checks  for null pointers in this case is desirable.   In this case you pointed out that it is impossible for the pointers  to be null and cause the crash, and poor programming caused crash.  While this maybe the case, if a simple check for null pointers was done it would cover these corner cases cause by bad programming elsewhere and allows the application (kword) to run just fine.

I am not saying we should promote bad programming, but rather utilize good progaming such as checking for nulls where ever practical, and thus making sure the user's experience is put first (no core dumps).  In this case KFileWidget is not created that often, and it's showEvent() method is seldom,called  so some simple checks and simple warning messages cost little. (A paintEvent() method might be a different story). 

This is my opionion, don't mean to ruffle any feathers.  I'll see about subscribing to the core group and posting there.

-David Boosalis

On Fri, Oct 17, 2008 at 2:56 AM, Andreas Pakulat <apaku@gmx.de> wrote:
On 16.10.08 21:39:12, David Boosalis wrote:
> HI KKDE maintainers.

Slightly wrong mailinglist, kde-core-devel is better for code that lives in
kdelibs.

> The change that fixes is to apply checks for null pointers in
> KFileWidget::showEvent().  I don't know what these pointers are, and perhaps
> it is an error in koffice applications, but a simple check as shown below
> does catch the error and allow kword to come up.

Well, maybe the author of the code expects those member to be always
non-null, which means that there's an error elsewhere that needs to be
found and fixed.

Looking through the code in kfilewidget, neither the d member nor the
d->ops member can possibly be null. Both are created inside the
constructore of KFileWidget and both are deleted inside the destructor of
KFileWidget/KFileWidgetPrivate and nowhere else. So the only option left is
that koffice uses KFileWidget in a wrong way, or your builds are wrong
(meaning should remove install and builddir and start building from
scratch).

Andreas

--
Truth will out this morning.  (Which may really mess things up.)

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

------=_Part_55136_7752061.1224261571603-- --===============1952825439== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline >> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe << --===============1952825439==--