[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: Fix for kfilewidget.cpp
From:       "David Boosalis" <david.boosalis () gmail ! com>
Date:       2008-10-17 16:39:31
Message-ID: 870c99310810170939t545adee5q6545ee702c78fef4 () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


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 <<
>

[Attachment #5 (text/html)]

<div dir="ltr">Thanks for pointing me to the write email group Andreas.&nbsp; I did \
as you suggested and did a complete and complete build of koffice.&nbsp; \
Unfortunately the results are the same -&nbsp; core dumps.<br><br>Turning to coding \
philosophies I think the checks&nbsp; for null pointers in this case is \
desirable.&nbsp;&nbsp; In this case you pointed out that it is impossible for the \
pointers&nbsp; to be null and cause the crash, and poor programming caused \
crash.&nbsp; 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. <br> <br>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&#39;s experience is put first (no core \
dumps).&nbsp; In this case KFileWidget is not created that often, and it&#39;s \
showEvent() method is seldom,called&nbsp; so some simple checks and simple warning \
messages cost little. (A paintEvent() method might be a different story).&nbsp; <br> \
<br>This is my opionion, don&#39;t mean to ruffle any feathers.&nbsp; I&#39;ll see \
about subscribing to the core group and posting there.<br><br>-David \
Boosalis<br><br><div class="gmail_quote">On Fri, Oct 17, 2008 at 2:56 AM, Andreas \
Pakulat <span dir="ltr">&lt;<a href="mailto:apaku@gmx.de">apaku@gmx.de</a>&gt;</span> \
wrote:<br> <blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, \
204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">On 16.10.08 21:39:12, David \
Boosalis wrote:<br> &gt; HI KKDE maintainers.<br>
<br>
Slightly wrong mailinglist, kde-core-devel is better for code that lives in<br>
kdelibs.<br>
<div class="Ih2E3d"><br>
&gt; The change that fixes is to apply checks for null pointers in<br>
&gt; KFileWidget::showEvent(). &nbsp;I don&#39;t know what these pointers are, and \
perhaps<br> &gt; it is an error in koffice applications, but a simple check as shown \
below<br> &gt; does catch the error and allow kword to come up.<br>
<br>
</div>Well, maybe the author of the code expects those member to be always<br>
non-null, which means that there&#39;s an error elsewhere that needs to be<br>
found and fixed.<br>
<br>
Looking through the code in kfilewidget, neither the d member nor the<br>
d-&gt;ops member can possibly be null. Both are created inside the<br>
constructore of KFileWidget and both are deleted inside the destructor of<br>
KFileWidget/KFileWidgetPrivate and nowhere else. So the only option left is<br>
that koffice uses KFileWidget in a wrong way, or your builds are wrong<br>
(meaning should remove install and builddir and start building from<br>
scratch).<br>
<br>
Andreas<br>
<font color="#888888"><br>
--<br>
Truth will out this morning. &nbsp;(Which may really mess things up.)<br>
<br>
&gt;&gt; Visit <a href="http://mail.kde.org/mailman/listinfo/kde-devel#unsub" \
target="_blank">http://mail.kde.org/mailman/listinfo/kde-devel#unsub</a> to \
unsubscribe &lt;&lt;<br> </font></blockquote></div><br></div>



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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic