[Kde-pim] Review Request: avoid the use of an undefined collection

[Guy Maurel]

Hello Kevin!

I'll try again to precise what I'm meaning...
On Monday, October 03, 2011 08:12:52 PM Kevin Krammer wrote:
> On Monday, 2011-10-03, Guy Maurel wrote:
> > > On Oct. 3, 2011, 4:56 p.m., Kevin Krammer wrote:
> > > > I am not sure what this will achieve. The current code makes the
> > > > DefaultResourceJob result in an error. Wouldn't your change make it
> > > > claim it succeeded?
> > 
> > Let me introduce a zero-mail-account.
> > This account is created on another system, let say Ubuntu, with kmail1.
> > A POP3-server is well defined, not any mail where got, not any new folder.
> 
> The POP resource does not have any folder, but the maildir resource it 
> downloads to should.
> 
> What resource type is the default resource job working on in your case?
> 
> > With such a account, the akonadi_maildispatcher_agent crashes every time.
> > 
> > The current code is not correct, at the place I noticed.
> 
> The code in DefaultResourceJobPrivate looks correct.
NO, is my answer! Not the part at
  DefaultResourceJobPrivate::collectionFetchResult( KJob *job )

Even if one doesn't know what the code is doing, it is NOT correct.
One may not use a variable which has not been assigned *before*.

> Do you mean the place from where it is called?
I mean the lines
  foreach ( const Collection &collection, collections ) {
    ...
and   
 if(  !resourceCollection.isValid() ) {
because the loop is not entered, the variable resourceCollection is NOT set.
We don't know what are the values of the memory. The code uses these values of
resourceCollection to make a call "isValid". I think, the result is NOT defined, and
might be different from one computer to another.

> > With my proposal, the agent doesn't crash any more. It might be not
> > suffisant enough, but I need some help from the coding guy.
> 
> Sure, which is why I try to understand the problem :)
I think that the problem isn't to know from where the method is called from.
More, the question is (I hope you could give me an answer):
WHAT is to do when the job has NO collection?
This is the fact I observe, and the line
  kDebug() << "Fetched" << collections.count() << "collections.";
informs us about that.

Looking once more on the code I would to ask:
What happens in the loop foreach, if the if-statement returns "false"?
The lines under
  // Find all children of the resource collection.
are using the variables resourceCollection and toRecover whitout any check.
Same at
  // These collections have been created by the maildir resource, when it
  // found the folders on disk. So give them the necessary attributes now.

Nothing take care IF there where NO collection!

My proposal with "return" might be too short, I know it!
What is better?
-- 
guy
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/