Review: Needs Information

OK, Let's have a review on it,

1. perhaps the slot onParentDestroyed() on location.h lines 94-96 is not being 
used.

2. on line 38 from networklistworker.cpp we should check if m_parent != 0, 
   the default value for 'parent' is 0 on networklistworker.h

   38: parent->connectDestructionSignal(this, SLOT(onParentDestroyed()));

3. are you sure the code from networklistworker.cpp lines 57-58 are really 
necessary?
   the default value for 'parent' is 0 on networklistworker.h, and, if 
onParentDestroyed() is called  
   m_parent is set to 0 and the "m_parent != 0" is checked on line 61.

   57: if (!m_parent)
   58:       return netContent;
  
   61: bool is_parent_of_smb_url = m_parent != 0 && 
m_parent->urlPath().startsWith(LocationUrl::SmbURL);
   
   if you want keep lines 57-58, please change to "if (m_parent != 0)" as 
m_parent is not a boolean type.

4. can you explain why you removed the call to "refreshInfo()" from 
Location::currentInfo()
   on location.cpp line 329? 

   I see that both Location::refreshInfo() and Location::becomeParent() delete 
the 'm_info', 
   this is the 'parent' used in NetworkListWorker class on the second thread. 
   That may be the real problem, perhaps just making an attribution into 
'm_info' 
   from the temporary item could solve the entire problem, see a grep output:

     ./src/plugin/folderlistmodel/trash/trashlocation.cpp:57:                
delete m_info;
     ./src/plugin/folderlistmodel/trash/trashlocation.cpp:176:        delete 
m_info;
     ./src/plugin/folderlistmodel/location.cpp:64:        delete m_info;
     ./src/plugin/folderlistmodel/location.cpp:96:        delete m_info;
     ./src/plugin/folderlistmodel/location.cpp:238:        delete m_info;
     ./src/plugin/folderlistmodel/location.cpp:252:            delete m_info;

   Would you consider (adding this) or reconsider  for this item #4 only? 

-- 
https://code.launchpad.net/~renatofilho/ubuntu-filemanager-app/fix-network-crash/+merge/314870
Your team Ubuntu File Manager Developers is subscribed to branch 
lp:ubuntu-filemanager-app.

-- 
Mailing list: https://launchpad.net/~ubuntu-touch-coreapps-reviewers
Post to     : ubuntu-touch-coreapps-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~ubuntu-touch-coreapps-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to