RFR: 8221541: clean up functions in CompilerOracle - was : RE: CompilerOracle::append_comment_to_file and CompilerOracle::append_exclude_to_file - missing fclose calls ?

[Doerr, Martin]

Could get pushed under trivial rule.
But you already have 2 reviews.

Best regards,
Martin


-----Original Message-----
From: Baesken, Matthias 
Sent: Freitag, 29. März 2019 17:01
To: Doerr, Martin <martin.doerr at sap.com>
Cc: hotspot-dev at openjdk.java.net
Subject: Re: RFR: 8221541: clean up functions in CompilerOracle - was : RE: CompilerOracle::append_comment_to_file and CompilerOracle::append_exclude_to_file - missing fclose calls ?

Thanks!
Can this be pushed as XS change with 1 review?

Von meinem iPhone gesendet

> Am 29.03.2019 um 16:33 schrieb Doerr, Martin <martin.doerr at sap.com>:
> 
> Hi Matthias,
> 
> looks good.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of Vladimir Kozlov
> Sent: Mittwoch, 27. März 2019 17:36
> To: hotspot-dev at openjdk.java.net
> Subject: Re: RFR: 8221541: clean up functions in CompilerOracle - was : RE: CompilerOracle::append_comment_to_file and CompilerOracle::append_exclude_to_file - missing fclose calls ?
> 
> Changes looks good.
> 
> It was part of debug functionality in C2 which was removed in jdk7u but adlc part was not:
> 
> http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/a5dd6e3ef9f3#l6.7
> 
> Thanks,
> Vladimir
> 
>> On 3/27/19 2:29 AM, Baesken, Matthias wrote:
>> Hi Thomas, good point .
>> I removed the 2 functions .
>> Btw.  I checked them (using  nm )   in the  product   libjvm.so  and they showed up in the binary .
>> So  it  might  help also  to get the binary a little bit smaller  …
>> 
>> 
>> Please review !
>> 
>> Bug/webrev :
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8221541
>> 
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8221541.0/
>> 
>> 
>> Thanks, Matthias
>> 
>> 
>> From: Thomas Stüfe <thomas.stuefe at gmail.com>
>> Sent: Dienstag, 26. März 2019 20:40
>> To: Baesken, Matthias <matthias.baesken at sap.com>
>> Cc: hotspot-dev at openjdk.java.net
>> Subject: Re: CompilerOracle::append_comment_to_file and CompilerOracle::append_exclude_to_file - missing fclose calls ?
>> 
>> Hi Matthias,
>> 
>> On Tue, Mar 26, 2019 at 4:17 PM Baesken, Matthias <matthias.baesken at sap.com<mailto:matthias.baesken at sap.com>> wrote:
>> 
>> Hello, it seems that the  fopen calls in CompilerOracle::append_comment_to_file and
>> CompilerOracle::append_exclude_to_file  do not have a corresponding fclose.
>> Or do I miss something ?
>> 
>> No, you are right, this is a bug. However, I think those functions are nowhere used, so maybe they could be removed completely.
>> 
>> Cheers, Thomas
>> 
>> We could also add a "true"  for the second argument of the fileStream constructor calls  ( fileStream(FILE* file, bool need_close = false)  )
>> to get the closing.
>> 
>> 
>> coding :
>> ------------
>> 
>> jdk/src/hotspot/share/compiler/compilerOracle.cpp
>> 
>> 
>> 741void CompilerOracle::append_comment_to_file(const char* message) {
>> 742  assert(has_command_file(), "command file must be specified");
>> 743  fileStream stream(fopen(cc_file(), "at"));
>> 744  stream.print("# ");
>> 745  for (int index = 0; message[index] != '\0'; index++) {
>> 746    stream.put(message[index]);
>> 747    if (message[index] == '\n') stream.print("# ");
>> 748  }
>> 749  stream.cr<http://stream.cr>();
>> 750}
>> 751
>> 752void CompilerOracle::append_exclude_to_file(const methodHandle& method) {
>> 753  assert(has_command_file(), "command file must be specified");
>> 754  fileStream stream(fopen(cc_file(), "at"));
>> 755  stream.print("exclude ");
>> 756  method->method_holder()->name()->print_symbol_on(&stream);
>> 757  stream.print(".");
>> 758  method->name()->print_symbol_on(&stream);
>> 759  method->signature()->print_symbol_on(&stream);
>> 760  stream.cr<http://stream.cr>();
>> 761  stream.cr<http://stream.cr>();
>> 762}
>> 
>> Best regards, Matthias
>>