lburgazzoli edited a comment on pull request #2766:
URL: https://github.com/apache/camel-k/pull/2766#issuecomment-976009518


   @astefanutti @squakez 
   
   I've somehow fixed some of the issues reported by `golanci-lint` and 
`gosec`, however I need some feedback for the fixes in particular for 
[this](https://github.com/apache/camel-k/pull/2766/commits/0a5b577a1a423db5c58019f98acd88afdb685998)
 commit where I've tried to address deferring invocation of "Close" on type 
"*os.File", like:
   
   ```go
   f, err := os.Open("/foo/bar.txt")
   if err != nil {
       return err
   }
   defer f.Close()
   ```
   
   The reason of this finding is that, when deferring the method call, we don't 
check if the file is really closed or not and in most of the case it is 
probably ok but it may cause very subtle bugs like we may leak file descriptor 
that may or may not impact the behavior of the application at some point (i.e. 
you may run out of file descriptors). So I think that when possible this 
problem should be addressed.
   
   However it is not very clear how to implement it. Some hint may be found in 
this blog post: https://www.joeshaw.org/dont-defer-close-on-writable-files.
   
   I've tried to implement what is written in that post but it does not seem to 
be applicable as a general rule so I ended up with some slightly different way 
(that also leverage some of the hints from the blog) that make use of closures. 
I'm not sure if that is the right approach or not so any feedback would be more 
that welcome.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to