----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106902/#review20459 -----------------------------------------------------------
kio/kio/kfileitem.cpp <http://git.reviewboard.kde.org/r/106902/#comment16158> I was actually thinking about url.isEmpty(), but probably got confused by your solution :) - Ignat Semenov On Oct. 16, 2012, 4:15 p.m., Ignat Semenov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106902/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2012, 4:15 p.m.) > > > Review request for kdelibs and David Faure. > > > Description > ------- > > Check the d pointer for validity since it is set to 0 by the defaut KFileItem > ctor. > > Patch based on similar code from QImage. > > I'd like to draw your attention to these issues: > > 1) KFileItem::setUDSEntry() > For a null KFileItem, this should probably create a new d and initialize it. > Ideally, we'd have a dedicated public function to do that (see > QImage::create()), but since we can not change the public API (or can we?), I > put the new in setUDSEntry(). Anyway, this is now the only interface function > that can init a null KFileItem (before the patch, there had been none). All > the other set* functions do not new the d, but rather return on d == 0. > 2) KFileItem::cmp() > What should this return for a null KFileItem? I return false, but still, it > feels wrong to me. > 3) operator << / operator >> > << returns "null KFileItem" for d == 0 and >> does nothing, returning the > original stream unmodified. operator << thus duplicates the public operator > <<(QDebug&). > 4) This also overrides dfaure's patch for KFileItem::metaInfo(). There is a > > return d->metaInfo() > > at the end of the function, so this can crash as well. > > All the classes' objects that are default-constructed and then returned in > the case of d == 0 can be checked for validity, looked it up in the code. > > > This addresses bug 299726. > http://bugs.kde.org/show_bug.cgi?id=299726 > > > Diffs > ----- > > kio/kio/kfileitem.cpp 8ab5a1d > > Diff: http://git.reviewboard.kde.org/r/106902/diff/ > > > Testing > ------- > > Builds fine. > > > Thanks, > > Ignat Semenov > >
