RFR(XXS): 8242485: Null _file checking in fileStream::flush()
david.holmes at oracle.com
Sun Apr 12 02:38:29 UTC 2020
On 12/04/2020 1:52 am, Denghui Dong wrote:
> Thank you for the review and point out other methods need to check _file.
> In addition to "fileStream::flush", I found there are still four methods need to check _file in fileStream: read, readln, eof, rewind.
> but those methods will crash if _file is NULL, and I didn't find any caller of them(Maybe I am wrong),
> so I think it's more appropriate to add assert check to those methods.
Assertions are really only appropriate if they will be exercised by
tests - the intent being that tests will expose any cases where the
Also it doesn't make sense to me to have assertions in most places but a
true NULL check in others - that suggests we expect some methods to be
called when _file is NULL, but others not. I'd prefer just to see the
missing NULL checks added, rather than using assertions.
> Webrev: http://cr.openjdk.java.net/~ddong/8242485/webrev.01/
> Could you review it again, and sponsor it if everything is okay?
> Testing: teir1
> Test summary
> TEST TOTAL PASS FAIL ERROR
> jtreg:test/hotspot/jtreg:tier1 1516 1516 0 0
> jtreg:test/jdk:tier1 1904 1904 0 0
> >> jtreg:test/langtools:tier1 4031 4029 2 0 <<
> jtreg:test/nashorn:tier1 0 0 0 0
> jtreg:test/jaxp:tier1 0 0 0 0
> There are two unrelated test failures in
> langtools: jdk/javadoc/tool/CheckResourceKeys.java and tools/javac/processing/model/TestSymtabItems.java
> From:Yasumasa Suenaga <suenaga at oss.nttdata.com>
> Send Time:2020年4月11日(星期六) 09:22
> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>; hotspot-dev
> <hotspot-dev at openjdk.java.net>; hotspot-runtime-dev at openjdk.java.net
> <hotspot-runtime-dev at openjdk.java.net>
> Subject:Re: RFR(XXS): 8242485: Null _file checking in
> Hi Denghui,
> null check lacks in other place too.
> Can you fix it?
> On 2020/04/10 21:34, Denghui Dong wrote:
> > Hi team,
> > Could you please review this small patch?
> > JBS: https://bugs.openjdk.java.net/browse/JDK-8242485
> > webrev: http://cr.openjdk.java.net/~ddong/8242485/webrev.00/
> > I found the method "fileStream::flush()" lacks null check before fflush which will cause
> > all open output streams are flushed by fflush() if _file is NULL.
> > Thanks,
> > Denghui Dong
More information about the hotspot-runtime-dev