RFR(S): 8201649: Remove dubious call_jio_print in ostream.cpp
thomas.stuefe at gmail.com
Thu Apr 19 12:27:27 UTC 2018
On Thu, Apr 19, 2018 at 1:08 PM, Langer, Christoph
<christoph.langer at sap.com> 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.
>> 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.
I am fine with webrev 0, beside the concern I already voiced about
feeding ("%.*s", prec, not-zero-terminated-string) to a vfprintf hook.
Not sure if David is though. I did not read his initial mail as
review, so I think we should wait for his confirmation.
More information about the hotspot-runtime-dev