RFR(S): 8201649: Remove dubious call_jio_print in ostream.cpp (original) (raw)

David Holmes david.holmes at oracle.com
Wed Apr 18 05:59:10 UTC 2018

On 18/04/2018 2:47 PM, Thomas Stüfe wrote:

Hi David,

On Tue, Apr 17, 2018 at 11:17 PM, David Holmes <david.holmes at oracle.com> wrote: On 18/04/2018 1:48 AM, Thomas Stüfe wrote:

Hi Christoph, I do not understand jioprint() at all. I think it is just wrong: if a vfprintf hook is set, it prints to the defaultStream::outputstream(), otherwise to defaultStream::outputfd()? Isnt that the same? Compare this to jiovfprintf(), which does the same logic, but correctly prints to the vfprintf hook if it is set. If there is a hook it does a formatted print: jioprint -> jiofprintf -> jiovfprintf -> Arguments::vfprintfhook() else it does a raw write. Oh, I missed that. So, jioprint() checks if a vfprintf hook is installed. If it is, it calls jiofprintf() with defaultStream::outputstream() as output file handle. It expects jiofprintf() to call jiovfprintf(), which should do the same check again and come to the same conclusion, then should disregard the output file handle handed down, instead call the vfprintf hook. (The output file handle is handed down as argument to the vfprintf hook; we had a discussion last year about it: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-May/026794.html and we were not sure that made any sense.)

I had forgotten about that. :)

Not very clear coding. I think that can be improved by removing the vfprintf hook check from jioprint completely and make jiovfprintf() be the only place where the vfprintf hook is handled.

For example, this would be equivalent: void jioprint(const char* s) { jioprintf("%s", s); } or this: void jioprint(const char* s) { jiofprintf(defaultStream::outputstream(), "%s", s); }

They aren't equivalent because the always do a formatted write, never the raw write the currently exists. Does that matter? no idea. I don't have time to do any archaeology on this piece of code - sorry.

Cheers, David

in which case we could also maybe also remove jioprintf(), because it is nowhere used inside the hotspot (though it is exported, but I find no reference to it being used anywhere in the OpenJDK).

Now why it does this is another matter. I have no idea. But I wouldn't suggest changing it just because I don't know why it's done the way it is. I was reasonably sure I had understood the function, and that it was broken. In that case the proposal to change or remove it made sense. Thomas David

I would propose to get rid of jioprint() altogether and replace the few callers of it (all in ostream.cpp) with this: jioprintf("%s", string); which does the same, but correctly. Best Regards, Thomas On Tue, Apr 17, 2018 at 4:48 PM, Langer, Christoph <christoph.langer at sap.com> wrote: Hi, can you please review a fix proposal for defaultStream::write(const char* s, sizet len). Bug: https://bugs.openjdk.java.net/browse/JDK-8201649 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201649.0/ I have seen occurrences of truncated buffers which don't need to happen. Thanks and best regards Christoph

More information about the hotspot-dev mailing list