RFR(S): 8201649: Remove dubious call_jio_print in ostream.cpp
David Holmes
david.holmes at oracle.com
Thu Apr 19 12:06:33 UTC 2018
Hi Christoph,
On 19/04/2018 9:08 PM, Langer, Christoph wrote:
> Hi Thomas, David,
>
>>> @David: No, I don't think it needs to be dialed back at this point, but I
>> agree, the bug should be updated somewhat to the current scope by
>> mentioning that the goal is to get rid of jio_print and jio_printf completely
>> (along with call_jio_print in ostream.cpp). Those 2 functions were just 2
>> hooks used in ostream.cpp and got routed through jio_vfprintf anyway,
>> except for the raw write case when no vfprint_hook was installed - and we'll
>> preserve that.
You preserved it in the sense it's still there, but you changed two call
sites so they can never now hit it. So I'm back to the unanswerable
"does that matter?".
Unless someone can do the archaeology on this I'm going to have to abstain.
David
-----
>>>
>>
>> Hm. Instead of broading the scope of the issue, I would actually
>> prefer the bug description to be clarified and limited to the original
>> problem we tried to solve. Because that was a real valid problem.
>>
>> If only to get David's full review and not to confuse him further :)
>
> Ok, I agree. So I tried to clarify the bug a bit more in its original sense. And we'd be back to webrev #0.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8201649
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201649.0/
>
> The change of the webrev is missing a cast in line 2729 - (len needs to be casted int). This produces a build warning which turns into an error in some platforms. I'll have that fixed in the submitted version.
>
> For the further cleanup of jio_print methods I'll probably file another bug.
>
> As I think we already had a principal agreement about this proposal earlier, I would consider this reviewed by stuefe and dholmes if I don't hear back from you today. I would push this tomorrow after push to jdk-submit and further internal testing here at SAP.
>
> Thanks
> Christoph
>
More information about the hotspot-runtime-dev
mailing list