8399 sun_fc: in C++11 destructors default to noexcept

Review Request #589 — Created June 15, 2017 and submitted

Information
tsoome
illumos-gate
8399
6b65b57...
Reviewers
general 
../common/Handle.cc: In destructor 'Handle::~Handle()':
../common/Handle.cc:172:6: error: throw will always call terminate() [-Werror=terminate]
      throw;
      ^~~~~
../common/Handle.cc:172:6: note: in C++11 destructors default to noexcept
../common/Handle.cc:187:6: error: throw will always call terminate() [-Werror=terminate]
      throw;
      ^~~~~
../common/Handle.cc:187:6: note: in C++11 destructors default to noexcept
cc1plus: all warnings being treated as errors




 













 

 






 
 
 

  

   
  

  
    
   
  • 
    
    0
   
    • 
   
  • 
    
    0
   
    • 
   
  • 
    
    2
   
    • 
   
  • 
    
    0
   
    • 
   
  • 
    
    2
   
    • 
  

 

 

  

   
    

     Description
     From
     Last Updated
    

   
   


    


    

   
  

 












  

 








 














 





 

  

   




marcel




   

   







  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/lib/sun_fc/common/Handle.cc
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    Since unlock() cannot throw an exception I suggest to move the unlock() calls out of the try-catch block.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/lib/sun_fc/common/Listener.cc
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    In a case the exception was thrown (and catched) in the above try-catch block, this will cause double unlock.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 





 

  

   




marcel




   

   





  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/lib/sun_fc/common/Listener.cc
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    I would just replace this by break.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




tsoome




   

   

  

 



 















 





 

  

   




marcel




   

   







  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




tsoome




   

   

  

 



 















 





 

  

   




vgusev




   

   







  

 



 

  


   

   

    
 
vgusev


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/lib/sun_fc/common/Handle.cc
      


       (Diff revision 3)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    By C++11 you also need to declare unlock() and lock() in class Lockable as noexcept, 
    
i.e. directly specify that lock/unlock cannot throw exception.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      hm, I guess so. unfortunately also we need to support pre C++11 for now at least, so some guarding is needed for time being.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Sorry for pointing to this piece of code. I thought the code was fully switched to c++11.
      

      Using throw; here is just signalling that std::find failed with exception and std::erase() was not called. But other resourses are leaked in this case.
      

      Anyway the other code is strange. Look at the line 306 in Handle.cc:
      

      HBA_HANDLE Handle::getHandle() {
        Trace log("Handle::getHandle");
        HBA_HANDLE tmp;
        lock();
        try {
            tmp = (HBA_HANDLE) id;
            unlock();
            return (tmp);
        } catch (...) {
            unlock();
            throw;
        }
}
      
Line is: "tmp = (HBA_HANLDE) id;". Typcast is excessive because type of id is HBA_HANDLE. And block "try" is excessive because this line does never thow exception.
      

      

      So I vote for revision 3, not for 5, because use throw in desctructor is real issue and we need to say to compiler "Thanks" :)
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




tsoome




   

   

  

 



 















 



 

  

   




tsoome




   

   

  

 



 















 





 

  

   




yuripv




   

   







  

 



 

  


   

   

    
 
yuripv


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




vgusev




   

   







  

 



 

  


   

   

    
 
vgusev


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




marcel




   

   







  

 



 

  


   

   

    
 
marcel


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




tsoome




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...