TomMD commented on a change in pull request #1997:
URL: https://github.com/apache/lucene-solr/pull/1997#discussion_r507839162
##########
File path: solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
##########
@@ -415,19 +415,15 @@ private void fetchIndex(SolrParams solrParams,
SolrQueryResponse rsp) throws Int
}
static Long getCheckSum(Checksum checksum, File f) {
- FileInputStream fis = null;
checksum.reset();
byte[] buffer = new byte[1024 * 1024];
int bytesRead;
- try {
- fis = new FileInputStream(f);
+ try (final FileInputStream fis = new FileInputStream(f)) {
while ((bytesRead = fis.read(buffer)) >= 0)
checksum.update(buffer, 0, bytesRead);
return checksum.getValue();
} catch (Exception e) {
Review comment:
@madrob Could you say more about why you think it's a bug? Do you think
it is a false positive? I don't have nearly the same view into the code as you
do, so perhaps you are right but my unfamiliar reading leads me to another
conclusion.
This Muse result originates from Infer - I can get the full trace if you'd
like but by eye I'd expect it to say:
1. `fis` is allocated a new FileInputStream on line 421.
2. The fall to `fis.read()` could throw an IO exception.
3. There are no further references to `fis` in the code path.
Notably, the prior code of `finally { IOUtils.closeQuietly(fis); }` has been
removed. S not only Infer, but also some prior author, believe `fis` needs
closed and now a path exists where `fis` is not closed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
