lburgazzoli commented 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 (it looks like the standard library is doing so) 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
